Bug 1340553 - The rulers are not dismiss if using the 'rulers' command in Developer Toolbar; r=jwalker draft
authorMatteo Ferretti <mferretti@mozilla.com>
Sat, 04 Mar 2017 12:27:08 +0100
changeset 493518 a43836857c1070c30e4f45135ff440a36b0e445a
parent 491973 c807c80d954a7761c3aae2036d3a1c4d3b55ba51
child 547881 3a1c3794777221effd58b770fc481af4dce63fd0
push id47786
push userbmo:zer0@mozilla.com
push dateSat, 04 Mar 2017 11:27:56 +0000
reviewersjwalker
bugs1340553
milestone54.0a1
Bug 1340553 - The rulers are not dismiss if using the 'rulers' command in Developer Toolbar; r=jwalker - Fixed the developer-toolbar's show method to avoid re-creating new objects and listeners every call. MozReview-Commit-ID: CX5tEIIWm
devtools/client/shared/developer-toolbar.js
--- a/devtools/client/shared/developer-toolbar.js
+++ b/devtools/client/shared/developer-toolbar.js
@@ -145,16 +145,26 @@ loader.lazyGetter(this, "isMac", functio
  * @param chromeWindow The browser window to which this toolbar is attached
  */
 function DeveloperToolbar(chromeWindow) {
   this._chromeWindow = chromeWindow;
 
   // Will be setup when show() is called
   this.target = null;
 
+  // The `_showPromise` will be set once `show` is called the first time, and resolved
+  // when the toolbar is shown. Since it will be set to `null` only when `hide` method
+  // is called, multiple calls to `show` method will returns this promise instead of
+  // process again all the initialization.
+  this._showPromise = null;
+  // The `_hidePromise` will be set once `hide` is called, and resolved when the method
+  // has finished. Once the toolbar is hidden, both `_showPromise` and `_hidePromise`
+  // will be set to `null`.
+  this._hidePromise = null;
+
   this._doc = chromeWindow.document;
 
   this._telemetry = new Telemetry();
   this._errorsCount = {};
   this._warningsCount = {};
   this._errorListeners = {};
 
   this._onToolboxReady = this._onToolboxReady.bind(this);
@@ -283,23 +293,45 @@ DeveloperToolbar.prototype.toggle = func
   if (this.visible) {
     return this.hide().catch(console.error);
   }
   return this.show(true).catch(console.error);
 };
 
 /**
  * Called from browser.xul in response to menu-click or keyboard shortcut to
- * toggle the toolbar
+ * toggle the toolbar.
+ * The method returns a promise that would be resolved once focused; if the toolbar is not
+ * visible yet it will be automatically shown.
  */
 DeveloperToolbar.prototype.focus = function () {
   if (this.visible) {
-    this._input.focus();
-    return promise.resolve();
+    // If the toolbar was just inserted, the <textbox> may still have
+    // its binding in process of being applied and not be focusable yet
+    let waitForBinding = defer();
+
+    let checkBinding = () => {
+      // Bail out if the toolbar has been destroyed in the meantime
+      if (!this._input) {
+        waitForBinding.reject();
+        return;
+      }
+      // mInputField is a xbl field of <xul:textbox>
+      if (typeof this._input.mInputField != "undefined") {
+        this._input.focus();
+        waitForBinding.resolve();
+      } else {
+        this._input.ownerDocument.defaultView.setTimeout(checkBinding, 50);
+      }
+    };
+    checkBinding();
+
+    return waitForBinding.promise;
   }
+
   return this.show(true);
 };
 
 /**
  * Called from browser.xul in response to menu-click or keyboard shortcut to
  * toggle the toolbar
  */
 DeveloperToolbar.prototype.focusToggle = function () {
@@ -325,17 +357,22 @@ DeveloperToolbar.prototype.focusToggle =
  * same as this.DeveloperToolbar when in browser.js context.
  */
 DeveloperToolbar.introShownThisSession = false;
 
 /**
  * Show the developer toolbar
  */
 DeveloperToolbar.prototype.show = function (focus) {
-  if (this._showPromise != null) {
+  // if `_showPromise` is set, just returns it instead of process all the initialization
+  // again; ensuring we're focusing the element too if `focus` argument is set to `true`.
+  if (this._showPromise !== null) {
+    if (focus) {
+      return this.focus();
+    }
     return this._showPromise;
   }
 
   this._showPromise = Task.spawn((function* () {
     // hide() is async, so ensure we don't need to wait for hide() to
     // finish.  We unconditionally yield here, even if _hidePromise is
     // null, so that the spawn call returns a promise before starting
     // to do any real work.
@@ -425,73 +462,62 @@ DeveloperToolbar.prototype.show = functi
     gDevTools.on("toolbox-ready", this._onToolboxReady);
     gDevTools.on("toolbox-destroyed", this._onToolboxDestroyed);
 
     this._initErrorsCount(tabbrowser.selectedTab);
 
     this._element.hidden = false;
 
     if (focus) {
-      // If the toolbar was just inserted, the <textbox> may still have
-      // its binding in process of being applied and not be focusable yet
-      let waitForBinding = () => {
-        // Bail out if the toolbar has been destroyed in the meantime
-        if (!this._input) {
-          return;
-        }
-        // mInputField is a xbl field of <xul:textbox>
-        if (typeof this._input.mInputField != "undefined") {
-          this._input.focus();
-          this._notify(NOTIFICATIONS.SHOW);
-        } else {
-          this._input.ownerDocument.defaultView.setTimeout(waitForBinding, 50);
-        }
-      };
-      waitForBinding();
-    } else {
-      this._notify(NOTIFICATIONS.SHOW);
+      yield this.focus();
     }
+    this._notify(NOTIFICATIONS.SHOW);
 
     if (!DeveloperToolbar.introShownThisSession) {
       let intro = require("gcli/ui/intro");
       intro.maybeShowIntro(this.requisition.commandOutputManager,
                            this.requisition.conversionContext,
                            this.outputPanel);
       DeveloperToolbar.introShownThisSession = true;
     }
-
-    this._showPromise = null;
   }).bind(this));
 
   return this._showPromise;
 };
 
 /**
  * Hide the developer toolbar.
  */
 DeveloperToolbar.prototype.hide = function () {
-  // If we're already in the process of hiding, just use the other promise
-  if (this._hidePromise != null) {
+  // If we're already in the process of hiding, just returns the promise
+  if (this._hidePromise !== null) {
     return this._hidePromise;
   }
 
+  // If `_showPromise` is `null`, it means `show` method was never called, so just
+  // returns a resolved promise.
+  if (this._showPromise === null) {
+    return promise.resolve();
+  }
+
   // show() is async, so ensure we don't need to wait for show() to finish
-  let waitPromise = this._showPromise || promise.resolve();
-
-  this._hidePromise = waitPromise.then(() => {
+  this._hidePromise = this._showPromise.then(() => {
     this._element.hidden = true;
 
     Services.prefs.setBoolPref("devtools.toolbar.visible", false);
 
     this._doc.getElementById("menu_devToolbar").setAttribute("checked", "false");
     this.destroy();
 
     this._telemetry.toolClosed("developertoolbar");
     this._notify(NOTIFICATIONS.HIDE);
 
+    // The developer toolbar is now closed, is neither shown or in process of hiding,
+    // so we're set to `null` both `_showPromise` and `_hidePromise`.
+    this._showPromise = null;
     this._hidePromise = null;
   });
 
   return this._hidePromise;
 };
 
 /**
  * Initialize the listeners needed for tracking the number of errors for a given