Bug 1463026 - Avoid unnecesarily setting the toolbox buttons when reloading the page; r=jryans a=RyanVM
authorBrian Birtles <birtles@gmail.com>
Thu, 24 May 2018 13:13:50 +0900
changeset 473479 1092f6f0d08d1d2450861a6de4770e73f3f9c6e8
parent 473478 042445c33dc3549e23ab24260b41557c17045bf3
child 473480 17fdd7dd7ba3a4c25de8adb311b4c3305389d12d
push id1728
push userjlund@mozilla.com
push dateMon, 18 Jun 2018 21:12:27 +0000
treeherdermozilla-release@c296fde26f5f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjryans, RyanVM
bugs1463026, 1451592
milestone61.0
Bug 1463026 - Avoid unnecesarily setting the toolbox buttons when reloading the page; r=jryans a=RyanVM 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
@@ -14,16 +14,17 @@ const HOST_HISTOGRAM = "DEVTOOLS_TOOLBOX
 const SCREENSIZE_HISTOGRAM = "DEVTOOLS_SCREEN_RESOLUTION_ENUMERATED_PER_USER";
 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");
@@ -2396,18 +2397,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.
    */