Bug 1989056 - [devtools] Set a fixed window on WindowGlobalTarget. r=devtools-reviewers,bomsy

Most targets are bound to a unique WindowGlobal (all but some related to
the browser toolbox in the parent process).
Given that accessing docShell.domWindow can easily on destruction,
let's get a fixed reference to the window.
The window itself may be throwing, but at least accessing to simple attributes
should be functional.

Also introduce a test to cover destroying iframes early after their creation
and fix some leftover unhandled exceptions.

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

Author: ochameau

devtools/client/framework/test/browser.toml

@@ -65,6 +65,12 @@ skip-if = [
 
 ["browser_commands_from_url.js"]
 
+["browser_destroying_iframes.js"]
+skip-if = [
+  "asan", # Getting random unhandled exceptions which aren't crashing DevTools
+  "tsan",
+]
+
 ["browser_devtools_api_destroy.js"]
 
 ["browser_dynamic_tool_enabling.js"]

devtools/client/framework/test/browser_destroying_iframes.js

@@ -0,0 +1,21 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+"use strict";
+
+// Testing that there's no breaking exception when destroying
+// an iframe early after its creation.
+
+add_task(async function () {
+  const { tab } = await openInspectorForURL("about:blank");
+  const browser = tab.linkedBrowser;
+
+  // Create/remove an extra one now, after the load event.
+  for (let i = 0; i < 10; i++) {
+    await SpecialPowers.spawn(browser, [], async function () {
+      const iframe = content.document.createElement("iframe");
+      content.document.body.appendChild(iframe);
+      await new Promise(res => (iframe.onload = res));
+      iframe.remove();
+    });
+  }
+});

devtools/client/inspector/shared/walker-event-listener.js

@@ -60,7 +60,17 @@ class WalkerEventListener {
   }
 
   async _onTargetAvailable({ targetFront }) {
-    const inspectorFront = await targetFront.getFront("inspector");
+    let inspectorFront;
+    try {
+      inspectorFront = await targetFront.getFront("inspector");
+    } catch (e) {
+      // When iframes are destroyed early during their creation,
+      // getFront method may throw
+      if (targetFront.isDestroyed()) {
+        return;
+      }
+      throw e;
+    }
     // In case of multiple fast navigations, the front may already be destroyed,
     // in such scenario bail out and ignore this short lived target.
     if (inspectorFront.isDestroyed() || !this._listenerMap) {

devtools/server/actors/highlighters.js

@@ -48,7 +48,7 @@ const registerHighlighter = (typeName, modulePath) => {
  * CustomHighlighterActor is a generic Actor that instantiates a custom implementation of
  * a highlighter class given its type name which must be registered in `highlighterTypes`.
  * CustomHighlighterActor proxies calls to methods of the highlighter class instance:
- * constructor(targetActor), show(node, options), hide(), destroy()
+ * constructor(nargetActor), show(node, options), hide(), destroy()
  */
 exports.CustomHighlighterActor = class CustomHighligherActor extends Actor {
   /**
@@ -308,7 +308,9 @@ class HighlighterEnvironment extends EventEmitter {
     if (this._targetActor && this._targetActor.isRootActor) {
       return this.window;
     }
-    return this.docShell && this.docShell.chromeEventHandler;
+    return (
+      this._targetActor?.chromeEventHandler || this.docShell.chromeEventHandler
+    );
   }
 
   relayTargetEvent(name, data) {

devtools/server/actors/targets/window-global.js

@@ -373,14 +373,24 @@ class WindowGlobalTargetActor extends BaseTargetActor {
       writable: true,
     });
 
-    // When this target tracks only one WindowGlobal, set a fixed innerWindowId,
+    // When this target tracks only one WindowGlobal, set a fixed innerWindowId and window,
     // so that it can easily be read safely while the related WindowGlobal is being destroyed.
     if (this.followWindowGlobalLifeCycle) {
       Object.defineProperty(this, "innerWindowId", {
         value: this.innerWindowId,
         configurable: false,
         writable: false,
       });
+      Object.defineProperty(this, "window", {
+        value: this.window,
+        configurable: false,
+        writable: false,
+      });
+      Object.defineProperty(this, "chromeEventHandler", {
+        value: this.chromeEventHandler,
+        configurable: false,
+        writable: false,
+      });
     }
 
     // Save references to the original document we attached to
@@ -461,25 +471,12 @@ class WindowGlobalTargetActor extends BaseTargetActor {
   _targetScopedActorPool = null;
 
   /**
-   * An object on which listen for DOMWindowCreated and pageshow events.
+   * A EventTarget object on which to listen for 'DOMWindowCreated' and 'pageshow' events.
    */
   get chromeEventHandler() {
     return getDocShellChromeEventHandler(this.docShell);
   }
 
-  /**
-   * Getter for the nsIMessageManager associated to the window global.
-   */
-  get messageManager() {
-    try {
-      return this.docShell.messageManager;
-    } catch (e) {
-      // In some cases we can't get a docshell.  We just have no message manager
-      // then,
-      return null;
-    }
-  }
-
   /**
    * Getter for the list of all `docShell`s in the window global.
    * @return {Array}