Backed out 3 changesets (bug 1782295) for causing Assertion failures on nsDirectoryService.cpp CLOSED TREE

Backed out changeset b332385ffca4 (bug 1782295)
Backed out changeset 8c36812a12dc (bug 1782295)
Backed out changeset 8016b1f25509 (bug 1782295)
This commit is contained in:
Cristian Tuns 2022-08-26 11:50:40 -04:00
parent b292d44a53
commit cabdc5ed0b
8 changed files with 51 additions and 146 deletions

View File

@ -654,8 +654,9 @@ pref("browser.privatebrowsing.enable-new-logo", true);
pref("browser.privacySegmentation.enabled", false);
// Temporary pref to control whether or not Private Browsing windows show up
// as separate icons in the Windows taskbar.
pref("browser.privacySegmentation.windowSeparation.enabled", true);
// as separate icons in the Windows taskbar. This will be removed and become
// the default behaviour with 106.
pref("browser.privacySegmentation.windowSeparation.enabled", false);
// Use dark theme variant for PBM windows. This is only supported if the theme
// sets darkTheme data.

View File

@ -22,7 +22,6 @@ XPCOMUtils.defineLazyModuleGetters(lazy, {
HomePage: "resource:///modules/HomePage.jsm",
FirstStartup: "resource://gre/modules/FirstStartup.jsm",
LaterRun: "resource:///modules/LaterRun.jsm",
NimbusFeatures: "resource://nimbus/ExperimentAPI.jsm",
PrivateBrowsingUtils: "resource://gre/modules/PrivateBrowsingUtils.jsm",
SessionStartup: "resource:///modules/sessionstore/SessionStartup.jsm",
ShellService: "resource:///modules/ShellService.jsm",
@ -45,6 +44,8 @@ const ONCE_PREF = "browser.startup.homepage_override.once";
// Index of Private Browsing icon in firefox.exe
// Must line up with the one in nsNativeAppSupportWin.h.
const PRIVATE_BROWSING_ICON_INDEX = 5;
const PRIVATE_WINDOW_SEPARATION_PREF =
"browser.privacySegmentation.windowSeparation.enabled";
function shouldLoadURI(aURI) {
if (aURI && !aURI.schemeIs("chrome")) {
@ -282,11 +283,11 @@ function openBrowserWindow(
win.docShell.QueryInterface(
Ci.nsILoadContext
).usePrivateBrowsing = true;
if (
lazy.NimbusFeatures.majorRelease2022.getVariable(
"feltPrivacyWindowSeparation"
)
) {
if (Services.prefs.getBoolPref(PRIVATE_WINDOW_SEPARATION_PREF)) {
// TODO: Changing this after the Window has been painted causes it to
// change Taskbar icons if the original one had a different AUMID.
// This must stay pref'ed off until this is resolved.
// https://bugzilla.mozilla.org/show_bug.cgi?id=1751010
lazy.WinTaskbar.setGroupIdForWindow(
win,
lazy.WinTaskbar.defaultPrivateGroupId

View File

@ -143,6 +143,8 @@ const PRIVATE_BROWSING_BINARY = "private_browsing.exe";
// Index of Private Browsing icon in private_browsing.exe
// Must line up with IDI_PBICON_PB_PB_EXE in nsNativeAppSupportWin.h.
const PRIVATE_BROWSING_EXE_ICON_INDEX = 1;
const PREF_PRIVATE_WINDOW_SEPARATION =
"browser.privacySegmentation.windowSeparation.enabled";
const PREF_PRIVATE_BROWSING_SHORTCUT_CREATED =
"browser.privacySegmentation.createdShortcut";
@ -2534,9 +2536,7 @@ BrowserGlue.prototype = {
// Pref'ed off until Private Browsing window separation is enabled by default
// to avoid a situation where a user pins the Private Browsing shortcut to
// the Taskbar, which will end up launching into a different Taskbar icon.
lazy.NimbusFeatures.majorRelease2022.getVariable(
"feltPrivacyWindowSeparation"
) &&
Services.prefs.getBoolPref(PREF_PRIVATE_WINDOW_SEPARATION, false) &&
// Private Browsing shortcuts for packaged builds come with the package,
// if they exist at all. We shouldn't try to create our own.
!Services.sysinfo.getProperty("hasWinPackageId") &&
@ -2556,10 +2556,10 @@ BrowserGlue.prototype = {
);
if (
!(await shellService.hasMatchingShortcut(
!shellService.hasMatchingShortcut(
winTaskbar.defaultPrivateGroupId,
true
))
)
) {
let appdir = Services.dirsvc.get("GreD", Ci.nsIFile);
let exe = appdir.clone();
@ -2571,7 +2571,7 @@ BrowserGlue.prototype = {
let [desc] = await strings.formatValues([
"private-browsing-shortcut-text",
]);
await shellService.createShortcut(
shellService.createShortcut(
exe,
[],
desc,
@ -2583,16 +2583,11 @@ BrowserGlue.prototype = {
desc + ".lnk",
appdir
);
Services.prefs.setBoolPref(
PREF_PRIVATE_BROWSING_SHORTCUT_CREATED,
true
);
}
// We always set this as long as no exception has been thrown. This
// ensure that it is `true` both if we created one because it didn't
// exist, or if it already existed (most likely because it was created
// by the installer). This avoids the need to call `hasMatchingShortcut`
// again, which necessarily does pointless I/O.
Services.prefs.setBoolPref(
PREF_PRIVATE_BROWSING_SHORTCUT_CREATED,
true
);
},
},

View File

@ -47,8 +47,7 @@ interface nsIWindowsShellService : nsISupports
* created or accessed
* @throws NS_ERROR_FAILURE for other types of failures
*/
[implicit_jscontext]
Promise createShortcut(in nsIFile aBinary, in Array<AString> aArguments,
AString createShortcut(in nsIFile aBinary, in Array<AString> aArguments,
in AString aDescription, in nsIFile aIconFile, in unsigned short aIconIndex,
in AString aAppUserModelId, in AString aShortcutFolder, in AString aShortcutName);
@ -145,8 +144,7 @@ interface nsIWindowsShellService : nsISupports
*/
AString classifyShortcut(in AString aPath);
[implicit_jscontext]
Promise hasMatchingShortcut(in AString aAUMID, in bool aPrivateBrowsing);
bool hasMatchingShortcut(in AString aAUMID, in bool aPrivateBrowsing);
/*
* Check if setDefaultBrowserUserChoice() is expected to succeed.

View File

@ -778,7 +778,7 @@ static nsresult WriteShortcutToLog(KNOWNFOLDERID aFolderId,
}
static nsresult CreateShortcutImpl(
nsIFile* aBinary, const CopyableTArray<nsString>& aArguments,
nsIFile* aBinary, const nsTArray<nsString>& aArguments,
const nsAString& aDescription, nsIFile* aIconFile, uint16_t aIconIndex,
const nsAString& aAppUserModelId, KNOWNFOLDERID aShortcutFolder,
const nsAString& aShortcutName, nsAString& aShortcutPath) {
@ -874,18 +874,7 @@ nsWindowsShellService::CreateShortcut(
nsIFile* aBinary, const nsTArray<nsString>& aArguments,
const nsAString& aDescription, nsIFile* aIconFile, uint16_t aIconIndex,
const nsAString& aAppUserModelId, const nsAString& aShortcutFolder,
const nsAString& aShortcutName, JSContext* aCx, dom::Promise** aPromise) {
if (!NS_IsMainThread()) {
return NS_ERROR_NOT_SAME_THREAD;
}
ErrorResult rv;
RefPtr<dom::Promise> promise =
dom::Promise::Create(xpc::CurrentNativeGlobal(aCx), rv);
if (MOZ_UNLIKELY(rv.Failed())) {
return rv.StealNSResult();
}
const nsAString& aShortcutName, nsAString& aShortcutPath) {
// In an ideal world we'd probably send along nsIFile pointers
// here, but it's easier to determine the needed shortcuts log
// entry with a KNOWNFOLDERID - so we pass this along instead
@ -900,48 +889,9 @@ nsWindowsShellService::CreateShortcut(
return NS_ERROR_INVALID_ARG;
}
auto promiseHolder = MakeRefPtr<nsMainThreadPtrHolder<dom::Promise>>(
"CreateShortcut promise", promise);
RefPtr<nsIFile> binary(aBinary);
RefPtr<nsIFile> iconFile(aIconFile);
NS_DispatchBackgroundTask(
NS_NewRunnableFunction(
"CreateShortcut",
[binary, aArguments = CopyableTArray<nsString>(aArguments),
aDescription = nsString{aDescription}, iconFile, aIconIndex,
aAppUserModelId = nsString{aAppUserModelId}, folderId,
aShortcutFolder = nsString{aShortcutFolder},
aShortcutName = nsString{aShortcutName},
promiseHolder = std::move(promiseHolder)] {
nsresult rv = NS_ERROR_FAILURE;
nsAutoString shortcutPath;
HRESULT hr = CoInitialize(nullptr);
if (SUCCEEDED(hr)) {
rv = CreateShortcutImpl(binary.get(), aArguments, aDescription,
iconFile.get(), aIconIndex,
aAppUserModelId, folderId, aShortcutName,
shortcutPath);
CoUninitialize();
}
NS_DispatchToMainThread(NS_NewRunnableFunction(
"CreateShortcut callback",
[rv, shortcutPath, promiseHolder = std::move(promiseHolder)] {
dom::Promise* promise = promiseHolder.get()->get();
if (NS_SUCCEEDED(rv)) {
promise->MaybeResolve(shortcutPath);
} else {
promise->MaybeReject(rv);
}
}));
}),
NS_DISPATCH_EVENT_MAY_BLOCK);
promise.forget(aPromise);
return NS_OK;
return CreateShortcutImpl(aBinary, aArguments, aDescription, aIconFile,
aIconIndex, aAppUserModelId, folderId,
aShortcutName, aShortcutPath);
}
// Constructs a path to an installer-created shortcut, under a directory
@ -1105,35 +1055,10 @@ static nsresult FindMatchingShortcut(const nsAString& aAppUserModelId,
return NS_ERROR_FILE_NOT_FOUND;
}
static bool HasMatchingShortcutImpl(const nsAString& aAppUserModelId,
const bool aPrivateBrowsing,
const nsAutoString& aShortcutName) {
// unused by us, but required
nsAutoString shortcutPath;
nsresult rv =
FindMatchingShortcut(aAppUserModelId, aShortcutName, shortcutPath);
if (SUCCEEDED(rv)) {
return true;
}
return false;
}
NS_IMETHODIMP nsWindowsShellService::HasMatchingShortcut(
const nsAString& aAppUserModelId, const bool aPrivateBrowsing,
JSContext* aCx, dom::Promise** aPromise) {
if (!NS_IsMainThread()) {
return NS_ERROR_NOT_SAME_THREAD;
}
ErrorResult rv;
RefPtr<dom::Promise> promise =
dom::Promise::Create(xpc::CurrentNativeGlobal(aCx), rv);
if (MOZ_UNLIKELY(rv.Failed())) {
return rv.StealNSResult();
}
NS_IMETHODIMP
nsWindowsShellService::HasMatchingShortcut(const nsAString& aAppUserModelId,
const bool aPrivateBrowsing,
bool* aHasMatch) {
// NOTE: In the installer, non-private shortcuts are named
// "${BrandShortName}.lnk". This is set from MOZ_APP_DISPLAYNAME in
// defines.nsi.in. (Except in dev edition where it's explicitly set to
@ -1163,34 +1088,15 @@ NS_IMETHODIMP nsWindowsShellService::HasMatchingShortcut(
shortcutName.AppendLiteral(MOZ_APP_DISPLAYNAME ".lnk");
}
auto promiseHolder = MakeRefPtr<nsMainThreadPtrHolder<dom::Promise>>(
"HasMatchingShortcut promise", promise);
nsAutoString shortcutPath;
nsresult rv =
FindMatchingShortcut(aAppUserModelId, shortcutName, shortcutPath);
if (SUCCEEDED(rv)) {
*aHasMatch = true;
} else {
*aHasMatch = false;
}
NS_DispatchBackgroundTask(
NS_NewRunnableFunction(
"HasMatchingShortcut",
[aAppUserModelId = nsString{aAppUserModelId}, aPrivateBrowsing,
shortcutName, promiseHolder = std::move(promiseHolder)] {
bool rv = false;
HRESULT hr = CoInitialize(nullptr);
if (SUCCEEDED(hr)) {
rv = HasMatchingShortcutImpl(aAppUserModelId, aPrivateBrowsing,
shortcutName);
CoUninitialize();
}
NS_DispatchToMainThread(NS_NewRunnableFunction(
"HasMatchingShortcut callback",
[rv, promiseHolder = std::move(promiseHolder)] {
dom::Promise* promise = promiseHolder.get()->get();
promise->MaybeResolve(rv);
}));
}),
NS_DISPATCH_EVENT_MAY_BLOCK);
promise.forget(aPromise);
return NS_OK;
}

View File

@ -619,7 +619,16 @@ Section "-Application" APP_IDX
; native "Pin to Taskbar" functionality can find an appropriate shortcut.
; See https://bugzilla.mozilla.org/show_bug.cgi?id=1762994 for additional
; background.
${AddPrivateBrowsingShortcut}
; Pref'ed off until Private Browsing window separation is enabled by default
; to avoid a situation where a user pins the Private Browsing shortcut to
; the Taskbar, which will end up launching into a different Taskbar icon.
ClearErrors
ReadRegDWORD $2 HKCU \
"Software\Mozilla\${AppName}\Installer\$AppUserModelID" \
"CreatePrivateBrowsingShortcut"
${IfNot} ${Errors}
${AddPrivateBrowsingShortcut}
${EndIf}
; Update lastwritetime of the Start Menu shortcut to clear the tile cache.
; Do this for both shell contexts in case the user has shortcuts in multiple

View File

@ -602,7 +602,3 @@ majorRelease2022:
type: boolean
fallbackPref: "browser.privacySegmentation.preferences.show"
description: "Controls visibility of the privacy segmentation preferences section."
feltPrivacyWindowSeparation:
type: boolean
fallbackPref: "browser.privacySegmentation.windowSeparation.enabled"
description: "Whether or not private browsing windows use a separate icon in the Windows taskbar"

View File

@ -160,7 +160,6 @@
#include "mozilla/StaticPrefs_layout.h"
#include "mozilla/StaticPrefs_widget.h"
#include "nsNativeAppSupportWin.h"
#include "mozilla/browser/NimbusFeatures.h"
#include "nsIGfxInfo.h"
#include "nsUXThemeConstants.h"
@ -1113,8 +1112,8 @@ nsresult nsWindow::Create(nsIWidget* aParent, nsNativeWidget aNativeParent,
}
if (aInitData->mIsPrivate) {
if (NimbusFeatures::GetBool("majorRelease2022"_ns,
"feltPrivacyWindowSeparation"_ns, true) &&
if (Preferences::GetBool(
"browser.privacySegmentation.windowSeparation.enabled", false) &&
// Although permanent Private Browsing mode is indeed Private Browsing,
// we choose to make it look like regular Firefox in terms of the icon
// it uses (which also means we shouldn't use the Private Browsing