Bug 1463026 - Avoid unnecesarily setting the toolbox buttons when reloading the page; r?jdescottes draft
authorBrian Birtles <birtles@gmail.com>
Thu, 24 May 2018 13:13:50 +0900
changeset 799577 62aa786ed2cc27cbcffe05b91e285670bebc68f0
parent 799120 47e81ea1ef10189ef210867934bf36e14cf223dc
push id111110
push userbmo:bbirtles@mozilla.com
push dateThu, 24 May 2018 23:02:49 +0000
reviewersjdescottes
bugs1463026, 1451592
milestone62.0a1
Bug 1463026 - Avoid unnecesarily setting the toolbox buttons when reloading the page; r?jdescottes Changeset https://hg.mozilla.org/mozilla-central/rev/00c968ee4e0c from bug 1451592 introduced a DAMP regression on several reload tests. This is because upon reloading a page we get a series of frame-update messages which tell us that we no longer have any iframes only to then tell us we do have iframes after all. As a result we update the toolbar buttons to remove the frames button only to re-add it again. This causes unnecessary updates and flicker. This patch mitigates this problem by introducing debouncing during reloading/navigating so that we only update the toolbar buttons after some timeout of inactivity if there has been a change in the frames button's state. MozReview-Commit-ID: 3CpEjLfNjbC
devtools/client/framework/toolbox.js
--- a/devtools/client/framework/toolbox.js
+++ b/devtools/client/framework/toolbox.js
@@ -13,16 +13,17 @@ const DISABLE_AUTOHIDE_PREF = "ui.popup.
 const HOST_HISTOGRAM = "DEVTOOLS_TOOLBOX_HOST";
 const CURRENT_THEME_SCALAR = "devtools.current_theme";
 const HTML_NS = "http://www.w3.org/1999/xhtml";
 const REGEX_PANEL = /webconsole|inspector|jsdebugger|styleeditor|netmonitor|storage/;
 
 var {Ci, Cc} = require("chrome");
 var promise = require("promise");
 var defer = require("devtools/shared/defer");
+const { debounce } = require("devtools/shared/debounce");
 var Services = require("Services");
 var ChromeUtils = require("ChromeUtils");
 var {gDevTools} = require("devtools/client/framework/devtools");
 var EventEmitter = require("devtools/shared/event-emitter");
 var Telemetry = require("devtools/client/shared/telemetry");
 const { getUnicodeUrl } = require("devtools/client/shared/unicode-url");
 var { attachThread, detachThread } = require("./attach-thread");
 var Menu = require("devtools/client/framework/menu");
@@ -2386,18 +2387,43 @@ Toolbox.prototype = {
 
     // If non-top level frame is selected the toolbar button is
     // marked as 'checked' indicating that a child frame is active.
     if (!topFrameSelected && this.selectedFrameId) {
       this._framesButtonChecked = false;
     }
 
     // We may need to hide/show the frames button now.
+    const wasVisible = this.frameButton.isVisible;
+    const wasDisabled = this.frameButton.disabled;
     this.updateFrameButton();
-    this.component.setToolboxButtons(this.toolbarButtons);
+
+    const toolbarUpdate = () => {
+      if (this.frameButton.isVisible === wasVisible &&
+          this.frameButton.disabled === wasDisabled) {
+        return;
+      }
+      this.component.setToolboxButtons(this.toolbarButtons);
+    };
+
+    // If we are navigating/reloading, however (in which case data.destroyAll
+    // will be true), we should debounce the update to avoid unnecessary
+    // flickering/rendering.
+    if (data.destroyAll && !this.debouncedToolbarUpdate) {
+      this.debouncedToolbarUpdate = debounce(() => {
+        toolbarUpdate();
+        this.debouncedToolbarUpdate = null;
+      }, 200, this);
+    }
+
+    if (this.debouncedToolbarUpdate) {
+      this.debouncedToolbarUpdate();
+    } else {
+      toolbarUpdate();
+    }
   },
 
   /**
    * Returns a 0-based selected frame depth.
    *
    * For example, if the root frame is selected, the returned value is 0.  For a sub-frame
    * of the root document, the returned value is 1, and so on.
    */