Bug 1794159 - Part 3: Enable MSIX pinning in nsWindowsShellService r=nrishel,nalexander,firefox-desktop-core-reviewers

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

browser/components/BrowserGlue.sys.mjs

@@ -2757,12 +2757,20 @@ BrowserGlue.prototype = {
                 winTaskbar.defaultGroupId
               )
             );
-            Services.telemetry.scalarSet(
+            // Bug 1911343: Pinning regular browsing on MSIX
-              "os.environment.is_taskbar_pinned_private",
+            // causes false positives when checking for private
-              await shellService.isCurrentAppPinnedToTaskbarAsync(
+            // browsing.
-                winTaskbar.defaultPrivateGroupId
+            if (
-              )
+              AppConstants.platform === "win" &&
-            );
+              !Services.sysinfo.getProperty("hasWinPackageId")
+            ) {
+              Services.telemetry.scalarSet(
+                "os.environment.is_taskbar_pinned_private",
+                await shellService.isCurrentAppPinnedToTaskbarAsync(
+                  winTaskbar.defaultPrivateGroupId
+                )
+              );
+            }
           } catch (ex) {
             console.error(ex);
           }

browser/components/privatebrowsing/test/browser/browser.toml

@@ -36,7 +36,7 @@ tags = "trackingprotection"
 skip-if = [
   "os == 'mac'",
   "os == 'linux'",
-  "os == 'win' && msix", # We don't support pinning in MSIX builds
+  "os == 'win' && msix", # We don't support private pinning in MSIX builds
 ]
 
 ["browser_privatebrowsing_about_default_promo.js"]

browser/components/shell/ShellService.sys.mjs

@@ -431,6 +431,16 @@ let ShellServiceInternal = {
       return false;
     }
 
