Bug 1893458 - attempt to fix tab pinning not working sometimes on the first time r=nalexander,nrishel

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

browser/components/shell/Windows11TaskbarPinning.cpp

@@ -100,6 +100,18 @@ static Result<ComPtr<ITaskbarManager>, HRESULT> InitializeTaskbar() {
   return taskbarManager;
 }
 
+static bool IsStatusToRetry(Win11PinToTaskBarResultStatus status) {
+  switch (status) {
+    case Win11PinToTaskBarResultStatus::NotCurrentlyAllowed:
+    case Win11PinToTaskBarResultStatus::LimitedAccessFeaturesLocked:
+    case Win11PinToTaskBarResultStatus::ErrorLimitedAccessFeatures:
+      return true;
+
+    default:
+      return false;
+  }
+}
+
 Win11PinToTaskBarResult PinCurrentAppToTaskbarWin11(
     bool aCheckOnly, const nsAString& aAppUserModelId,
     nsAutoString aShortcutPath) {
@@ -108,229 +120,257 @@ Win11PinToTaskBarResult PinCurrentAppToTaskbarWin11(
                         "thread only. It blocks, waiting on things to execute "
                         "asynchronously on the main thread.");
 
-  {
-    RefPtr<Win11LimitedAccessFeaturesInterface> limitedAccessFeatures =
-        CreateWin11LimitedAccessFeaturesInterface();
-    auto result =
-        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;
   Win11PinToTaskBarResultStatus resultStatus =
       Win11PinToTaskBarResultStatus::NotSupported;
 
-  EventWrapper event;
+  // Pinning with the Win 11 APIs sometimes reports that pinning is not
+  // available. There is no documentation on what causes it to not be available
+  // or why. One way to work around that is to try again. We try 3 times if it's
+  // not currently available.
+  const int maxNumberOfRetryAttempts = 3;
+  int delayInMilliseconds = 200;
+  int count = 0;
+  do {
+    EventWrapper event;
 
-  // Everything related to the taskbar and pinning must be done on the main /
+    // Everything related to the taskbar and pinning must be done on the main /
-  // user interface thread or Windows will cause them to fail.
+    // user interface thread or Windows will cause them to fail.
-  NS_DispatchToMainThread(NS_NewRunnableFunction(
+    NS_DispatchToMainThread(NS_NewRunnableFunction(
-      "PinCurrentAppToTaskbarWin11", [&event, &hr, &resultStatus, aCheckOnly] {
+        "PinCurrentAppToTaskbarWin11",
-        auto CompletedOperations =
+        [&event, &count, &hr, &resultStatus, aCheckOnly] {
-            [&event, &resultStatus](Win11PinToTaskBarResultStatus status) {
+          auto CompletedOperations =
-              resultStatus = status;
+              [&event, &resultStatus](Win11PinToTaskBarResultStatus status) {
-              event.Set();
+                resultStatus = status;
-            };
+                event.Set();
+              };
 
-        auto result = InitializeTaskbar();
+          {
-        if (result.isErr()) {
+            RefPtr<Win11LimitedAccessFeaturesInterface> limitedAccessFeatures =
-          hr = result.unwrapErr();
+                CreateWin11LimitedAccessFeaturesInterface();
-          return CompletedOperations(
+            auto result = limitedAccessFeatures->Unlock(
-              Win11PinToTaskBarResultStatus::NotSupported);
+                Win11LimitedAccessFeatureType::Taskbar);
-        }
+            if (result.isErr()) {
+              hr = result.unwrapErr();
+              TASKBAR_PINNING_LOG(LogLevel::Debug,
+                                  "Taskbar unlock: Error. HRESULT = 0x%lx", hr);
+              return CompletedOperations(
+                  Win11PinToTaskBarResultStatus::ErrorLimitedAccessFeatures);
+            }
 
-        ComPtr<ITaskbarManager> taskbar = result.unwrap();
+            if (result.unwrap() == false) {
-        boolean supported;
+              TASKBAR_PINNING_LOG(LogLevel::Debug,
-        hr = taskbar->get_IsSupported(&supported);
+                                  "Taskbar unlock: failed. Not supported on "
-        if (FAILED(hr) || !supported) {
+                                  "this version of Windows.");
-          if (FAILED(hr)) {
+              return CompletedOperations(
-            TASKBAR_PINNING_LOG(
+                  Win11PinToTaskBarResultStatus::LimitedAccessFeaturesLocked);
-                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) {
+          auto result = InitializeTaskbar();
-          TASKBAR_PINNING_LOG(LogLevel::Debug, "Taskbar: check succeeded.");
+          if (result.isErr()) {
-          return CompletedOperations(Win11PinToTaskBarResultStatus::Success);
+            hr = result.unwrapErr();
-        }
+            return CompletedOperations(
+                Win11PinToTaskBarResultStatus::NotSupported);
+          }
 
-        boolean isAllowed = false;
+          ComPtr<ITaskbarManager> taskbar = result.unwrap();
-        hr = taskbar->get_IsPinningAllowed(&isAllowed);
+          boolean supported;
-        if (FAILED(hr) || !isAllowed) {
+          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;
+          hr = taskbar->IsCurrentAppPinnedAsync(&isPinnedOperation);
           if (FAILED(hr)) {
             TASKBAR_PINNING_LOG(
                 LogLevel::Debug,
-                "Taskbar: error checking if pinning is allowed. HRESULT = "
+                "Taskbar: is current app pinned operation failed. 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;
-        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;
+          // Copy the taskbar; don't use it as a reference.
-          hr = asyncInfo->GetResults(&isCurrentAppPinned);
+          // With the async calls, it's not guaranteed to still be valid
-          if (FAILED(hr)) {
+          // if sent as a reference.
-            TASKBAR_PINNING_LOG(
+          // resultStatus and event are not defined on the main thread and will
-                LogLevel::Debug,
+          // be alive until the async functions complete, so they can be used as
-                "Taskbar: is current app pinned check failed. HRESULT = 0x%lx",
+          // references.
-                hr);
+          auto isPinnedCallback = Callback<IAsyncOperationCompletedHandler<
-            return CompletedOperations(Win11PinToTaskBarResultStatus::Failed);
+              bool>>([taskbar, &event, &resultStatus, &hr](
-          }
-
-          if (isCurrentAppPinned) {
-            TASKBAR_PINNING_LOG(LogLevel::Debug,
-                                "Taskbar: current app is already pinned.");
-            return CompletedOperations(
-                Win11PinToTaskBarResultStatus::AlreadyPinned);
-          }
-
-          ComPtr<IAsyncOperation<bool>> requestPinOperation = nullptr;
-          hr = taskbar->RequestPinCurrentAppAsync(&requestPinOperation);
-          if (FAILED(hr)) {
-            TASKBAR_PINNING_LOG(
-                LogLevel::Debug,
-                "Taskbar: request pin current app operation creation failed. "
-                "HRESULT = 0x%lx",
-                hr);
-            return CompletedOperations(Win11PinToTaskBarResultStatus::Failed);
-          }
-
-          auto pinAppCallback = Callback<IAsyncOperationCompletedHandler<
-              bool>>([CompletedOperations, &hr](
                          IAsyncOperation<bool>* asyncInfo,
-                         AsyncStatus status) -> HRESULT {
+                         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: request pin current app operation did not "
+                  "Taskbar: is pinned operation failed to complete.");
-                  "complete.");
               return CompletedOperations(Win11PinToTaskBarResultStatus::Failed);
             }
 
-            unsigned char successfullyPinned = 0;
+            unsigned char isCurrentAppPinned = false;
-            hr = asyncInfo->GetResults(&successfullyPinned);
+            hr = asyncInfo->GetResults(&isCurrentAppPinned);
-            if (FAILED(hr) || !successfullyPinned) {
+            if (FAILED(hr)) {
-              if (FAILED(hr)) {
+              TASKBAR_PINNING_LOG(LogLevel::Debug,
-                TASKBAR_PINNING_LOG(
+                                  "Taskbar: is current app pinned check "
-                    LogLevel::Debug,
+                                  "failed. HRESULT = 0x%lx",
-                    "Taskbar: request pin current app operation failed to pin "
+                                  hr);
-                    "due to error. HRESULT = 0x%lx",
-                    hr);
-              } else {
-                TASKBAR_PINNING_LOG(
-                    LogLevel::Debug,
-                    "Taskbar: request pin current app operation failed to pin");
-              }
               return CompletedOperations(Win11PinToTaskBarResultStatus::Failed);
             }
 
-            TASKBAR_PINNING_LOG(
+            if (isCurrentAppPinned) {
-                LogLevel::Debug,
+              TASKBAR_PINNING_LOG(LogLevel::Debug,
-                "Taskbar: request pin current app operation succeeded");
+                                  "Taskbar: current app is already pinned.");
-            return CompletedOperations(Win11PinToTaskBarResultStatus::Success);
+              return CompletedOperations(
+                  Win11PinToTaskBarResultStatus::AlreadyPinned);
+            }
+
+            ComPtr<IAsyncOperation<bool>> requestPinOperation = nullptr;
+            hr = taskbar->RequestPinCurrentAppAsync(&requestPinOperation);
+            if (FAILED(hr)) {
+              TASKBAR_PINNING_LOG(
+                  LogLevel::Debug,
+                  "Taskbar: request pin current app operation creation failed. "
+                  "HRESULT = 0x%lx",
+                  hr);
+              return CompletedOperations(Win11PinToTaskBarResultStatus::Failed);
+            }
+
+            auto pinAppCallback = Callback<
+                IAsyncOperationCompletedHandler<bool>>(
+                [CompletedOperations, &hr](IAsyncOperation<bool>* asyncInfo,
+                                           AsyncStatus status) -> HRESULT {
+                  bool asyncOpSucceeded = status == AsyncStatus::Completed;
+                  if (!asyncOpSucceeded) {
+                    TASKBAR_PINNING_LOG(
+                        LogLevel::Debug,
+                        "Taskbar: request pin current app operation did not "
+                        "complete.");
+                    return CompletedOperations(
+                        Win11PinToTaskBarResultStatus::Failed);
+                  }
+
+                  unsigned char successfullyPinned = 0;
+                  hr = asyncInfo->GetResults(&successfullyPinned);
+                  if (FAILED(hr) || !successfullyPinned) {
+                    if (FAILED(hr)) {
+                      TASKBAR_PINNING_LOG(LogLevel::Debug,
+                                          "Taskbar: request pin current app "
+                                          "operation failed to pin "
+                                          "due to error. HRESULT = 0x%lx",
+                                          hr);
+                    } else {
+                      TASKBAR_PINNING_LOG(LogLevel::Debug,
+                                          "Taskbar: request pin current app "
+                                          "operation failed to pin");
+                    }
+                    return CompletedOperations(
+                        Win11PinToTaskBarResultStatus::Failed);
+                  }
+
+                  TASKBAR_PINNING_LOG(
+                      LogLevel::Debug,
+                      "Taskbar: request pin current app operation succeeded");
+                  return CompletedOperations(
+                      Win11PinToTaskBarResultStatus::Success);
+                });
+
+            HRESULT pinOperationHR =
+                requestPinOperation->put_Completed(pinAppCallback.Get());
+            if (FAILED(pinOperationHR)) {
+              TASKBAR_PINNING_LOG(LogLevel::Debug,
+                                  "Taskbar: request pin operation failed when "
+                                  "setting completion "
+                                  "callback. HRESULT = 0x%lx",
+                                  hr);
+              hr = pinOperationHR;
+              return CompletedOperations(Win11PinToTaskBarResultStatus::Failed);
+            }
+
+            // DO NOT SET event HERE. It will be set in the pin operation
+            // callback As in, operations are not completed, so don't call
+            // CompletedOperations
+            return S_OK;
           });
 
-          HRESULT pinOperationHR =
+          HRESULT isPinnedOperationHR =
-              requestPinOperation->put_Completed(pinAppCallback.Get());
+              isPinnedOperation->put_Completed(isPinnedCallback.Get());
-          if (FAILED(pinOperationHR)) {
+          if (FAILED(isPinnedOperationHR)) {
+            hr = isPinnedOperationHR;
             TASKBAR_PINNING_LOG(
                 LogLevel::Debug,
-                "Taskbar: request pin operation failed when setting completion "
+                "Taskbar: is pinned operation failed when setting completion "
                 "callback. HRESULT = 0x%lx",
                 hr);
-            hr = pinOperationHR;
             return CompletedOperations(Win11PinToTaskBarResultStatus::Failed);
           }
 
-          // DO NOT SET event HERE. It will be set in the pin operation
+          // 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
+          // callback. As in, operations are not completed, so don't call
           // CompletedOperations
-          return S_OK;
+        }));
-        });
 
-        HRESULT isPinnedOperationHR =
+    // block until the pinning is completed on the main thread
-            isPinnedOperation->put_Completed(isPinnedCallback.Get());
+    event.Wait();
-        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
+    count++;
-        // callback As in, operations are not completed, so don't call
+    if (!IsStatusToRetry(resultStatus) || (count >= maxNumberOfRetryAttempts)) {
-        // CompletedOperations
+      break;
-      }));
+    }
 
-  // block until the pinning is completed on the main thread
+    // If retrying, put in a delay. Make it short the first time and slightly
-  event.Wait();
+    // longer the second time. This is not on the main thread so shouldn't
+    // forcefully block the user. If called from Javascript with an await it
+    // will still block, but that's a decision on the Javascript side
+    Sleep(delayInMilliseconds);
+    delayInMilliseconds *= 2;
+  } while (count < maxNumberOfRetryAttempts);
 
-  return {hr, resultStatus};
+  return {hr, resultStatus, count};
 }
 
 #else  // MINGW32 implementation below

browser/components/shell/Windows11TaskbarPinning.h

@@ -21,11 +21,14 @@ enum class Win11PinToTaskBarResultStatus {
   AlreadyPinned,
   Success,
   NotSupported,
+  ErrorLimitedAccessFeatures,
+  LimitedAccessFeaturesLocked,
 };
 
 struct Win11PinToTaskBarResult {
   HRESULT errorCode;
   Win11PinToTaskBarResultStatus result;
+  int numAttempts;
 };
 
 Win11PinToTaskBarResult PinCurrentAppToTaskbarWin11(

browser/components/shell/nsWindowsShellService.cpp

@@ -1707,6 +1707,8 @@ static nsresult PinCurrentAppToTaskbarImpl(
     case Win11PinToTaskBarResultStatus::AlreadyPinned:
       return NS_OK;
 
+    case Win11PinToTaskBarResultStatus::ErrorLimitedAccessFeatures:
+    case Win11PinToTaskBarResultStatus::LimitedAccessFeaturesLocked:
     case Win11PinToTaskBarResultStatus::NotCurrentlyAllowed:
     case Win11PinToTaskBarResultStatus::Failed:
       // return NS_ERROR_FAILURE;