Bug 1797678 - Fix Windows native notifications interaction from prior Firefox instances and closed tabs. r=nalexander,Jamie,mossop

We need to ensure we're not in a `WndProc` callback context when interacting with native notification COM objects, which is currently a problem when called from the command line handler (see Bug 1805802). We achieve this by moving COM work onto a background task which defers processing until after `WndProc` returns.

Now that retrieving the fallback URL occurs on a background task waiting for it before returning would block the main thread. Making `handleWindowsTag` async introduced race conditions with commandline handling, requiring a `enterLastWindowClosingSurvivalArea`/`exitLastWindowClosingSurvivalArea` to prevent misbehavior in case early blank window is pref'd off. In order to simplify exit cleanup, the API was updated to return a `Promise`.

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

browser/components/BrowserContentHandler.sys.mjs

@@ -1045,18 +1045,37 @@ nsDefaultCommandLineHandler.prototype = {
     if (AppConstants.platform == "win") {
       // Windows itself does disk I/O when the notification service is
       // initialized, so make sure that is lazy.
-      var tag;
+      while (true) {
-      while (
+        let tag = cmdLine.handleFlagWithParam("notification-windowsTag", false);
-        (tag = cmdLine.handleFlagWithParam("notification-windowsTag", false))
+        if (!tag) {
-      ) {
+          break;
-        let onUnknownWindowsTag = (unknownTag, launchUrl, privilegedName) => {
+        }
+
+        let alertService = lazy.gWindowsAlertsService;
+        if (!alertService) {
+          console.error("Windows alert service not available.");
+          break;
+        }
+
+        async function handleNotification() {
+          const {
+            launchUrl,
+            privilegedName,
+          } = await alertService.handleWindowsTag(tag);
+
+          // If `launchUrl` or `privilegedName` are provided, then the
+          // notification was from a prior instance of the application and we
+          // need to handled fallback behavior.
+          if (launchUrl || privilegedName) {
             console.info(
               `Completing Windows notification (tag=${JSON.stringify(
-              unknownTag
+                tag
               )}, launchUrl=${JSON.stringify(
                 launchUrl
               )}, privilegedName=${JSON.stringify(privilegedName)}))`
             );
+          }
+
           if (privilegedName) {
             Services.telemetry.setEventRecordingEnabled(
               "browser.launched_to_handle",
@@ -1066,28 +1085,55 @@ nsDefaultCommandLineHandler.prototype = {
               name: privilegedName,
             });
           }
-          if (!launchUrl) {
-            return;
-          }
-          let uri = resolveURIInternal(cmdLine, launchUrl);
-          urilist.push(uri);
-        };
 
+          if (launchUrl) {
+            let uri = resolveURIInternal(cmdLine, launchUrl);
+            if (cmdLine.state != Ci.nsICommandLine.STATE_INITIAL_LAUNCH) {
+              // Try to find an existing window and load our URI into the current
+              // tab, new tab, or new window as prefs determine.
               try {
-          if (
+                handURIToExistingBrowser(
-            lazy.gWindowsAlertsService?.handleWindowsTag(
+                  uri,
-              tag,
+                  Ci.nsIBrowserDOMWindow.OPEN_DEFAULTWINDOW,
-              onUnknownWindowsTag
+                  cmdLine,
-            )
+                  false,
-          ) {
+                  lazy.gSystemPrincipal
-            // Don't pop open a new window.
+                );
-            cmdLine.preventDefault = true;
+                return;
+              } catch (e) {}
             }
-        } catch (e) {
+
+            if (shouldLoadURI(uri)) {
+              openBrowserWindow(cmdLine, lazy.gSystemPrincipal, [uri.spec]);
+            }
+          } else if (cmdLine.state == Ci.nsICommandLine.STATE_INITIAL_LAUNCH) {
+            // No URL provided, but notification was interacted with while the
+            // application was closed. Fall back to opening the browser without url.
+            openBrowserWindow(cmdLine, lazy.gSystemPrincipal);
+          }
+        }
+
+        // Notification handling occurs asynchronously to prevent blocking the
+        // main thread. As a result we won't have the information we need to open
+        // a new tab in the case of notification fallback handling before
+        // returning. We call `enterLastWindowClosingSurvivalArea` to prevent
+        // the browser from exiting in case early blank window is pref'd off.
+        if (cmdLine.state == Ci.nsICommandLine.STATE_INITIAL_LAUNCH) {
+          Services.startup.enterLastWindowClosingSurvivalArea();
+        }
+        handleNotification()
+          .catch(e => {
             console.error(
               `Error handling Windows notification with tag '${tag}': ${e}`
             );
+          })
+          .finally(() => {
+            if (cmdLine.state == Ci.nsICommandLine.STATE_INITIAL_LAUNCH) {
+              Services.startup.exitLastWindowClosingSurvivalArea();
             }
+          });
+
+        return;
       }
     }
 

toolkit/components/alerts/nsIWindowsAlertsService.idl

@@ -32,12 +32,21 @@ interface nsIWindowsAlertsService : nsIAlertsService
    * this Firefox process, set the associated event.
    *
    * @param {AString} aWindowsTag the tag
-   * @param {nsIUnhandledWindowsTagListener} aListener the listener to callback
+   * @return {Promise}
-   * if the tag is unknown and has an associated launch URL.
+   * @resolves {Object}
-   * @return {boolean} `true` iff the tag is registered and an event was set.
+   *           Resolves with an Object, may contain the following optional
+   *           properties if notification exists but wasn't registered with
+   *           the WindowsAlertService:
+   *
+   *           `launchUrl` {string} a fallback URL to open.
+   *
+   *           `privilegedName` {string} a privileged name assigned by the
+   *                            browser chrome.
+   *
+   * @rejects `nsresult` when there was an error retrieving the notification.
    */
-  bool handleWindowsTag(in AString aWindowsTag,
+  [implicit_jscontext]
-                        in nsIUnknownWindowsTagListener aListener);
+  Promise handleWindowsTag(in AString aWindowsTag);
 
   /**
    * Get the Windows-specific XML generated for the given alert.

widget/windows/ToastNotification.cpp

@@ -15,8 +15,11 @@
 #include "ErrorList.h"
 #include "mozilla/BasePrincipal.h"
 #include "mozilla/Buffer.h"
+#include "mozilla/dom/Promise.h"
 #include "mozilla/DynamicallyLinkedFunctionPtr.h"
+#include "mozilla/ErrorResult.h"
 #include "mozilla/mscom/COMWrappers.h"
+#include "mozilla/mscom/Utils.h"
 #include "mozilla/Logging.h"
 #include "mozilla/Services.h"
 #include "mozilla/WidgetUtils.h"
@@ -537,13 +540,10 @@ ToastNotification::GetXmlStringForWindowsAlert(nsIAlertNotification* aAlert,
   return handler->CreateToastXmlString(imageURL, aString);
 }
 
-NS_IMETHODIMP
+// Verifies that the tag recieved associates to a notification created during
-ToastNotification::HandleWindowsTag(const nsAString& aWindowsTag,
+// this application's session, or handles fallback behavior.
-                                    nsIUnknownWindowsTagListener* aListener,
+RefPtr<ToastHandledPromise> ToastNotification::VerifyTagPresentOrFallback(
-                                    bool* aRetVal) {
+    const nsAString& aWindowsTag) {
-  *aRetVal = false;
-  NS_ENSURE_TRUE(mAumid.isSome(), NS_ERROR_UNEXPECTED);
-
   MOZ_LOG(sWASLog, LogLevel::Debug,
           ("Iterating %d handlers", mActiveHandlers.Count()));
 
@@ -561,19 +561,75 @@ ToastNotification::HandleWindowsTag(const nsAString& aWindowsTag,
         MOZ_LOG(sWASLog, LogLevel::Debug,
                 ("External windowsTag '%s' is handled by handler [%p]",
                  NS_ConvertUTF16toUTF8(aWindowsTag).get(), handler.get()));
-        *aRetVal = true;
+        ToastHandledResolve handled{u""_ns, u""_ns};
+        return ToastHandledPromise::CreateAndResolve(handled, __func__);
+      }
+    } else {
+      MOZ_LOG(sWASLog, LogLevel::Debug,
+              ("Failed to get windowsTag for handler [%p]", handler.get()));
+    }
+  }
 
+  // Fallback handling
+
+  RefPtr<ToastHandledPromise::Private> fallbackPromise =
+      new ToastHandledPromise::Private(__func__);
+
+  // TODO: Bug 1806005 - At time of writing this function is called in a call
+  // stack containing `WndProc` callback on an STA thread. As a result attempts
+  // to create a `ToastNotificationManager` instance results an an
+  // `RPC_E_CANTCALLOUT_ININPUTSYNCCALL` error. We can simplify the the XPCOM
+  // interface and synchronize the COM interactions if notification fallback
+  // handling were no longer handled in a `WndProc` context.
+  NS_DispatchBackgroundTask(NS_NewRunnableFunction(
+      "VerifyTagPresentOrFallback fallback background task",
+      [fallbackPromise, aWindowsTag = nsString(aWindowsTag),
+       aAumid = nsString(mAumid.ref())]() {
+        MOZ_ASSERT(mscom::IsCOMInitializedOnCurrentThread());
+
+        bool foundTag;
+        nsAutoString launchUrl;
+        nsAutoString privilegedName;
+
+        nsresult rv = ToastNotificationHandler::
+            FindLaunchURLAndPrivilegedNameForWindowsTag(
+                aWindowsTag, aAumid, foundTag, launchUrl, privilegedName);
+
+        if (NS_FAILED(rv) || !foundTag) {
+          MOZ_LOG(sWASLog, LogLevel::Error,
+                  ("Failed to get launch URL and privileged name for "
+                   "notification tag '%s'",
+                   NS_ConvertUTF16toUTF8(aWindowsTag).get()));
+
+          fallbackPromise->Reject(false, __func__);
+          return;
+        }
+
+        MOZ_LOG(sWASLog, LogLevel::Debug,
+                ("Found launch URL '%s' and privileged name '%s' for "
+                 "windowsTag '%s'",
+                 NS_ConvertUTF16toUTF8(launchUrl).get(),
+                 NS_ConvertUTF16toUTF8(privilegedName).get(),
+                 NS_ConvertUTF16toUTF8(aWindowsTag).get()));
+
+        ToastHandledResolve handled{launchUrl, privilegedName};
+        fallbackPromise->Resolve(handled, __func__);
+      }));
+
+  return fallbackPromise;
+}
+
+void ToastNotification::SignalComNotificationHandled(
+    const nsAString& aWindowsTag) {
   nsString eventName(aWindowsTag);
-        nsAutoHandle event(
+  nsAutoHandle event(OpenEventW(EVENT_MODIFY_STATE, FALSE, eventName.get()));
-            OpenEventW(EVENT_MODIFY_STATE, FALSE, eventName.get()));
   if (event.get()) {
     if (SetEvent(event)) {
       MOZ_LOG(sWASLog, LogLevel::Info,
               ("Set event for event named '%s'",
                NS_ConvertUTF16toUTF8(eventName).get()));
     } else {
-            MOZ_LOG(
+      MOZ_LOG(sWASLog, LogLevel::Error,
-                sWASLog, LogLevel::Error,
               ("Failed to set event for event named '%s' (GetLastError=%lu)",
                NS_ConvertUTF16toUTF8(eventName).get(), GetLastError()));
     }
@@ -582,29 +638,67 @@ ToastNotification::HandleWindowsTag(const nsAString& aWindowsTag,
             ("Failed to open event named '%s' (GetLastError=%lu)",
              NS_ConvertUTF16toUTF8(eventName).get(), GetLastError()));
   }
+}
 
-        return NS_OK;
+NS_IMETHODIMP
-      }
+ToastNotification::HandleWindowsTag(const nsAString& aWindowsTag,
-    } else {
+                                    JSContext* aCx, dom::Promise** aPromise) {
-      MOZ_LOG(sWASLog, LogLevel::Debug,
+  NS_ENSURE_TRUE(mAumid.isSome(), NS_ERROR_UNEXPECTED);
-              ("Failed to get windowsTag for handler [%p]", handler.get()));
+  NS_ENSURE_TRUE(NS_IsMainThread(), NS_ERROR_NOT_SAME_THREAD);
-    }
+
+  ErrorResult rv;
+  RefPtr<dom::Promise> promise =
+      dom::Promise::Create(xpc::CurrentNativeGlobal(aCx), rv);
+  ENSURE_SUCCESS(rv, rv.StealNSResult());
+
+  this->VerifyTagPresentOrFallback(aWindowsTag)
+      ->Then(
+          GetMainThreadSerialEventTarget(), __func__,
+          [aWindowsTag = nsString(aWindowsTag),
+           promise](const ToastHandledResolve& aResolved) {
+            // We no longer need to query toast information from OS and can
+            // allow the COM server to proceed (toast information is lost once
+            // the COM server's `Activate` callback returns).
+            SignalComNotificationHandled(aWindowsTag);
+
+            dom::AutoJSAPI js;
+            if (NS_WARN_IF(!js.Init(promise->GetGlobalObject()))) {
+              promise->MaybeReject(NS_ERROR_FAILURE);
+              return;
             }
 
-  MOZ_LOG(sWASLog, LogLevel::Debug, ("aListener [%p]", aListener));
+            // Resolve the DOM Promise with a JS object. Set `launchUrl` and/or
-  if (aListener) {
+            // `privilegedName` properties if fallback handling is necessary.
-    bool foundTag;
-    nsAutoString launchUrl;
-    nsAutoString privilegedName;
-    MOZ_TRY(
-        ToastNotificationHandler::FindLaunchURLAndPrivilegedNameForWindowsTag(
-            aWindowsTag, mAumid.ref(), foundTag, launchUrl, privilegedName));
 
-    // The tag should always be found, so invoke the callback (even just for
+            JSContext* cx = js.cx();
-    // logging).
+            JS::Rooted<JSObject*> obj(cx, JS_NewPlainObject(cx));
-    aListener->HandleUnknownWindowsTag(aWindowsTag, launchUrl, privilegedName);
+
+            auto setProperty = [&](const char* name, const nsString& value) {
+              JS::Rooted<JSString*> title(cx,
+                                          JS_NewUCStringCopyZ(cx, value.get()));
+              JS::Rooted<JS::Value> attVal(cx, JS::StringValue(title));
+              Unused << NS_WARN_IF(!JS_SetProperty(cx, obj, name, attVal));
+            };
+
+            if (!aResolved.launchUrl.IsEmpty()) {
+              setProperty("launchUrl", aResolved.launchUrl);
+            }
+            if (!aResolved.privilegedName.IsEmpty()) {
+              setProperty("privilegedName", aResolved.privilegedName);
             }
 
+            promise->MaybeResolve(obj);
+          },
+          [aWindowsTag = nsString(aWindowsTag), promise]() {
+            // We no longer need to query toast information from OS and can
+            // allow the COM server to proceed (toast information is lost once
+            // the COM server's `Activate` callback returns).
+            SignalComNotificationHandled(aWindowsTag);
+
+            promise->MaybeReject(NS_ERROR_FAILURE);
+          });
+
+  promise.forget(aPromise);
   return NS_OK;
 }
 

widget/windows/ToastNotification.h

@@ -7,6 +7,7 @@
 #define ToastNotification_h__
 
 #include "mozilla/Maybe.h"
+#include "mozilla/MozPromise.h"
 #include "nsIAlertsService.h"
 #include "nsIObserver.h"
 #include "nsIThread.h"
@@ -16,6 +17,12 @@
 namespace mozilla {
 namespace widget {
 
+struct ToastHandledResolve {
+  const nsString launchUrl;
+  const nsString privilegedName;
+};
+using ToastHandledPromise = MozPromise<ToastHandledResolve, bool, true>;
+
 class ToastNotificationHandler;
 
 class ToastNotification final : public nsIWindowsAlertsService,
@@ -49,6 +56,10 @@ class ToastNotification final : public nsIWindowsAlertsService,
   static bool RegisterRuntimeAumid(nsAutoString& aInstallHash,
                                    Maybe<nsAutoString>& aAumid);
 
+  RefPtr<ToastHandledPromise> VerifyTagPresentOrFallback(
+      const nsAString& aWindowsTag);
+  static void SignalComNotificationHandled(const nsAString& aWindowsTag);
+
   nsRefPtrHashtable<nsStringHashKey, ToastNotificationHandler> mActiveHandlers;
   Maybe<nsAutoString> mAumid;
   bool mSuppressForScreenSharing = false;