Bug 1591952 - Add a global try catch around grid-inspector onReflow to swallow exceptions after destroy r=gl
authorJulian Descottes <jdescottes@mozilla.com>
Mon, 04 Nov 2019 19:44:32 +0000
changeset 500432 33e81e5b64766b68ef11059c7f3c408de2760fc2
parent 500431 6b96a7fa53976ca297f06934e8b649e9f6e9afb6
child 500433 5a3b621d152d45ffc5b1f85e0129ef32ad8681b4
push id114164
push useraiakab@mozilla.com
push dateTue, 05 Nov 2019 10:06:15 +0000
treeherdermozilla-inbound@4d585c7edc76 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgl
bugs1591952
milestone72.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1591952 - Add a global try catch around grid-inspector onReflow to swallow exceptions after destroy r=gl This method is very asynchronous, called on reflows and throttled. Which means it has a high chance of intermittently failing after destroy. Differential Revision: https://phabricator.services.mozilla.com/D51663
devtools/client/inspector/grids/grid-inspector.js
--- a/devtools/client/inspector/grids/grid-inspector.js
+++ b/devtools/client/inspector/grids/grid-inspector.js
@@ -313,21 +313,16 @@ class GridInspector {
   async updateGridPanel() {
     // Stop refreshing if the inspector or store is already destroyed.
     if (!this.inspector || !this.store) {
       return;
     }
 
     const gridFronts = await this.getGrids();
 
-    // Stop if the panel has been destroyed during the call to getGrids
-    if (!this.inspector) {
-      return;
-    }
-
     if (!gridFronts.length) {
       try {
         this.store.dispatch(updateGrids([]));
         this.inspector.emit("grid-panel-updated");
         return;
       } catch (e) {
         // This call might fail if called asynchrously after the toolbox is finished
         // closing.
@@ -365,20 +360,16 @@ class GridInspector {
           nodeFront = await grid.walkerFront.getNodeFromActor(grid.actorID, [
             "containerEl",
           ]);
         } catch (e) {
           // This call might fail if called asynchrously after the toolbox is finished
           // closing.
           return;
         }
-        // Stop if the panel has been destroyed during the call getNodeFromActor
-        if (!this.inspector) {
-          return;
-        }
       }
 
       const colorForHost = customColors[hostname]
         ? customColors[hostname][i]
         : null;
       const fallbackColor = GRID_COLORS[i % GRID_COLORS.length];
       const color = this.getInitialGridColor(
         nodeFront,
@@ -539,57 +530,63 @@ class GridInspector {
    * cause the grids to change. So, we want to limit how many times we update the grid
    * panel to only reflows that actually either change the list of grids, or those that
    * change the current outlined grid.
    * To achieve this, this function compares the list of grid containers from before and
    * after the reflow, as well as the grid fragment data on the currently highlighted
    * grid.
    */
   async onReflow() {
-    if (!this.isPanelVisible()) {
-      return;
-    }
+    try {
+      if (!this.isPanelVisible()) {
+        return;
+      }
+
+      // The list of grids currently displayed.
+      const { grids } = this.store.getState();
 
-    // The list of grids currently displayed.
-    const { grids } = this.store.getState();
+      // The new list of grids from the server.
+      const newGridFronts = await this.getGrids();
 
-    // The new list of grids from the server.
-    const newGridFronts = await this.getGrids();
+      // In some cases, the nodes for current grids may have been removed from the DOM in
+      // which case we need to update.
+      if (grids.length && grids.some(grid => !grid.nodeFront.actorID)) {
+        await this.updateGridPanel(newGridFronts);
+        return;
+      }
 
-    // Stop if the panel has been destroyed during the call to getGrids
-    if (!this.inspector) {
-      return;
-    }
+      // Get the node front(s) from the current grid(s) so we can compare them to them to
+      // the node(s) of the new grids.
+      const oldNodeFronts = grids.map(grid => grid.nodeFront.actorID);
+      const newNodeFronts = newGridFronts
+        .filter(grid => grid.containerNode)
+        .map(grid => grid.containerNodeFront.actorID);
 
-    // In some cases, the nodes for current grids may have been removed from the DOM in
-    // which case we need to update.
-    if (grids.length && grids.some(grid => !grid.nodeFront.actorID)) {
-      this.updateGridPanel(newGridFronts);
-      return;
+      if (
+        grids.length === newGridFronts.length &&
+        oldNodeFronts.sort().join(",") == newNodeFronts.sort().join(",") &&
+        !this.haveCurrentFragmentsChanged(newGridFronts)
+      ) {
+        // Same list of containers and the geometry of all the displayed grids remained the
+        // same, we can safely abort.
+        return;
+      }
+
+      // Either the list of containers or the current fragments have changed, do update.
+      await this.updateGridPanel(newGridFronts);
+    } catch (e) {
+      if (!this.inspector) {
+        // onReflow/updateGridPanel are highly asynchronous and might still run
+        // after the inspector was destroyed. Swallow the error in this case.
+        console.warn("Inspector destroyed while executing onReflow callback");
+      } else {
+        // If the grid inspector was not destroyed, this is an unexpected error.
+        throw e;
+      }
     }
-
-    // Get the node front(s) from the current grid(s) so we can compare them to them to
-    // the node(s) of the new grids.
-    const oldNodeFronts = grids.map(grid => grid.nodeFront.actorID);
-    const newNodeFronts = newGridFronts
-      .filter(grid => grid.containerNode)
-      .map(grid => grid.containerNodeFront.actorID);
-
-    if (
-      grids.length === newGridFronts.length &&
-      oldNodeFronts.sort().join(",") == newNodeFronts.sort().join(",") &&
-      !this.haveCurrentFragmentsChanged(newGridFronts)
-    ) {
-      // Same list of containers and the geometry of all the displayed grids remained the
-      // same, we can safely abort.
-      return;
-    }
-
-    // Either the list of containers or the current fragments have changed, do update.
-    this.updateGridPanel(newGridFronts);
   }
 
   /**
    * Handler for a change in the grid overlay color picker for a grid container.
    *
    * @param  {NodeFront} node
    *         The NodeFront of the grid container element for which the grid color is
    *         being updated.