From 556346ac758e09e89a713f5b40c594ca12d07a82 Mon Sep 17 00:00:00 2001
From: Matteo Ferretti <mferretti@mozilla.com>
Date: Sat, 4 Mar 2017 12:27:08 +0100
Subject: [PATCH] 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

--HG--
extra : rebase_source : 14cc537432ef85b5d2ba758e8e48dff02bb4805f
---
 devtools/client/shared/developer-toolbar.js | 84 ++++++++++++++-------
 1 file changed, 55 insertions(+), 29 deletions(-)

diff --git a/devtools/client/shared/developer-toolbar.js b/devtools/client/shared/developer-toolbar.js
index 52b3a578e9cb..db6fae81db93 100644
--- a/devtools/client/shared/developer-toolbar.js
+++ b/devtools/client/shared/developer-toolbar.js
@@ -150,6 +150,16 @@ function DeveloperToolbar(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();
@@ -288,13 +298,35 @@ DeveloperToolbar.prototype.toggle = function () {
 
 /**
  * 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);
 };
 
@@ -330,7 +362,12 @@ 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;
   }
 
@@ -430,25 +467,9 @@ DeveloperToolbar.prototype.show = function (focus) {
     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");
@@ -457,8 +478,6 @@ DeveloperToolbar.prototype.show = function (focus) {
                            this.outputPanel);
       DeveloperToolbar.introShownThisSession = true;
     }
-
-    this._showPromise = null;
   }).bind(this));
 
   return this._showPromise;
@@ -468,15 +487,19 @@ DeveloperToolbar.prototype.show = function (focus) {
  * 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;
   }
 
-  // show() is async, so ensure we don't need to wait for show() to finish
-  let waitPromise = this._showPromise || promise.resolve();
+  // If `_showPromise` is `null`, it means `show` method was never called, so just
+  // returns a resolved promise.
+  if (this._showPromise === null) {
+    return promise.resolve();
+  }
 
-  this._hidePromise = waitPromise.then(() => {
+  // show() is async, so ensure we don't need to wait for show() to finish
+  this._hidePromise = this._showPromise.then(() => {
     this._element.hidden = true;
 
     Services.prefs.setBoolPref("devtools.toolbar.visible", false);
@@ -487,6 +510,9 @@ DeveloperToolbar.prototype.hide = function () {
     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;
   });