Revert "Bug 1973269 - Pass the URI directly to browser chrome page r=asuth,Gijs" for causing mochitest failures @ test_notification_serviceworker_openWindow.html

This reverts commit 8c1f5ef.

Author: Sandor Molnar

dom/clients/manager/ClientOpenWindowUtils.cpp

@@ -215,16 +215,6 @@ static Result<Ok, nsresult> OpenNewWindow(
     nsOpenWindowInfo* aOpenWindowInfo) {
   nsresult rv;
 
-  // XXX(krosylight): In an ideal world we should be able to pass the nsIURI
-  // directly. See bug 1485961.
-  nsAutoCString uriToLoad;
-  MOZ_TRY(aArgsValidated.uri->GetSpec(uriToLoad));
-
-  nsCOMPtr<nsISupportsCString> nsUriToLoad =
-      do_CreateInstance(NS_SUPPORTS_CSTRING_CONTRACTID, &rv);
-  MOZ_TRY(rv);
-  MOZ_TRY(nsUriToLoad->SetData(uriToLoad));
-
   nsCOMPtr<nsISupportsPRBool> nsFalse =
       do_CreateInstance(NS_SUPPORTS_PRBOOL_CONTRACTID, &rv);
   MOZ_TRY(rv);
@@ -237,7 +227,7 @@ static Result<Ok, nsresult> OpenNewWindow(
 
   nsCOMPtr<nsIMutableArray> args = do_CreateInstance(NS_ARRAY_CONTRACTID);
   // https://searchfox.org/mozilla-central/rev/02d33f4bf984f65bd394bfd2d19d66569ae2cfe1/browser/base/content/browser-init.js#725-735
-  args->AppendElement(nsUriToLoad);               // 0: uriToLoad
+  args->AppendElement(nullptr);                   // 0: uriToLoad
   args->AppendElement(nullptr);                   // 1: extraOptions
   args->AppendElement(nullptr);                   // 2: referrerInfo
   args->AppendElement(nullptr);                   // 3: postData
@@ -320,7 +310,7 @@ void OpenWindow(const ClientOpenWindowArgsParsed& aArgsValidated,
 
 void WaitForLoad(const ClientOpenWindowArgsParsed& aArgsValidated,
                  BrowsingContext* aBrowsingContext,
-                 ClientOpPromise::Private* aPromise, bool aShouldLoadURI) {
+                 ClientOpPromise::Private* aPromise) {
   MOZ_DIAGNOSTIC_ASSERT(aBrowsingContext);
 
   RefPtr<ClientOpPromise::Private> promise = aPromise;
@@ -338,7 +328,7 @@ void WaitForLoad(const ClientOpenWindowArgsParsed& aArgsValidated,
     return;
   }
 
-  // Add a progress listener before we start the load of the requested URI
+  // Add a progress listener before we start the load of the service worker URI
   RefPtr<WebProgressListener> listener = new WebProgressListener(
       aBrowsingContext, aArgsValidated.baseURI, do_AddRef(promise));
 
@@ -352,27 +342,24 @@ void WaitForLoad(const ClientOpenWindowArgsParsed& aArgsValidated,
     return;
   }
 
-  if (aShouldLoadURI) {
-    // Load the requested URI
-    RefPtr<nsDocShellLoadState> loadState =
-        new nsDocShellLoadState(aArgsValidated.uri);
-    loadState->SetTriggeringPrincipal(aArgsValidated.principal);
-    loadState->SetFirstParty(true);
-    loadState->SetLoadFlags(
-        nsIWebNavigation::LOAD_FLAGS_DISALLOW_INHERIT_PRINCIPAL);
-    loadState->SetTriggeringRemoteType(
-        aArgsValidated.originContent
-            ? aArgsValidated.originContent->GetRemoteType()
-            : NOT_REMOTE_TYPE);
-
-    rv = aBrowsingContext->LoadURI(loadState, true);
-    if (NS_FAILED(rv)) {
-      CopyableErrorResult result;
-      result.ThrowInvalidStateError(
-          "Unable to start the load of the actual URI");
-      promise->Reject(result, __func__);
-      return;
-    }
+  // Load the service worker URI
+  RefPtr<nsDocShellLoadState> loadState =
+      new nsDocShellLoadState(aArgsValidated.uri);
+  loadState->SetTriggeringPrincipal(aArgsValidated.principal);
+  loadState->SetFirstParty(true);
+  loadState->SetLoadFlags(
+      nsIWebNavigation::LOAD_FLAGS_DISALLOW_INHERIT_PRINCIPAL);
+  loadState->SetTriggeringRemoteType(
+      aArgsValidated.originContent
+          ? aArgsValidated.originContent->GetRemoteType()
+          : NOT_REMOTE_TYPE);
+
+  rv = aBrowsingContext->LoadURI(loadState, true);
+  if (NS_FAILED(rv)) {
+    CopyableErrorResult result;
+    result.ThrowInvalidStateError("Unable to start the load of the actual URI");
+    promise->Reject(result, __func__);
+    return;
   }
 
   // Hold the listener alive until the promise settles.
@@ -384,7 +371,8 @@ void WaitForLoad(const ClientOpenWindowArgsParsed& aArgsValidated,
 
 #ifdef MOZ_GECKOVIEW
 void GeckoViewOpenWindow(const ClientOpenWindowArgsParsed& aArgsValidated,
-                         nsOpenWindowInfo* aOpenInfo, ErrorResult& aRv) {
+                         nsOpenWindowInfo* aOpenInfo, BrowsingContext** aBC,
+                         ErrorResult& aRv) {
   MOZ_ASSERT(aOpenInfo);
 
   // passes the request to open a new window to GeckoView. Allowing the
@@ -499,13 +487,11 @@ RefPtr<ClientOpPromise> ClientOpenWindow(
 
   RefPtr<BrowsingContext> bc;
   IgnoredErrorResult errResult;
-  bool shouldLoadURI = true;
 #ifdef MOZ_GECKOVIEW
   // GeckoView has a delegation for service worker window.
-  GeckoViewOpenWindow(argsValidated, openInfo, errResult);
+  GeckoViewOpenWindow(argsValidated, openInfo, getter_AddRefs(bc), errResult);
 #else
   OpenWindow(argsValidated, openInfo, getter_AddRefs(bc), errResult);
-  shouldLoadURI = !!bc;
 #endif
   if (NS_WARN_IF(errResult.Failed())) {
     promise->Reject(errResult, __func__);
@@ -514,9 +500,8 @@ RefPtr<ClientOpPromise> ClientOpenWindow(
 
   browsingContextReadyPromise->Then(
       GetCurrentSerialEventTarget(), __func__,
-      [argsValidated, promise,
-       shouldLoadURI](const RefPtr<BrowsingContext>& aBC) {
-        WaitForLoad(argsValidated, aBC, promise, shouldLoadURI);
+      [argsValidated, promise](const RefPtr<BrowsingContext>& aBC) {
+        WaitForLoad(argsValidated, aBC, promise);
       },
       [promise]() {
         // in case of failure, reject the original promise

dom/notification/test/browser/browser_openwindow_without_window.js

@@ -46,31 +46,12 @@ add_setup(() => {
   BrowserTestUtils.concealWindow(window, { signal });
 });
 
-let configs = [
-  {
-    permanentPbm: false,
-    browserStartup: 1,
-  },
-  {
-    permanentPbm: true,
-    browserStartup: 1,
-  },
-  {
-    permanentPbm: false,
-    browserStartup: 3,
-  },
-];
-
-for (let { permanentPbm, browserStartup } of configs) {
+for (let permanentPbm of [false, true]) {
   add_task(async function () {
     info(`Test with PBM: ${permanentPbm}`);
-    info(`Test with startup behavior: ${browserStartup}`);
 
     await SpecialPowers.pushPrefEnv({
-      set: [
-        ["browser.privatebrowsing.autostart", permanentPbm],
-        ["browser.startup.page", browserStartup],
-      ],
+      set: [["browser.privatebrowsing.autostart", permanentPbm]],
     });
 
     let win = await BrowserTestUtils.openNewBrowserWindow();
@@ -104,15 +85,8 @@ for (let { permanentPbm, browserStartup } of configs) {
 
     registerCleanupFunction(() => SpecialPowers.removeAllServiceWorkerData());
 
-    // Now close the current window via BrowserCommands
-    // (simple .close() will not trigger session store as it does not notify
-    // browser-lastwindow-close-granted)
-    let closedPromise = BrowserTestUtils.windowClosed(win);
-    win.BrowserCommands.tryToCloseWindow();
-    await closedPromise;
-
-    // Let session startup read the changed pref again
-    SessionStartup.resetForTest();
+    // Now close the current window
+    await BrowserTestUtils.closeWindow(win);
 
     let newWinPromise = BrowserTestUtils.waitForNewWindow({
       url: OPEN_URI,