+    // Bug 1758770: Pinning private browsing on MSIX is currently
+    // not possible.
+    if (
+      privateBrowsing &&
+      AppConstants.platform === "win" &&
+      Services.sysinfo.getProperty("hasWinPackageId")
+    ) {
+      return false;
+    }
+
     // Currently this only works on certain Windows versions.
     try {
       // First check if we can even pin the app where an exception means no.

browser/components/shell/Windows11TaskbarPinning.cpp

@@ -68,6 +68,27 @@ using namespace ABI::Windows::UI::Shell;
 using namespace ABI::Windows::Foundation;
 using namespace ABI::Windows::ApplicationModel;
 
+static Win11PinToTaskBarResult UnlockLimitedAccessFeature(
+    Win11LimitedAccessFeatureType featureType) {
+  RefPtr<Win11LimitedAccessFeaturesInterface> limitedAccessFeatures =
+      CreateWin11LimitedAccessFeaturesInterface();
+  auto result = limitedAccessFeatures->Unlock(featureType);
+  if (result.isErr()) {
+    auto hr = result.unwrapErr();
+    TASKBAR_PINNING_LOG(LogLevel::Debug,
+                        "Taskbar unlock: Error. HRESULT = 0x%lx", hr);
+    return {hr, Win11PinToTaskBarResultStatus::NotSupported};
+  }
+
+  if (result.unwrap() == false) {
+    TASKBAR_PINNING_LOG(
+        LogLevel::Debug,
+        "Taskbar unlock: failed. Not supported on this version of Windows.");
+    return {S_OK, Win11PinToTaskBarResultStatus::NotSupported};
+  }
+  return {S_OK, Win11PinToTaskBarResultStatus::Success};
+}
+
 static Result<ComPtr<ITaskbarManager>, HRESULT> InitializeTaskbar() {
   ComPtr<IInspectable> taskbarStaticsInspectable;
 
@@ -105,6 +126,55 @@ static Result<ComPtr<ITaskbarManager>, HRESULT> InitializeTaskbar() {
   return taskbarManager;
 }
 
+static Win11PinToTaskBarResultStatus IsTaskbarPinningAllowed(
+    bool aCheckOnly, ComPtr<ITaskbarManager>& taskbar) {
+  HRESULT hr;
+  auto result = InitializeTaskbar();
+  if (result.isErr()) {
+    hr = result.unwrapErr();
+    return Win11PinToTaskBarResultStatus::NotSupported;
+  }
+
+  taskbar = result.unwrap();
+  boolean supported;
+  hr = taskbar->get_IsSupported(&supported);
+  if (FAILED(hr) || !supported) {
+    if (FAILED(hr)) {
+      TASKBAR_PINNING_LOG(
+          LogLevel::Debug,
+          "Taskbar: error checking if supported. HRESULT = 0x%lx", hr);
+    } else {
+      TASKBAR_PINNING_LOG(LogLevel::Debug, "Taskbar: not supported.");
+    }
+    return Win11PinToTaskBarResultStatus::NotSupported;
+  }
+
+  if (aCheckOnly) {
+    TASKBAR_PINNING_LOG(LogLevel::Debug, "Taskbar: check succeeded.");
+    return Win11PinToTaskBarResultStatus::Success;
+  }
+
+  boolean isAllowed = false;
+  hr = taskbar->get_IsPinningAllowed(&isAllowed);
+  if (FAILED(hr) || !isAllowed) {
+    if (FAILED(hr)) {
+      TASKBAR_PINNING_LOG(
+          LogLevel::Debug,
+          "Taskbar: error checking if pinning is allowed. HRESULT = "
+          "0x%lx",
+          hr);
+    } else {
+      TASKBAR_PINNING_LOG(
+          LogLevel::Debug,
+          "Taskbar: is pinning allowed error or isn't allowed right now. "
+          "It's not clear when it will be allowed. Possibly after a "
+          "reboot.");
+    }
+    return Win11PinToTaskBarResultStatus::NotCurrentlyAllowed;
+  }
+  return Win11PinToTaskBarResultStatus::Success;
+}
+
 Win11PinToTaskBarResult PinCurrentAppToTaskbarWin11(
     bool aCheckOnly, const nsAString& aAppUserModelId) {
   MOZ_DIAGNOSTIC_ASSERT(!NS_IsMainThread(),
@@ -112,24 +182,10 @@ Win11PinToTaskBarResult PinCurrentAppToTaskbarWin11(
                         "thread only. It blocks, waiting on things to execute "
                         "asynchronously on the main thread.");
 
-  {
+  Win11PinToTaskBarResult unlockStatus =
-    RefPtr<Win11LimitedAccessFeaturesInterface> limitedAccessFeatures =
+      UnlockLimitedAccessFeature(Win11LimitedAccessFeatureType::Taskbar);
-        CreateWin11LimitedAccessFeaturesInterface();
+  if (unlockStatus.result != Win11PinToTaskBarResultStatus::Success) {
-    auto result =
+    return unlockStatus;
-        limitedAccessFeatures->Unlock(Win11LimitedAccessFeatureType::Taskbar);
-    if (result.isErr()) {
-      auto hr = result.unwrapErr();
-      TASKBAR_PINNING_LOG(LogLevel::Debug,
-                          "Taskbar unlock: Error. HRESULT = 0x%lx", hr);
-      return {hr, Win11PinToTaskBarResultStatus::NotSupported};
-    }
-
-    if (result.unwrap() == false) {
-      TASKBAR_PINNING_LOG(
-          LogLevel::Debug,
-          "Taskbar unlock: failed. Not supported on this version of Windows.");
-      return {S_OK, Win11PinToTaskBarResultStatus::NotSupported};
-    }
   }
 
   HRESULT hr;
@@ -174,51 +230,12 @@ Win11PinToTaskBarResult PinCurrentAppToTaskbarWin11(
           return CompletedOperations(Win11PinToTaskBarResultStatus::Failed);
         }
 
-        auto result = InitializeTaskbar();
+        ComPtr<ITaskbarManager> taskbar;
-        if (result.isErr()) {
+        Win11PinToTaskBarResultStatus allowed =
-          hr = result.unwrapErr();
+            IsTaskbarPinningAllowed(aCheckOnly, taskbar);
-          return CompletedOperations(
+        if ((aCheckOnly && allowed == Win11PinToTaskBarResultStatus::Success) ||
-              Win11PinToTaskBarResultStatus::NotSupported);
+            allowed != Win11PinToTaskBarResultStatus::Success) {
-        }
+          return CompletedOperations(allowed);
-
-        ComPtr<ITaskbarManager> taskbar = result.unwrap();
-        boolean supported;
-        hr = taskbar->get_IsSupported(&supported);
-        if (FAILED(hr) || !supported) {
-          if (FAILED(hr)) {
-            TASKBAR_PINNING_LOG(
-                LogLevel::Debug,
-                "Taskbar: error checking if supported. HRESULT = 0x%lx", hr);
-          } else {
-            TASKBAR_PINNING_LOG(LogLevel::Debug, "Taskbar: not supported.");
-          }
-          return CompletedOperations(
-              Win11PinToTaskBarResultStatus::NotSupported);
-        }
-
-        if (aCheckOnly) {
-          TASKBAR_PINNING_LOG(LogLevel::Debug, "Taskbar: check succeeded.");
-          return CompletedOperations(Win11PinToTaskBarResultStatus::Success);
-        }
-
-        boolean isAllowed = false;
-        hr = taskbar->get_IsPinningAllowed(&isAllowed);
-        if (FAILED(hr) || !isAllowed) {
-          if (FAILED(hr)) {
-            TASKBAR_PINNING_LOG(
-                LogLevel::Debug,
-                "Taskbar: error checking if pinning is allowed. HRESULT = "
-                "0x%lx",
-                hr);
-          } else {
-            TASKBAR_PINNING_LOG(
-                LogLevel::Debug,
-                "Taskbar: is pinning allowed error or isn't allowed right now. "
-                "It's not clear when it will be allowed. Possibly after a "
-                "reboot.");
-          }
-          return CompletedOperations(
-              Win11PinToTaskBarResultStatus::NotCurrentlyAllowed);
         }
 
         ComPtr<IAsyncOperation<bool>> isPinnedOperation = nullptr;
@@ -373,6 +390,123 @@ Win11PinToTaskBarResult PinCurrentAppToTaskbarWin11(
   return {hr, resultStatus};
 }
 
+Win11PinToTaskBarResult IsCurrentAppPinnedToTaskbarWin11(bool aCheckOnly) {
+  MOZ_DIAGNOSTIC_ASSERT(
+      !NS_IsMainThread(),
+      "IsCurrentAppPinnedToTaskbarWin11 should be called off main "
+      "thread only. It blocks, waiting on things to execute "
+      "asynchronously on the main thread.");
+
+  Win11PinToTaskBarResult unlockStatus =
+      UnlockLimitedAccessFeature(Win11LimitedAccessFeatureType::Taskbar);
+  if (unlockStatus.result != Win11PinToTaskBarResultStatus::Success) {
+    return unlockStatus;
+  }
+
+  HRESULT hr;
+  Win11PinToTaskBarResultStatus resultStatus =
+      Win11PinToTaskBarResultStatus::NotSupported;
+
+  EventWrapper event;
+
+  // 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(
+      "IsCurrentAppPinnedToTaskbarWin11",
+      [&event, &hr, &resultStatus, aCheckOnly] {
+        auto CompletedOperations =
+            [&event, &resultStatus](Win11PinToTaskBarResultStatus status) {
+              resultStatus = status;
+              event.Set();
+            };
+
+        ComPtr<ITaskbarManager> taskbar;
+        Win11PinToTaskBarResultStatus allowed =
+            IsTaskbarPinningAllowed(aCheckOnly, taskbar);
+        if ((aCheckOnly && allowed == Win11PinToTaskBarResultStatus::Success) ||
+            allowed != Win11PinToTaskBarResultStatus::Success) {
+          return CompletedOperations(allowed);
+        }
+
+        ComPtr<IAsyncOperation<bool>> isPinnedOperation = nullptr;
+        hr = taskbar->IsCurrentAppPinnedAsync(&isPinnedOperation);
+        if (FAILED(hr)) {
+          TASKBAR_PINNING_LOG(
+              LogLevel::Debug,
+              "Taskbar: is current app pinned operation failed. HRESULT = "
+              "0x%lx",
+              hr);
+          return CompletedOperations(Win11PinToTaskBarResultStatus::Failed);
+        }
+
+        // Copy the taskbar; don't use it as a reference.
+        // With the async calls, it's not guaranteed to still be valid
+        // if sent as a reference.
+        // resultStatus and event are not defined on the main thread and will
+        // be alive until the async functions complete, so they can be used as
+        // references.
+        auto isPinnedCallback = Callback<IAsyncOperationCompletedHandler<
+            bool>>([taskbar, &event, &resultStatus, &hr](
+                       IAsyncOperation<bool>* asyncInfo,
+                       AsyncStatus status) mutable -> HRESULT {
+          auto CompletedOperations =
+              [&event,
+               &resultStatus](Win11PinToTaskBarResultStatus status) -> HRESULT {
+            resultStatus = status;
+            event.Set();
+            return S_OK;
+          };
+
+          bool asyncOpSucceeded = status == AsyncStatus::Completed;
+          if (!asyncOpSucceeded) {
+            TASKBAR_PINNING_LOG(
+                LogLevel::Debug,
+                "Taskbar: is pinned operation failed to complete.");
+            return CompletedOperations(Win11PinToTaskBarResultStatus::Failed);
+          }
+
+          unsigned char isCurrentAppPinned = false;
+          hr = asyncInfo->GetResults(&isCurrentAppPinned);
+          if (FAILED(hr)) {
+            TASKBAR_PINNING_LOG(
+                LogLevel::Debug,
+                "Taskbar: is current app pinned check failed. HRESULT = 0x%lx",
+                hr);
+            return CompletedOperations(Win11PinToTaskBarResultStatus::Failed);
+          }
+
+          if (isCurrentAppPinned) {
+            TASKBAR_PINNING_LOG(LogLevel::Debug,
+                                "Taskbar: current app is already pinned.");
+            return CompletedOperations(
+                Win11PinToTaskBarResultStatus::AlreadyPinned);
+          }
+          return CompletedOperations(Win11PinToTaskBarResultStatus::NotPinned);
+        });
+
+        HRESULT isPinnedOperationHR =
+            isPinnedOperation->put_Completed(isPinnedCallback.Get());
+        if (FAILED(isPinnedOperationHR)) {
+          hr = isPinnedOperationHR;
+          TASKBAR_PINNING_LOG(
+              LogLevel::Debug,
+              "Taskbar: is pinned operation failed when setting completion "
+              "callback. HRESULT = 0x%lx",
+              hr);
+          return CompletedOperations(Win11PinToTaskBarResultStatus::Failed);
+        }
+
+        // DO NOT SET event HERE. It will be set in the is pin operation
+        // callback As in, operations are not completed, so don't call
+        // CompletedOperations
+      }));
+
+  // block until the pinning check is completed on the main thread
+  event.Wait();
+
+  return {hr, resultStatus};
+}
+
 #else  // MINGW32 implementation below
 
 Win11PinToTaskBarResult PinCurrentAppToTaskbarWin11(
@@ -380,4 +514,8 @@ Win11PinToTaskBarResult PinCurrentAppToTaskbarWin11(
   return {S_OK, Win11PinToTaskBarResultStatus::NotSupported};
 }
 
+Win11PinToTaskBarResult IsCurrentAppPinnedToTaskbarWin11(bool aCheckOnly) {
+  return {S_OK, Win11PinToTaskBarResultStatus::NotSupported};
+}
+
 #endif  // #ifndef __MINGW32__  // WinRT headers not yet supported by MinGW

browser/components/shell/Windows11TaskbarPinning.h

@@ -19,6 +19,7 @@ enum class Win11PinToTaskBarResultStatus {
   Failed,
   NotCurrentlyAllowed,
   AlreadyPinned,
+  NotPinned,
   Success,
   NotSupported,
 };
@@ -31,4 +32,6 @@ struct Win11PinToTaskBarResult {
 Win11PinToTaskBarResult PinCurrentAppToTaskbarWin11(
     bool aCheckOnly, const nsAString& aAppUserModelId);
 
+Win11PinToTaskBarResult IsCurrentAppPinnedToTaskbarWin11(bool aCheckOnly);
+
 #endif  // SHELL_WINDOWS11TASKBARPINNING_H__

browser/components/shell/nsWindowsShellService.cpp

@@ -1284,6 +1284,33 @@ NS_IMETHODIMP nsWindowsShellService::HasMatchingShortcut(
 }
 
 static bool IsCurrentAppPinnedToTaskbarSync(const nsAString& aumid) {
+  // Use new Windows pinning APIs to determine whether or not we're pinned.
+  // If these fail we can safely fall back to the old method for regular
+  // installs however MSIX will always return false.
+
+  // Bug 1911343: Add a check for whether we're looking for a regular pin
+  // or PB pin based on the AUMID value once private browser pinning
+  // is supported on MSIX.
+  // Right now only run this check on MSIX to avoid
+  // false positives when only private browsing is pinned.
+  if (widget::WinUtils::HasPackageIdentity()) {
+    auto pinWithWin11TaskbarAPIResults =
+        IsCurrentAppPinnedToTaskbarWin11(false);
+    switch (pinWithWin11TaskbarAPIResults.result) {
+      case Win11PinToTaskBarResultStatus::NotPinned:
+        return false;
+        break;
+      case Win11PinToTaskBarResultStatus::AlreadyPinned:
+        return true;
+        break;
+      default:
+        // Fall through to the old mechanism.
+        // The old mechanism should continue working for non-MSIX
+        // builds.
+        break;
+    }
+  }
+
   // There are two shortcut targets that we created. One always matches the
   // binary we're running as (eg: firefox.exe). The other is the wrapper
   // for launching in Private Browsing mode. We need to inspect shortcuts
@@ -1740,6 +1767,7 @@ static nsresult PinCurrentAppToTaskbarImpl(
     case Win11PinToTaskBarResultStatus::AlreadyPinned:
       return NS_OK;
 
+    case Win11PinToTaskBarResultStatus::NotPinned:
     case Win11PinToTaskBarResultStatus::NotCurrentlyAllowed:
     case Win11PinToTaskBarResultStatus::Failed:
       // return NS_ERROR_FAILURE;
@@ -1864,12 +1892,6 @@ NS_IMETHODIMP
 nsWindowsShellService::PinCurrentAppToTaskbarAsync(bool aPrivateBrowsing,
                                                    JSContext* aCx,
                                                    dom::Promise** aPromise) {
-  // https://bugzilla.mozilla.org/show_bug.cgi?id=1712628 tracks implementing
-  // this for MSIX packages.
-  if (widget::WinUtils::HasPackageIdentity()) {
-    return NS_ERROR_NOT_IMPLEMENTED;
-  }
-
   return PinCurrentAppToTaskbarAsyncImpl(
       /* aCheckOnly */ false, aPrivateBrowsing, aCx, aPromise);
 }
@@ -1877,12 +1899,6 @@ nsWindowsShellService::PinCurrentAppToTaskbarAsync(bool aPrivateBrowsing,
 NS_IMETHODIMP
 nsWindowsShellService::CheckPinCurrentAppToTaskbarAsync(
     bool aPrivateBrowsing, JSContext* aCx, dom::Promise** aPromise) {
-  // https://bugzilla.mozilla.org/show_bug.cgi?id=1712628 tracks implementing
-  // this for MSIX packages.
-  if (widget::WinUtils::HasPackageIdentity()) {
-    return NS_ERROR_NOT_IMPLEMENTED;
-  }
-
   return PinCurrentAppToTaskbarAsyncImpl(
       /* aCheckOnly = */ true, aPrivateBrowsing, aCx, aPromise);
 }
@@ -1890,12 +1906,6 @@ nsWindowsShellService::CheckPinCurrentAppToTaskbarAsync(
 NS_IMETHODIMP
 nsWindowsShellService::IsCurrentAppPinnedToTaskbarAsync(
     const nsAString& aumid, JSContext* aCx, /* out */ dom::Promise** aPromise) {
-  // https://bugzilla.mozilla.org/show_bug.cgi?id=1712628 tracks implementing
-  // this for MSIX packages.
-  if (widget::WinUtils::HasPackageIdentity()) {
-    return NS_ERROR_NOT_IMPLEMENTED;
-  }
-
   if (!NS_IsMainThread()) {
     return NS_ERROR_NOT_SAME_THREAD;
   }

browser/components/tests/browser/browser_browserGlue_telemetry.js

@@ -14,10 +14,16 @@ add_task(function check_startup_pinned_telemetry() {
         AppConstants.platform === "win" &&
         Services.sysinfo.getProperty("hasWinPackageId")
       ) {
-        TelemetryTestUtils.assertScalarUnset(
+        TelemetryTestUtils.assertScalar(
           scalars,
-          "os.environment.is_taskbar_pinned"
+          "os.environment.is_taskbar_pinned",
+          false,
+          "Pin set on win MSIX"
         );
+        // Bug 1911343: Pinning regular browsing on MSIX
+        // causes false positives when checking for private
+        // browsing. As a result no telemetry is logged regarding
+        // private pin status.
         TelemetryTestUtils.assertScalarUnset(
           scalars,
           "os.environment.is_taskbar_pinned_private"

toolkit/components/messaging-system/schemas/SpecialMessageActionSchemas/test/browser/browser.toml

@@ -44,7 +44,7 @@ support-files = [
 ["browser_sma_pin_firefox.js"]
 
 ["browser_sma_pin_private_firefox.js"]
-skip-if = ["os != 'win'"]
+run-if = ["(os == 'win' && !msix)"] # Bug 1911343
 
 ["browser_sma_pin_start_menu.js"]
 run-if = ["(os == 'win' && msix)"]