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,42 +120,53 @@ 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;
 
+  // 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 /
     // user interface thread or Windows will cause them to fail.
     NS_DispatchToMainThread(NS_NewRunnableFunction(
-      "PinCurrentAppToTaskbarWin11", [&event, &hr, &resultStatus, aCheckOnly] {
+        "PinCurrentAppToTaskbarWin11",
+        [&event, &count, &hr, &resultStatus, aCheckOnly] {
           auto CompletedOperations =
               [&event, &resultStatus](Win11PinToTaskBarResultStatus status) {
                 resultStatus = status;
                 event.Set();
               };
 
+          {
+            RefPtr<Win11LimitedAccessFeaturesInterface> limitedAccessFeatures =
+                CreateWin11LimitedAccessFeaturesInterface();
+            auto result = limitedAccessFeatures->Unlock(
+                Win11LimitedAccessFeatureType::Taskbar);
+            if (result.isErr()) {
+              hr = result.unwrapErr();
+              TASKBAR_PINNING_LOG(LogLevel::Debug,
+                                  "Taskbar unlock: Error. HRESULT = 0x%lx", hr);
+              return CompletedOperations(
+                  Win11PinToTaskBarResultStatus::ErrorLimitedAccessFeatures);
+            }
+
+            if (result.unwrap() == false) {
+              TASKBAR_PINNING_LOG(LogLevel::Debug,
+                                  "Taskbar unlock: failed. Not supported on "
+                                  "this version of Windows.");
+              return CompletedOperations(
+                  Win11PinToTaskBarResultStatus::LimitedAccessFeaturesLocked);
+            }
+          }
+
           auto result = InitializeTaskbar();
           if (result.isErr()) {
             hr = result.unwrapErr();
@@ -183,7 +206,8 @@ Win11PinToTaskBarResult PinCurrentAppToTaskbarWin11(
             } else {
               TASKBAR_PINNING_LOG(
                   LogLevel::Debug,
-                "Taskbar: is pinning allowed error or isn't allowed right now. "
+                  "Taskbar: is pinning allowed error or isn't allowed right "
+                  "now. "
                   "It's not clear when it will be allowed. Possibly after a "
                   "reboot.");
             }
@@ -213,8 +237,8 @@ Win11PinToTaskBarResult PinCurrentAppToTaskbarWin11(
                          IAsyncOperation<bool>* asyncInfo,
                          AsyncStatus status) mutable -> HRESULT {
             auto CompletedOperations =
-              [&event,
+                [&event, &resultStatus](
-               &resultStatus](Win11PinToTaskBarResultStatus status) -> HRESULT {
+                    Win11PinToTaskBarResultStatus status) -> HRESULT {
               resultStatus = status;
               event.Set();
               return S_OK;
@@ -231,9 +255,9 @@ Win11PinToTaskBarResult PinCurrentAppToTaskbarWin11(
             unsigned char isCurrentAppPinned = false;
             hr = asyncInfo->GetResults(&isCurrentAppPinned);
             if (FAILED(hr)) {
-            TASKBAR_PINNING_LOG(
+              TASKBAR_PINNING_LOG(LogLevel::Debug,
-                LogLevel::Debug,
+                                  "Taskbar: is current app pinned check "
-                "Taskbar: is current app pinned check failed. HRESULT = 0x%lx",
+                                  "failed. HRESULT = 0x%lx",
                                   hr);
               return CompletedOperations(Win11PinToTaskBarResultStatus::Failed);
             }
@@ -256,9 +280,9 @@ Win11PinToTaskBarResult PinCurrentAppToTaskbarWin11(
               return CompletedOperations(Win11PinToTaskBarResultStatus::Failed);
             }
 
-          auto pinAppCallback = Callback<IAsyncOperationCompletedHandler<
+            auto pinAppCallback = Callback<
-              bool>>([CompletedOperations, &hr](
+                IAsyncOperationCompletedHandler<bool>>(
-                         IAsyncOperation<bool>* asyncInfo,
+                [CompletedOperations, &hr](IAsyncOperation<bool>* asyncInfo,
                                            AsyncStatus status) -> HRESULT {
                   bool asyncOpSucceeded = status == AsyncStatus::Completed;
                   if (!asyncOpSucceeded) {
@@ -266,38 +290,41 @@ Win11PinToTaskBarResult PinCurrentAppToTaskbarWin11(
                         LogLevel::Debug,
                         "Taskbar: request pin current app operation did not "
                         "complete.");
-              return CompletedOperations(Win11PinToTaskBarResultStatus::Failed);
+                    return CompletedOperations(
+                        Win11PinToTaskBarResultStatus::Failed);
                   }
 
                   unsigned char successfullyPinned = 0;
                   hr = asyncInfo->GetResults(&successfullyPinned);
                   if (FAILED(hr) || !successfullyPinned) {
                     if (FAILED(hr)) {
-                TASKBAR_PINNING_LOG(
+                      TASKBAR_PINNING_LOG(LogLevel::Debug,
-                    LogLevel::Debug,
+                                          "Taskbar: request pin current app "
-                    "Taskbar: request pin current app operation failed to pin "
+                                          "operation failed to pin "
                                           "due to error. HRESULT = 0x%lx",
                                           hr);
                     } else {
-                TASKBAR_PINNING_LOG(
+                      TASKBAR_PINNING_LOG(LogLevel::Debug,
-                    LogLevel::Debug,
+                                          "Taskbar: request pin current app "
-                    "Taskbar: request pin current app operation failed to pin");
+                                          "operation failed to pin");
                     }
-              return CompletedOperations(Win11PinToTaskBarResultStatus::Failed);
+                    return CompletedOperations(
+                        Win11PinToTaskBarResultStatus::Failed);
                   }
 
                   TASKBAR_PINNING_LOG(
                       LogLevel::Debug,
                       "Taskbar: request pin current app operation succeeded");
-            return CompletedOperations(Win11PinToTaskBarResultStatus::Success);
+                  return CompletedOperations(
+                      Win11PinToTaskBarResultStatus::Success);
                 });
 
             HRESULT pinOperationHR =
                 requestPinOperation->put_Completed(pinAppCallback.Get());
             if (FAILED(pinOperationHR)) {
-            TASKBAR_PINNING_LOG(
+              TASKBAR_PINNING_LOG(LogLevel::Debug,
-                LogLevel::Debug,
+                                  "Taskbar: request pin operation failed when "
-                "Taskbar: request pin operation failed when setting completion "
+                                  "setting completion "
                                   "callback. HRESULT = 0x%lx",
                                   hr);
               hr = pinOperationHR;
@@ -323,14 +350,27 @@ Win11PinToTaskBarResult PinCurrentAppToTaskbarWin11(
           }
 
           // 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
         }));
 
     // block until the pinning is completed on the main thread
     event.Wait();
 
-  return {hr, resultStatus};
+    count++;
+    if (!IsStatusToRetry(resultStatus) || (count >= maxNumberOfRetryAttempts)) {
+      break;
+    }
+
+    // If retrying, put in a delay. Make it short the first time and slightly
+    // 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, 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;