browser(firefox): simplify PageTarget lifecycle (#4014)

As of today, we create `PageTarget` instances whenever we get a
sync IPC from the content process. This, however, breaks an invariant
that `browserContext.pages` always has all pages (and *browsing contexts* - not to be confused with *browser contexts*), associated with browser context. This invariant will be especially important when we move
user agent emulation to browser-side.

This patch makes `PageTarget` lifecycle symmetrical:
- `PageTarget` instance is created when tab is opened
- `PageTarget` is destroyed when tab is crashed or closed

This should also fix a bunch of race conditions with persistent mode, since sometimes we arrive to the window after its
initialization.

Drive-by: straighten viewport management and put a nice descriptive comment.

browser_patches/firefox/BUILD_NUMBER

@@ -1,2 +1,2 @@
-1175
+1176
-Changed: lushnikov@chromium.org Wed Sep 30 00:44:40 MDT 2020
+Changed: lushnikov@chromium.org Wed Sep 30 03:06:09 MDT 2020

browser_patches/firefox/juggler/TargetRegistry.js

@@ -141,50 +141,18 @@ class TargetRegistry {
     Services.mm.addMessageListener('juggler:content-ready', {
       receiveMessage: message => {
         const linkedBrowser = message.target;
-        if (this._browserToTarget.has(linkedBrowser))
+        const target = this._browserToTarget.get(linkedBrowser);
-          throw new Error(`Internal error: two targets per linkedBrowser`);
+        if (!target)
-
-        let tab;
-        let gBrowser;
-        const windowsIt = Services.wm.getEnumerator('navigator:browser');
-        let window;
-        while (windowsIt.hasMoreElements()) {
-          window = windowsIt.getNext();
-          // gBrowser is always created before tabs. If gBrowser is not
-          // initialized yet the browser belongs to another window.
-          if (!window.gBrowser)
-            continue;
-          tab = window.gBrowser.getTabForBrowser(linkedBrowser);
-          if (tab) {
-            gBrowser = window.gBrowser;
-            break;
-          }
-        }
-        if (!tab)
           return;
 
-        const { userContextId } = message.data;
-        const openerContext = linkedBrowser.browsingContext.opener;
-        let openerTarget;
-        if (openerContext) {
-          // Popups usually have opener context.
-          openerTarget = this._browserBrowsingContextToTarget.get(openerContext);
-        } else if (tab.openerTab) {
-          // Noopener popups from the same window have opener tab instead.
-          openerTarget = this._browserToTarget.get(tab.openerTab.linkedBrowser);
-        }
-        const browserContext = this._userContextIdToBrowserContext.get(userContextId);
-        if (!browserContext)
-          throw new Error(`Internal error: cannot find context for userContextId=${userContextId}`);
-        const target = new PageTarget(this, window, gBrowser, tab, linkedBrowser, browserContext, openerTarget);
-
         const sessions = [];
         const readyData = { sessions, target };
         this.emit(TargetRegistry.Events.TargetCreated, readyData);
+        target.markAsReported();
         return {
-          scriptsToEvaluateOnNewDocument: browserContext ? browserContext.scriptsToEvaluateOnNewDocument : [],
+          scriptsToEvaluateOnNewDocument: target.browserContext().scriptsToEvaluateOnNewDocument,
-          bindings: browserContext ? browserContext.bindings : [],
+          bindings: target.browserContext().bindings,
-          settings: browserContext ? browserContext.settings : {},
+          settings: target.browserContext().settings,
           sessionIds: sessions.map(session => session.sessionId()),
         };
       },
@@ -195,8 +163,20 @@ class TargetRegistry {
       const userContextId = tab.userContextId;
       const browserContext = this._userContextIdToBrowserContext.get(userContextId);
       const hasExplicitSize = (appWindow.chromeFlags & Ci.nsIWebBrowserChrome.JUGGLER_WINDOW_EXPLICIT_SIZE) !== 0;
-      if (!hasExplicitSize && browserContext && browserContext.defaultViewportSize)
+      const openerContext = tab.linkedBrowser.browsingContext.opener;
-        setViewportSizeForBrowser(browserContext.defaultViewportSize, tab.linkedBrowser, window);
+      let openerTarget;
+      if (openerContext) {
+        // Popups usually have opener context.
+        openerTarget = this._browserBrowsingContextToTarget.get(openerContext);
+      } else if (tab.openerTab) {
+        // Noopener popups from the same window have opener tab instead.
+        openerTarget = this._browserToTarget.get(tab.openerTab.linkedBrowser);
+      }
+      if (!browserContext)
+        throw new Error(`Internal error: cannot find context for userContextId=${userContextId}`);
+      const target = new PageTarget(this, window, tab, browserContext, openerTarget);
+      if (!hasExplicitSize)
+        target.updateViewportSize();
     };
 
     const onTabCloseListener = event => {
@@ -215,9 +195,18 @@ class TargetRegistry {
       const domWindow = appWindow.QueryInterface(Ci.nsIInterfaceRequestor).getInterface(Ci.nsIDOMWindowInternal || Ci.nsIDOMWindow);
       if (!(domWindow instanceof Ci.nsIDOMChromeWindow))
         return;
-      if (domWindow.document.readyState !== 'uninitialized')
+      // In persistent mode, window might be opened long ago and might be
-        throw new Error('DOMWindow should not be loaded yet');
+      // already initialized.
-      await helper.awaitEvent(domWindow, 'DOMContentLoaded');
+      //
+      // In this case, we want to keep this callback synchronous so that we will call
+      // `onTabOpenListener` synchronously and before the sync IPc message `juggler:content-ready`.
+      if (domWindow.document.readyState === 'uninitialized' || domWindow.document.readyState === 'loading') {
+        // For non-initialized windows, DOMContentLoaded initializes gBrowser
+        // and starts tab loading (see //browser/base/content/browser.js), so we
+        // are guaranteed to call `onTabOpenListener` before the sync IPC message
+        // `juggler:content-ready`.
+        await helper.awaitEvent(domWindow, 'DOMContentLoaded');
+      }
 
       if (!domWindow.gBrowser)
         return;
@@ -244,12 +233,12 @@ class TargetRegistry {
         onTabCloseListener({ target: tab });
     };
 
+    const extHelperAppSvc = Cc["@mozilla.org/uriloader/external-helper-app-service;1"].getService(Ci.nsIExternalHelperAppService);
+    extHelperAppSvc.setDownloadInterceptor(new DownloadInterceptor(this));
+
     Services.wm.addListener({ onOpenWindow, onCloseWindow });
     for (const win of Services.wm.getEnumerator(null))
       onOpenWindow(win);
-
-    const extHelperAppSvc = Cc["@mozilla.org/uriloader/external-helper-app-service;1"].getService(Ci.nsIExternalHelperAppService);
-    extHelperAppSvc.setDownloadInterceptor(new DownloadInterceptor(this));
   }
 
   setBrowserProxy(proxy) {
@@ -321,19 +310,13 @@ class TargetRegistry {
     if (window.gBrowser.browsers.length !== 1)
       throw new Error(`Unexpected number of tabs in the new window: ${window.gBrowser.browsers.length}`);
     const browser = window.gBrowser.browsers[0];
-    const target = this._browserToTarget.get(browser) || await new Promise(fulfill => {
+    const target = this._browserToTarget.get(browser);
-      const listener = helper.on(this, TargetRegistry.Events.TargetCreated, ({target}) => {
-        if (target._linkedBrowser === browser) {
-          helper.removeListeners([listener]);
-          fulfill(target);
-        }
-      });
-    });
     browser.focus();
     if (browserContext.settings.timezoneId) {
       if (await target.hasFailedToOverrideTimezone())
         throw new Error('Failed to override timezone');
     }
+    await target.reportedPromise();
     return target.id();
   }
 
@@ -356,15 +339,15 @@ class TargetRegistry {
 }
 
 class PageTarget {
-  constructor(registry, win, gBrowser, tab, linkedBrowser, browserContext, opener) {
+  constructor(registry, win, tab, browserContext, opener) {
     EventEmitter.decorate(this);
 
     this._targetId = helper.generateId();
     this._registry = registry;
     this._window = win;
-    this._gBrowser = gBrowser;
+    this._gBrowser = win.gBrowser;
     this._tab = tab;
-    this._linkedBrowser = linkedBrowser;
+    this._linkedBrowser = tab.linkedBrowser;
     this._browserContext = browserContext;
     this._viewportSize = undefined;
     this._url = 'about:blank';
@@ -380,12 +363,24 @@ class PageTarget {
       helper.addProgressListener(tab.linkedBrowser, navigationListener, Ci.nsIWebProgress.NOTIFY_LOCATION),
     ];
 
+    this._reportedPromise = new Promise(resolve => {
+      this._reportedCallback = resolve;
+    });
+
     this._disposed = false;
     browserContext.pages.add(this);
     this._registry._browserToTarget.set(this._linkedBrowser, this);
     this._registry._browserBrowsingContextToTarget.set(this._linkedBrowser.browsingContext, this);
   }
 
+  reportedPromise() {
+    return this._reportedPromise;
+  }
+
+  markAsReported() {
+    this._reportedCallback();
+  }
+
   async windowReady() {
     await waitForWindowReady(this._window);
   }
@@ -398,8 +393,17 @@ class PageTarget {
     return this._browserContext;
   }
 
-  async setViewportSize(viewportSize) {
+  async updateViewportSize() {
-    this._viewportSize = viewportSize;
+    // Viewport size is defined by three arguments:
+    // 1. default size. Could be explicit if set as part of `window.open` call, e.g.
+    //   `window.open(url, title, 'width=400,height=400')`
+    // 2. page viewport size
+    // 3. browserContext viewport size
+    //
+    // The "default size" (1) is only respected when the page is opened.
+    // Otherwise, explicitly set page viewport prevales over browser context
+    // default viewport.
+    const viewportSize = this._viewportSize || this._browserContext.defaultViewportSize;
     const actualSize = setViewportSizeForBrowser(viewportSize, this._linkedBrowser, this._window);
     await this._channel.connect('').send('awaitViewportDimensions', {
       width: actualSize.width,
@@ -407,6 +411,11 @@ class PageTarget {
     });
   }
 
+  async setViewportSize(viewportSize) {
+    this._viewportSize = viewportSize;
+    await this.updateViewportSize();
+  }
+
   connectSession(session) {
     this._channel.connect('').send('attach', { sessionId: session.sessionId() });
   }
@@ -552,11 +561,7 @@ class BrowserContext {
 
   async setDefaultViewport(viewport) {
     this.defaultViewportSize = viewport ? viewport.viewportSize : undefined;
-    const promises = Array.from(this.pages).map(async page => {
+    const promises = Array.from(this.pages).map(page => page.updateViewportSize());
-      // Resize to new default, unless the page has a custom viewport.
-      if (!page._viewportSize)
-        await page.setViewportSize(this.defaultViewportSize);
-    });
     await Promise.all([
       this.applySetting('deviceScaleFactor', viewport ? viewport.deviceScaleFactor : undefined),
       ...promises,