Bug 1904436 - Implement private browser pinning using new Windows taskbar APIs r=nrishel,win-reviewers,rkraesig

Differential Revision: https://phabricator.services.mozilla.com/D214757

browser/components/shell/Windows11TaskbarPinning.cpp

@@ -9,10 +9,14 @@
 #include "nsWindowsHelpers.h"
 #include "MainThreadUtils.h"
 #include "nsThreadUtils.h"
+#include <shobjidl.h>
 #include <strsafe.h>
 
 #include "mozilla/Result.h"
 #include "mozilla/ResultVariant.h"
+#include "mozilla/UniquePtr.h"
+#include "mozilla/WinHeaderOnlyUtils.h"
+#include "mozilla/widget/WinTaskbar.h"
 
 #include "mozilla/Logging.h"
 
@@ -27,6 +31,7 @@ static mozilla::LazyLogModule sLog("Windows11TaskbarPinning");
 
 #  include <inspectable.h>
 #  include <roapi.h>
+#  include <shlobj_core.h>
 #  include <windows.services.store.h>
 #  include <windows.foundation.h>
 #  include <windows.ui.shell.h>
@@ -101,8 +106,7 @@ static Result<ComPtr<ITaskbarManager>, HRESULT> InitializeTaskbar() {
 }
 
 Win11PinToTaskBarResult PinCurrentAppToTaskbarWin11(
-    bool aCheckOnly, const nsAString& aAppUserModelId,
-    nsAutoString aShortcutPath) {
+    bool aCheckOnly, const nsAString& aAppUserModelId) {
   MOZ_DIAGNOSTIC_ASSERT(!NS_IsMainThread(),
                         "PinCurrentAppToTaskbarWin11 should be called off main "
                         "thread only. It blocks, waiting on things to execute "
@@ -137,13 +141,39 @@ Win11PinToTaskBarResult PinCurrentAppToTaskbarWin11(
   // Everything related to the taskbar and pinning must be done on the main /
   // user interface thread or Windows will cause them to fail.
   NS_DispatchToMainThread(NS_NewRunnableFunction(
-      "PinCurrentAppToTaskbarWin11", [&event, &hr, &resultStatus, aCheckOnly] {
-        auto CompletedOperations =
-            [&event, &resultStatus](Win11PinToTaskBarResultStatus status) {
+      "PinCurrentAppToTaskbarWin11", [&event, &hr, &resultStatus, aCheckOnly,
+                                      aumid = nsString(aAppUserModelId)] {
+        // We eventualy want to call SetCurrentProcessExplicitAppUserModelID()
+        // on the main thread as it is not thread safe and pinning is called
+        // numerous times in many different places. This is a hack used
+        // explicitly for the purpose of re-enabling private browser pinning
+        // as a stopgap and should not be replicated elsewhere.
+        // GenerateAppUserModelId needs to be called on the main thread as
+        // it checks against preferences.
+        nsAutoString primaryAumid;
+        mozilla::widget::WinTaskbar::GenerateAppUserModelID(primaryAumid,
+                                                            false);
+        auto CompletedOperations = [&event, &resultStatus,
+                                    primaryAumid = nsString(primaryAumid)](
+                                       Win11PinToTaskBarResultStatus status) {
+          // Set AUMID back and ensure the icon is set correctly
+          HRESULT hr =
+              SetCurrentProcessExplicitAppUserModelID(primaryAumid.get());
+          if (FAILED(hr)) {
+            TASKBAR_PINNING_LOG(LogLevel::Debug,
+                                "Taskbar: reverting AUMID after pinning "
+                                "operation failed. HRESULT = 0x%lx",
+                                hr);
+          }
           resultStatus = status;
           event.Set();
         };
 
+        hr = SetCurrentProcessExplicitAppUserModelID(aumid.get());
+        if (FAILED(hr)) {
+          return CompletedOperations(Win11PinToTaskBarResultStatus::Failed);
+        }
+
         auto result = InitializeTaskbar();
         if (result.isErr()) {
           hr = result.unwrapErr();
@@ -209,12 +239,22 @@ Win11PinToTaskBarResult PinCurrentAppToTaskbarWin11(
         // be alive until the async functions complete, so they can be used as
         // references.
         auto isPinnedCallback = Callback<IAsyncOperationCompletedHandler<
-            bool>>([taskbar, &event, &resultStatus, &hr](
+            bool>>([taskbar, &event, &resultStatus, &hr,
+                    primaryAumid = nsString(primaryAumid)](
                        IAsyncOperation<bool>* asyncInfo,
                        AsyncStatus status) mutable -> HRESULT {
           auto CompletedOperations =
-              [&event,
-               &resultStatus](Win11PinToTaskBarResultStatus status) -> HRESULT {
+              [&event, &resultStatus,
+               primaryAumid](Win11PinToTaskBarResultStatus status) -> HRESULT {
+            // Set AUMID back and ensure the icon is set correctly
+            HRESULT hr =
+                SetCurrentProcessExplicitAppUserModelID(primaryAumid.get());
+            if (FAILED(hr)) {
+              TASKBAR_PINNING_LOG(LogLevel::Debug,
+                                  "Taskbar: reverting AUMID after pinning "
+                                  "operation failed. HRESULT = 0x%lx",
+                                  hr);
+            }
             resultStatus = status;
             event.Set();
             return S_OK;
@@ -336,8 +376,7 @@ Win11PinToTaskBarResult PinCurrentAppToTaskbarWin11(
 #else  // MINGW32 implementation below
 
 Win11PinToTaskBarResult PinCurrentAppToTaskbarWin11(
-    bool aCheckOnly, const nsAString& aAppUserModelId,
-    nsAutoString aShortcutPath) {
+    bool aCheckOnly, const nsAString& aAppUserModelId) {
   return {S_OK, Win11PinToTaskBarResultStatus::NotSupported};
 }
 

browser/components/shell/Windows11TaskbarPinning.h

@@ -29,7 +29,6 @@ struct Win11PinToTaskBarResult {
 };
 
 Win11PinToTaskBarResult PinCurrentAppToTaskbarWin11(
-    bool aCheckOnly, const nsAString& aAppUserModelId,
-    nsAutoString aShortcutPath);
+    bool aCheckOnly, const nsAString& aAppUserModelId);
 
 #endif  // SHELL_WINDOWS11TASKBARPINNING_H__

browser/components/shell/nsWindowsShellService.cpp

@@ -1730,7 +1730,7 @@ static nsresult PinCurrentAppToTaskbarImpl(
   }
 
   auto pinWithWin11TaskbarAPIResults =
-      PinCurrentAppToTaskbarWin11(aCheckOnly, aAppUserModelId, shortcutPath);
+      PinCurrentAppToTaskbarWin11(aCheckOnly, aAppUserModelId);
   switch (pinWithWin11TaskbarAPIResults.result) {
     case Win11PinToTaskBarResultStatus::NotSupported:
       // Fall through to the win 10 mechanism

widget/windows/nsWindow.cpp

@@ -992,22 +992,24 @@ nsresult nsWindow::Create(nsIWidget* aParent, nsNativeWidget aNativeParent,
     }
   }
 
-  if (Preferences::GetBool("browser.privateWindowSeparation.enabled", true) &&
-      (aInitData->mIsPrivate)) {
+  {
     // 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
     // AUMID).
-    if (!StaticPrefs::browser_privatebrowsing_autostart()) {
+    bool usePrivateAumid =
+        Preferences::GetBool("browser.privateWindowSeparation.enabled", true) &&
+        (aInitData->mIsPrivate) &&
+        !StaticPrefs::browser_privatebrowsing_autostart();
     RefPtr<IPropertyStore> pPropStore;
     if (!FAILED(SHGetPropertyStoreForWindow(mWnd, IID_IPropertyStore,
                                             getter_AddRefs(pPropStore)))) {
       PROPVARIANT pv;
       nsAutoString aumid;
-        // make sure we're using the private browsing AUMID so that taskbar
+      // Make sure we're using the correct AUMID so that taskbar
       // grouping works properly
-        Unused << NS_WARN_IF(
-            !mozilla::widget::WinTaskbar::GenerateAppUserModelID(aumid, true));
+      Unused << NS_WARN_IF(!mozilla::widget::WinTaskbar::GenerateAppUserModelID(
+          aumid, usePrivateAumid));
       if (!FAILED(InitPropVariantFromString(aumid.get(), &pv))) {
         if (!FAILED(pPropStore->SetValue(PKEY_AppUserModel_ID, pv))) {
           pPropStore->Commit();
@@ -1016,12 +1018,12 @@ nsresult nsWindow::Create(nsIWidget* aParent, nsNativeWidget aNativeParent,
         PropVariantClear(&pv);
       }
     }
-      HICON icon = ::LoadIconW(::GetModuleHandleW(nullptr),
-                               MAKEINTRESOURCEW(IDI_PBMODE));
+    HICON icon = ::LoadIconW(
+        ::GetModuleHandleW(nullptr),
+        MAKEINTRESOURCEW(usePrivateAumid ? IDI_PBMODE : IDI_APPICON));
     SetBigIcon(icon);
     SetSmallIcon(icon);
   }
-  }
 
   mDeviceNotifyHandle = InputDeviceUtils::RegisterNotification(mWnd);