Bug 1777509 - Don't return browsing context directly when creating new window. r=nika,geckoview-reviewers,owlish

Desktop's `nsIBrowserWindow.createContetnWindow` will return null when
creating new window. Then browsing context is observed by `nsFrameLoader`.

Actually, GeckoView's `createContentWindow` always creates new window.
So if using new window option, we should return null like desktop.

Also, if no callback, we return browsing context even if creating new window
due to no way to notify browsing context. And,
`nsIWindowProvider.provideWindow` doesn't use the callback, we have to return
browsing context for this situation.

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

mobile/android/modules/geckoview/GeckoViewNavigation.sys.mjs

@@ -291,10 +291,10 @@ export class GeckoViewNavigation extends GeckoViewModule {
 
   waitAndSetupWindow(aSessionId, aOpenWindowInfo, aName) {
     if (!aSessionId) {
-      return Promise.resolve(null);
+      return Promise.reject();
     }
 
-    return new Promise(resolve => {
+    return new Promise((resolve, reject) => {
       const handler = {
         observe(aSubject, aTopic) {
           if (
@@ -318,6 +318,10 @@ export class GeckoViewNavigation extends GeckoViewModule {
               aSubject.browser.removeAttribute("remoteType");
             }
             Services.obs.removeObserver(handler, "geckoview-window-created");
+            if (!aSubject) {
+              reject();
+              return;
+            }
             resolve(aSubject);
           }
         },
@@ -332,8 +336,46 @@ export class GeckoViewNavigation extends GeckoViewModule {
     debug`handleNewSession: uri=${aUri && aUri.spec}
                              where=${aWhere} flags=${aFlags}`;
 
+    const setupPromise = this.#handleNewSessionAsync(
+      aUri,
+      aOpenWindowInfo,
+      aFlags,
+      aName
+    );
+
+    let browser = undefined;
+    setupPromise.then(
+      window => {
+        browser = window.browser;
+      },
+      () => {
+        browser = null;
+      }
+    );
+
+    // Wait indefinitely for app to respond with a browser or null
+    Services.tm.spinEventLoopUntil(
+      "GeckoViewNavigation.jsm:handleNewSession",
+      () => this.window.closed || browser !== undefined
+    );
+    return browser || null;
+  }
+
+  #isNewTab(aWhere) {
+    return [
+      Ci.nsIBrowserDOMWindow.OPEN_NEWTAB,
+      Ci.nsIBrowserDOMWindow.OPEN_NEWTAB_BACKGROUND,
+      Ci.nsIBrowserDOMWindow.OPEN_NEWTAB_FOREGROUND,
+    ].includes(aWhere);
+  }
+
+  /**
+   * Similar to handleNewSession. But this returns a promise to wait for new
+   * browser.
+   */
+  #handleNewSessionAsync(aUri, aOpenWindowInfo, aFlags, aName) {
     if (!this.enabled) {
-      return null;
+      return Promise.reject();
     }
 
     const newSessionId = Services.uuid
@@ -356,30 +398,15 @@ export class GeckoViewNavigation extends GeckoViewModule {
       aName
     );
 
-    let browser = undefined;
-    this.eventDispatcher
+    return this.eventDispatcher
       .sendRequestForResult(message)
       .then(didOpenSession => {
         if (!didOpenSession) {
+          // New session cannot be opened, so we should throw NS_ERROR_ABORT.
           return Promise.reject();
         }
         return setupPromise;
-      })
-      .then(
-        window => {
-          browser = window.browser;
-        },
-        () => {
-          browser = null;
-        }
-      );
-
-    // Wait indefinitely for app to respond with a browser or null
-    Services.tm.spinEventLoopUntil(
-      "GeckoViewNavigation.sys.mjs:handleNewSession",
-      () => this.window.closed || browser !== undefined
-    );
-    return browser || null;
+      });
   }
 
   // nsIBrowserDOMWindow.
@@ -393,6 +420,11 @@ export class GeckoViewNavigation extends GeckoViewModule {
     debug`createContentWindow: uri=${aUri && aUri.spec}
                                 where=${aWhere} flags=${aFlags}`;
 
+    if (!this.enabled) {
+      Components.returnCode = Cr.NS_ERROR_ABORT;
+      return null;
+    }
+
     if (
       lazy.LoadURIDelegate.load(
         this.window,
@@ -408,13 +440,45 @@ export class GeckoViewNavigation extends GeckoViewModule {
       return null;
     }
 
-    const browser = this.handleNewSession(
+    const newTab = this.#isNewTab(aWhere);
+    const promise = this.#handleNewSessionAsync(
       aUri,
       aOpenWindowInfo,
       aWhere,
       aFlags,
       null
     );
+
+    // Actually, GeckoView's createContentWindow always creates new window even
+    // if OPEN_NEWTAB. So the browsing context will be observed via
+    // nsFrameLoader.
+    if (aOpenWindowInfo && !newTab) {
+      promise.catch(() => {
+        aOpenWindowInfo.cancel();
+      });
+      // If nsIOpenWindowInfo isn't null, caller should use the callback.
+      // Also, nsIWindowProvider.provideWindow doesn't use callback, if new
+      // tab option, we have to return browsing context instead of async.
+      return null;
+    }
+
+    let browser = undefined;
+    promise.then(
+      window => {
+        browser = window.browser;
+      },
+      () => {
+        browser = null;
+      }
+    );
+
+    // Wait indefinitely for app to respond with a browser or null.
+    // if browser is null, return error.
+    Services.tm.spinEventLoopUntil(
+      "GeckoViewNavigation.sys.mjs:createContentWindow",
+      () => this.window.closed || browser !== undefined
+    );
+
     if (!browser) {
       Components.returnCode = Cr.NS_ERROR_ABORT;
       return null;

toolkit/components/windowwatcher/nsIOpenWindowInfo.idl

@@ -74,4 +74,10 @@ interface nsIOpenWindowInfo : nsISupports {
   /* Callback to invoke when the browsing context for a new window is ready. */
   [notxpcom, nostdcall]
   nsIBrowsingContextReadyCallback browsingContextReadyCallback();
+
+  /**
+   * When creating a window fails, we will be called to notify an error via
+   * callback. If no callback, do nothing.
+   */
+  void cancel();
 };

toolkit/components/windowwatcher/nsOpenWindowInfo.cpp

@@ -57,6 +57,14 @@ nsOpenWindowInfo::BrowsingContextReadyCallback() {
   return mBrowsingContextReadyCallback;
 }
 
+NS_IMETHODIMP nsOpenWindowInfo::Cancel() {
+  if (mBrowsingContextReadyCallback) {
+    mBrowsingContextReadyCallback->BrowsingContextReady(nullptr);
+    mBrowsingContextReadyCallback = nullptr;
+  }
+  return NS_OK;
+}
+
 NS_IMPL_ISUPPORTS(nsBrowsingContextReadyCallback,
                   nsIBrowsingContextReadyCallback)