From cabdc5ed0ba4c0eee24a5e5266d5b76ea526072c Mon Sep 17 00:00:00 2001 From: Cristian Tuns Date: Fri, 26 Aug 2022 11:50:40 -0400 Subject: [PATCH] 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) --- browser/app/profile/firefox.js | 5 +- browser/components/BrowserContentHandler.jsm | 13 +- browser/components/BrowserGlue.jsm | 25 ++-- .../shell/nsIWindowsShellService.idl | 6 +- .../shell/nsWindowsShellService.cpp | 128 +++--------------- browser/installer/windows/nsis/installer.nsi | 11 +- .../components/nimbus/FeatureManifest.yaml | 4 - widget/windows/nsWindow.cpp | 5 +- 8 files changed, 51 insertions(+), 146 deletions(-) diff --git a/browser/app/profile/firefox.js b/browser/app/profile/firefox.js index 13c3746355cb..4ed08727a888 100644 --- a/browser/app/profile/firefox.js +++ b/browser/app/profile/firefox.js @@ -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. diff --git a/browser/components/BrowserContentHandler.jsm b/browser/components/BrowserContentHandler.jsm index 3bdf00c43300..98f955207c52 100644 --- a/browser/components/BrowserContentHandler.jsm +++ b/browser/components/BrowserContentHandler.jsm @@ -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 diff --git a/browser/components/BrowserGlue.jsm b/browser/components/BrowserGlue.jsm index 68fb3383d494..d727eefd18ac 100644 --- a/browser/components/BrowserGlue.jsm +++ b/browser/components/BrowserGlue.jsm @@ -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 - ); }, }, diff --git a/browser/components/shell/nsIWindowsShellService.idl b/browser/components/shell/nsIWindowsShellService.idl index 26025290e9bc..380449dbabd0 100644 --- a/browser/components/shell/nsIWindowsShellService.idl +++ b/browser/components/shell/nsIWindowsShellService.idl @@ -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 aArguments, + AString createShortcut(in nsIFile aBinary, in Array 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. diff --git a/browser/components/shell/nsWindowsShellService.cpp b/browser/components/shell/nsWindowsShellService.cpp index 2dd566b9d1a9..fd0144daf5c3 100644 --- a/browser/components/shell/nsWindowsShellService.cpp +++ b/browser/components/shell/nsWindowsShellService.cpp @@ -778,7 +778,7 @@ static nsresult WriteShortcutToLog(KNOWNFOLDERID aFolderId, } static nsresult CreateShortcutImpl( - nsIFile* aBinary, const CopyableTArray& aArguments, + nsIFile* aBinary, const nsTArray& 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& 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 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>( - "CreateShortcut promise", promise); - - RefPtr binary(aBinary); - RefPtr iconFile(aIconFile); - NS_DispatchBackgroundTask( - NS_NewRunnableFunction( - "CreateShortcut", - [binary, aArguments = CopyableTArray(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 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>( - "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; } diff --git a/browser/installer/windows/nsis/installer.nsi b/browser/installer/windows/nsis/installer.nsi index 6f344a55ee30..b0d5a13cce91 100755 --- a/browser/installer/windows/nsis/installer.nsi +++ b/browser/installer/windows/nsis/installer.nsi @@ -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 diff --git a/toolkit/components/nimbus/FeatureManifest.yaml b/toolkit/components/nimbus/FeatureManifest.yaml index 165fa34ad0dc..888cbbff019a 100644 --- a/toolkit/components/nimbus/FeatureManifest.yaml +++ b/toolkit/components/nimbus/FeatureManifest.yaml @@ -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" diff --git a/widget/windows/nsWindow.cpp b/widget/windows/nsWindow.cpp index 73400accc1f8..6c082e0bdb34 100644 --- a/widget/windows/nsWindow.cpp +++ b/widget/windows/nsWindow.cpp @@ -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