Bug 1441113 - Can only update a mounted or mounting component when closing Memory Tool r?nchevobbe draft
authorMichael Ratcliffe <mratcliffe@mozilla.com>
Mon, 26 Feb 2018 11:59:23 +0000
changeset 759679 e74a3120c47f890790a8427cb39894bbffa9a98b
parent 759663 02aa9c921aedfd0e768a92a6a8c5cba1b14191c1
push id100427
push userbmo:mratcliffe@mozilla.com
push dateMon, 26 Feb 2018 12:00:02 +0000
reviewersnchevobbe
bugs1441113
milestone60.0a1
Bug 1441113 - Can only update a mounted or mounting component when closing Memory Tool r?nchevobbe The onclick handler of the toolbox close button looked like this... spot the deliberate mistake: ``` onClick: () => { closeToolbox(); focusButton(closeButtonId); }, ``` So we were closing the toolbox and then trying to focus the toolbox's close button. There is also an obvious race condition with setState inside the toolbox controller not using a callback even though the next line calls a function that uses this.state, which could cause intermittent issues. MozReview-Commit-ID: 9VRcZw4RvE5
devtools/client/framework/components/toolbox-controller.js
devtools/client/framework/components/toolbox-toolbar.js
--- a/devtools/client/framework/components/toolbox-controller.js
+++ b/devtools/client/framework/components/toolbox-controller.js
@@ -97,29 +97,28 @@ class ToolboxController extends Componen
     if (this.state.focusedButton !== focusedButton) {
       this.setState({
         focusedButton
       });
     }
   }
 
   setCurrentToolId(currentToolId) {
-    this.setState({currentToolId});
-    // Also set the currently focused button to this tool.
-    this.setFocusedButton(currentToolId);
+    this.setState({currentToolId}, () => {
+      // Also set the currently focused button to this tool.
+      this.setFocusedButton(currentToolId);
+    });
   }
 
   setCanRender() {
-    this.setState({ canRender: true });
-    this.updateButtonIds();
+    this.setState({ canRender: true }, this.updateButtonIds);
   }
 
   setOptionsPanel(optionsPanel) {
-    this.setState({ optionsPanel });
-    this.updateButtonIds();
+    this.setState({ optionsPanel }, this.updateButtonIds);
   }
 
   highlightTool(highlightedTool) {
     let { highlightedTools } = this.state;
     highlightedTools.add(highlightedTool);
     this.setState({ highlightedTools });
   }
 
@@ -127,50 +126,45 @@ class ToolboxController extends Componen
     let { highlightedTools } = this.state;
     if (highlightedTools.has(id)) {
       highlightedTools.delete(id);
       this.setState({ highlightedTools });
     }
   }
 
   setDockButtonsEnabled(areDockButtonsEnabled) {
-    this.setState({ areDockButtonsEnabled });
-    this.updateButtonIds();
+    this.setState({ areDockButtonsEnabled }, this.updateButtonIds);
   }
 
   setHostTypes(hostTypes) {
-    this.setState({ hostTypes });
-    this.updateButtonIds();
+    this.setState({ hostTypes }, this.updateButtonIds);
   }
 
   setCanCloseToolbox(canCloseToolbox) {
-    this.setState({ canCloseToolbox });
-    this.updateButtonIds();
+    this.setState({ canCloseToolbox }, this.updateButtonIds);
   }
 
   setPanelDefinitions(panelDefinitions) {
-    this.setState({ panelDefinitions });
-    this.updateButtonIds();
+    this.setState({ panelDefinitions }, this.updateButtonIds);
   }
 
   get panelDefinitions() {
     return this.state.panelDefinitions;
   }
 
   setToolboxButtons(toolboxButtons) {
     // Listen for updates of the checked attribute.
     this.state.toolboxButtons.forEach(button => {
       button.off("updatechecked", this.state.checkedButtonsUpdated);
     });
     toolboxButtons.forEach(button => {
       button.on("updatechecked", this.state.checkedButtonsUpdated);
     });
 
-    this.setState({ toolboxButtons });
-    this.updateButtonIds();
+    this.setState({ toolboxButtons }, this.updateButtonIds);
   }
 
   setCanMinimize(canMinimize) {
     /* Bug 1177463 - The minimize button is currently hidden until we agree on
        the UI for it, and until bug 1173849 is fixed too. */
 
     // this.setState({ canMinimize });
   }
--- a/devtools/client/framework/components/toolbox-toolbar.js
+++ b/devtools/client/framework/components/toolbox-toolbar.js
@@ -225,17 +225,16 @@ function renderDockButtons(props) {
   const closeButton = canCloseToolbox
     ? button({
       id: closeButtonId,
       onFocus: () => focusButton(closeButtonId),
       className: "devtools-button",
       title: L10N.getStr("toolbox.closebutton.tooltip"),
       onClick: () => {
         closeToolbox();
-        focusButton(closeButtonId);
       },
       tabIndex: focusedButton === "toolbox-close" ? "0" : "-1",
     })
     : null;
 
   return div({id: "toolbox-controls"},
     div({id: "toolbox-dock-buttons"}, ...buttons),
     closeButton