Bug 1694390 - [bidi] Implement browsingContext.contextDestroyed event. r=webdriver-reviewers,jdescottes,whimboo

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

remote/shared/TabManager.sys.mjs

@@ -6,6 +6,8 @@ const lazy = {};
 
 ChromeUtils.defineESModuleGetters(lazy, {
   AppInfo: "chrome://remote/content/shared/AppInfo.sys.mjs",
+  BrowsingContextListener:
+    "chrome://remote/content/shared/listeners/BrowsingContextListener.sys.mjs",
   EventPromise: "chrome://remote/content/shared/Sync.sys.mjs",
   generateUUID: "chrome://remote/content/shared/UUID.sys.mjs",
   MobileTabBrowser: "chrome://remote/content/shared/MobileTabBrowser.sys.mjs",
@@ -13,10 +15,31 @@ ChromeUtils.defineESModuleGetters(lazy, {
 
 class TabManagerClass {
   #browserUniqueIds;
+  #contextListener;
+  #navigableIds;
 
   constructor() {
     // Maps browser's permanentKey to uuid: WeakMap.<Object, string>
     this.#browserUniqueIds = new WeakMap();
+
+    // Maps browsing contexts to uuid: WeakMap.<BrowsingContext, string>.
+    // It's required as a fallback, since in the case when a context was discarded
+    // embedderElement is gone, and we cannot retrieve
+    // the context id from this.#browserUniqueIds.
+    this.#navigableIds = new WeakMap();
+
+    this.#contextListener = new lazy.BrowsingContextListener();
+    this.#contextListener.on("attached", this.#onContextAttached);
+    this.#contextListener.startListening();
+
+    this.browsers.forEach(browser => {
+      if (this.isValidCanonicalBrowsingContext(browser.browsingContext)) {
+        this.#navigableIds.set(
+          browser.browsingContext,
+          this.getIdForBrowsingContext(browser.browsingContext)
+        );
+      }
+    });
   }
 
   /**
@@ -219,6 +242,10 @@ class TabManagerClass {
     }
 
     const key = browserElement.permanentKey;
+    if (key === undefined) {
+      return null;
+    }
+
     if (!this.#browserUniqueIds.has(key)) {
       this.#browserUniqueIds.set(key, lazy.generateUUID());
     }
@@ -243,7 +270,11 @@ class TabManagerClass {
 
     if (!browsingContext.parent) {
       // Top-level browsing contexts have their own custom unique id.
-      return this.getIdForBrowser(browsingContext.embedderElement);
+      // If a context was discarded, embedderElement is already gone,
+      // so use navigable id instead.
+      return browsingContext.embedderElement
+        ? this.getIdForBrowser(browsingContext.embedderElement)
+        : this.#navigableIds.get(browsingContext);
     }
 
     return browsingContext.id.toString();
@@ -395,6 +426,16 @@ class TabManagerClass {
   supportsTabs() {
     return lazy.AppInfo.isAndroid || lazy.AppInfo.isFirefox;
   }
+
+  #onContextAttached = (eventName, data = {}) => {
+    const { browsingContext } = data;
+    if (this.isValidCanonicalBrowsingContext(browsingContext)) {
+      this.#navigableIds.set(
+        browsingContext,
+        this.getIdForBrowsingContext(browsingContext)
+      );
+    }
+  };
 }
 
 // Expose a shared singleton.

remote/shared/listeners/BrowsingContextListener.sys.mjs

@@ -58,6 +58,7 @@ export class BrowsingContextListener {
 
   destroy() {
     this.stopListening();
+    this.#topContextsToAttach = null;
   }
 
   observe(subject, topic, data) {

remote/test/puppeteer/test/TestExpectations.json

@@ -1919,12 +1919,6 @@
     "parameters": ["firefox", "webDriverBiDi"],
     "expectations": ["FAIL", "TIMEOUT"]
   },
-  {
-    "testIdPattern": "[frame.spec] Frame specs Frame Management should detach child frames on navigation",
-    "platforms": ["darwin", "linux", "win32"],
-    "parameters": ["firefox", "webDriverBiDi"],
-    "expectations": ["FAIL"]
-  },
   {
     "testIdPattern": "[frame.spec] Frame specs Frame Management should report different frame instance when frame re-attaches",
     "platforms": ["darwin", "linux", "win32"],
@@ -1973,12 +1967,6 @@
     "parameters": ["firefox", "webDriverBiDi"],
     "expectations": ["FAIL"]
   },
-  {
-    "testIdPattern": "[frame.spec] Frame specs Frame Management should support framesets",
-    "platforms": ["darwin", "linux", "win32"],
-    "parameters": ["firefox", "webDriverBiDi"],
-    "expectations": ["FAIL"]
-  },
   {
     "testIdPattern": "[frame.spec] Frame specs Frame Management should support lazy frames",
     "platforms": ["darwin", "linux", "win32"],
@@ -2459,12 +2447,6 @@
     "parameters": ["cdp", "firefox"],
     "expectations": ["SKIP"]
   },
-  {
-    "testIdPattern": "[navigation.spec] navigation Frame.waitForNavigation should fail when frame detaches",
-    "platforms": ["darwin", "linux", "win32"],
-    "parameters": ["firefox", "webDriverBiDi"],
-    "expectations": ["TIMEOUT"]
-  },
   {
     "testIdPattern": "[navigation.spec] navigation Frame.waitForNavigation should work",
     "platforms": ["darwin", "linux", "win32"],
@@ -3134,7 +3116,7 @@
   {
     "testIdPattern": "[page.spec] Page Page.close should not be visible in browser.pages",
     "platforms": ["darwin", "linux", "win32"],
-    "parameters": ["chrome", "webDriverBiDi"],
+    "parameters": ["webDriverBiDi"],
     "expectations": ["PASS"]
   },
   {
@@ -3158,7 +3140,7 @@
   {
     "testIdPattern": "[page.spec] Page Page.Events.Close should work with window.close",
     "platforms": ["darwin", "linux", "win32"],
-    "parameters": ["chrome", "webDriverBiDi"],
+    "parameters": ["webDriverBiDi"],
     "expectations": ["PASS"]
   },
   {

remote/webdriver-bidi/modules/root/browsingContext.sys.mjs

@@ -125,9 +125,9 @@ class BrowsingContextModule extends Module {
   constructor(messageHandler) {
     super(messageHandler);
 
-    // Create the browsing context listener and listen to "attached" events.
     this.#contextListener = new lazy.BrowsingContextListener();
     this.#contextListener.on("attached", this.#onContextAttached);
+    this.#contextListener.on("discarded", this.#onContextDiscarded);
 
     // Create the navigation listener and listen to "navigation-started" and
     // "location-changed" events.
@@ -147,10 +147,14 @@ class BrowsingContextModule extends Module {
 
     // Set of event names which have active subscriptions.
     this.#subscribedEvents = new Set();
+
+    // Treat the event of moving a page to BFCache as context discarded event for iframes.
+    this.messageHandler.on("windowglobal-pagehide", this.#onPageHideEvent);
   }
 
   destroy() {
     this.#contextListener.off("attached", this.#onContextAttached);
+    this.#contextListener.off("discarded", this.#onContextDiscarded);
     this.#contextListener.destroy();
 
     this.#promptListener.off("closed", this.#onPromptClosed);
@@ -158,6 +162,8 @@ class BrowsingContextModule extends Module {
     this.#promptListener.destroy();
 
     this.#subscribedEvents = null;
+
+    this.messageHandler.off("windowglobal-pagehide", this.#onPageHideEvent);
   }
 
   /**
@@ -1216,36 +1222,94 @@ class BrowsingContextModule extends Module {
   }
 
   #onContextAttached = async (eventName, data = {}) => {
-    const { browsingContext, why } = data;
+    if (this.#subscribedEvents.has("browsingContext.contextCreated")) {
+      const { browsingContext, why } = data;
 
-    // Filter out top-level browsing contexts that are created because of a
-    // cross-group navigation.
-    if (why === "replace") {
-      return;
+      // Filter out top-level browsing contexts that are created because of a
+      // cross-group navigation.
+      if (why === "replace") {
+        return;
+      }
+
+      // TODO: Bug 1852941. We should also filter out events which are emitted
+      // for DevTools frames.
+
+      // Filter out notifications for chrome context until support gets
+      // added (bug 1722679).
+      if (!browsingContext.webProgress) {
+        return;
+      }
+
+      const browsingContextInfo = this.#getBrowsingContextInfo(
+        browsingContext,
+        {
+          maxDepth: 0,
+        }
+      );
+
+      // This event is emitted from the parent process but for a given browsing
+      // context. Set the event's contextInfo to the message handler corresponding
+      // to this browsing context.
+      const contextInfo = {
+        contextId: browsingContext.id,
+        type: lazy.WindowGlobalMessageHandler.type,
+      };
+      this.emitEvent(
+        "browsingContext.contextCreated",
+        browsingContextInfo,
+        contextInfo
+      );
     }
+  };
 
-    // Filter out notifications for chrome context until support gets
-    // added (bug 1722679).
-    if (!browsingContext.webProgress) {
-      return;
+  #onContextDiscarded = async (eventName, data = {}) => {
+    if (this.#subscribedEvents.has("browsingContext.contextDestroyed")) {
+      const { browsingContext, why } = data;
+
+      // Filter out top-level browsing contexts that are destroyed because of a
+      // cross-group navigation.
+      if (why === "replace") {
+        return;
+      }
+
+      // TODO: Bug 1852941. We should also filter out events which are emitted
+      // for DevTools frames.
+
+      // Filter out notifications for chrome context until support gets
+      // added (bug 1722679).
+      if (!browsingContext.webProgress) {
+        return;
+      }
+
+      // If this event is for a child context whose top or parent context is also destroyed,
+      // we don't need to send it, in this case the event for the top/parent context is enough.
+      if (
+        browsingContext.parent &&
+        (browsingContext.top.isDiscarded || browsingContext.parent.isDiscarded)
+      ) {
+        return;
+      }
+
+      const browsingContextInfo = this.#getBrowsingContextInfo(
+        browsingContext,
+        {
+          maxDepth: 0,
+        }
+      );
+
+      // This event is emitted from the parent process but for a given browsing
+      // context. Set the event's contextInfo to the message handler corresponding
+      // to this browsing context.
+      const contextInfo = {
+        contextId: browsingContext.id,
+        type: lazy.WindowGlobalMessageHandler.type,
+      };
+      this.emitEvent(
+        "browsingContext.contextDestroyed",
+        browsingContextInfo,
+        contextInfo
+      );
     }
-
-    const browsingContextInfo = this.#getBrowsingContextInfo(browsingContext, {
-      maxDepth: 0,
-    });
-
-    // This event is emitted from the parent process but for a given browsing
-    // context. Set the event's contextInfo to the message handler corresponding
-    // to this browsing context.
-    const contextInfo = {
-      contextId: browsingContext.id,
-      type: lazy.WindowGlobalMessageHandler.type,
-    };
-    this.emitEvent(
-      "browsingContext.contextCreated",
-      browsingContextInfo,
-      contextInfo
-    );
   };
 
   #onLocationChanged = async (eventName, data) => {
@@ -1361,6 +1425,15 @@ class BrowsingContextModule extends Module {
     }
   };
 
+  #onPageHideEvent = (name, eventPayload) => {
+    const { context } = eventPayload;
+    if (context.parent) {
+      this.#onContextDiscarded("windowglobal-pagehide", {
+        browsingContext: context,
+      });
+    }
+  };
+
   #stopListeningToNavigationEvent(event) {
     this.#subscribedEvents.delete(event);
 
@@ -1387,7 +1460,8 @@ class BrowsingContextModule extends Module {
 
   #subscribeEvent(event) {
     switch (event) {
-      case "browsingContext.contextCreated": {
+      case "browsingContext.contextCreated":
+      case "browsingContext.contextDestroyed": {
         this.#contextListener.startListening();
         this.#subscribedEvents.add(event);
         break;
@@ -1409,7 +1483,8 @@ class BrowsingContextModule extends Module {
 
   #unsubscribeEvent(event) {
     switch (event) {
-      case "browsingContext.contextCreated": {
+      case "browsingContext.contextCreated":
+      case "browsingContext.contextDestroyed": {
         this.#contextListener.stopListening();
         this.#subscribedEvents.delete(event);
         break;
@@ -1460,6 +1535,7 @@ class BrowsingContextModule extends Module {
   static get supportedEvents() {
     return [
       "browsingContext.contextCreated",
+      "browsingContext.contextDestroyed",
       "browsingContext.domContentLoaded",
       "browsingContext.fragmentNavigated",
       "browsingContext.load",