Bug 1320149 - Prevent loading gcli when opening a toolbox. r=miker draft
authorAlexandre Poirot <poirot.alex@gmail.com>
Thu, 05 Jan 2017 10:27:31 -0800
changeset 467968 4734083583700019d3b2480cf5415db120466926
parent 467834 71224049c0b52ab190564d3ea0eab089a159a4cf
child 543817 04291f350c7ec05047ef0a06e93213fef77b1b84
push id43320
push userbmo:poirot.alex@gmail.com
push dateMon, 30 Jan 2017 14:17:26 +0000
reviewersmiker
bugs1320149
milestone54.0a1
Bug 1320149 - Prevent loading gcli when opening a toolbox. r=miker MozReview-Commit-ID: KE209SRA15u
devtools/client/definitions.js
devtools/client/framework/toolbox.js
devtools/client/inspector/inspector.js
devtools/client/locales/en-US/startup.properties
devtools/client/menus.js
devtools/client/preferences/devtools.js
devtools/client/shared/developer-toolbar.js
devtools/client/shared/test/browser_telemetry_button_paintflashing.js
devtools/shared/fronts/inspector.js
devtools/shared/gcli/commands/paintflashing.js
--- a/devtools/client/definitions.js
+++ b/devtools/client/definitions.js
@@ -19,16 +19,21 @@ loader.lazyGetter(this, "CanvasDebuggerP
 loader.lazyGetter(this, "WebAudioEditorPanel", () => require("devtools/client/webaudioeditor/panel").WebAudioEditorPanel);
 loader.lazyGetter(this, "MemoryPanel", () => require("devtools/client/memory/panel").MemoryPanel);
 loader.lazyGetter(this, "PerformancePanel", () => require("devtools/client/performance/panel").PerformancePanel);
 loader.lazyGetter(this, "NetMonitorPanel", () => require("devtools/client/netmonitor/panel").NetMonitorPanel);
 loader.lazyGetter(this, "StoragePanel", () => require("devtools/client/storage/panel").StoragePanel);
 loader.lazyGetter(this, "ScratchpadPanel", () => require("devtools/client/scratchpad/scratchpad-panel").ScratchpadPanel);
 loader.lazyGetter(this, "DomPanel", () => require("devtools/client/dom/dom-panel").DomPanel);
 
+// Other dependencies
+loader.lazyRequireGetter(this, "CommandUtils", "devtools/client/shared/developer-toolbar", true);
+loader.lazyImporter(this, "ResponsiveUIManager", "resource://devtools/client/responsivedesign/responsivedesign.jsm");
+loader.lazyImporter(this, "ScratchpadManager", "resource://devtools/client/scratchpad/scratchpad-manager.jsm");
+
 const {LocalizationHelper} = require("devtools/shared/l10n");
 const L10N = new LocalizationHelper("devtools/client/locales/startup.properties");
 
 var Tools = {};
 exports.Tools = Tools;
 
 // Definitions
 Tools.options = {
@@ -465,36 +470,103 @@ exports.defaultThemes = [
   Tools.lightTheme,
   Tools.firebugTheme,
 ];
 
 // White-list buttons that can be toggled to prevent adding prefs for
 // addons that have manually inserted toolbarbuttons into DOM.
 // (By default, supported target is only local tab)
 exports.ToolboxButtons = [
-  { id: "command-button-pick",
-    isTargetSupported: target => {
-      return target.activeTab && target.activeTab.traits.frames;
+  { id: "command-button-splitconsole",
+    description: l10n("toolbox.buttons.splitconsole"),
+    isTargetSupported: target => !target.isAddon,
+    onClick(event, toolbox) {
+      toolbox.toggleSplitConsole();
+    },
+    isChecked(toolbox) {
+      return toolbox.splitConsole;
+    },
+    setup(toolbox, onChange) {
+      toolbox.on("split-console", onChange);
+    },
+    teardown(toolbox, onChange) {
+      toolbox.off("split-console", onChange);
     }
   },
-  { id: "command-button-frames",
-    isTargetSupported: target => {
-      return target.activeTab && target.activeTab.traits.frames;
+  { id: "command-button-paintflashing",
+    description: l10n("toolbox.buttons.paintflashing"),
+    isTargetSupported: target => target.isLocalTab,
+    onClick(event, toolbox) {
+      CommandUtils.executeOnTarget(toolbox.target, "paintflashing toggle");
+    },
+    autoToggle: true
+  },
+  { id: "command-button-scratchpad",
+    description: l10n("toolbox.buttons.scratchpad"),
+    isTargetSupported: target => target.isLocalTab,
+    onClick(event, toolbox) {
+      ScratchpadManager.openScratchpad();
     }
   },
-  { id: "command-button-splitconsole",
-    isTargetSupported: target => !target.isAddon },
-  { id: "command-button-responsive" },
-  { id: "command-button-paintflashing" },
-  { id: "command-button-scratchpad" },
-  { id: "command-button-screenshot" },
-  { id: "command-button-rulers" },
-  { id: "command-button-measure" },
-  { id: "command-button-noautohide",
-    isTargetSupported: target => target.chrome },
+  { id: "command-button-responsive",
+    description: l10n("toolbox.buttons.responsive",
+                      osString == "Darwin" ? "Cmd+Opt+M" : "Ctrl+Shift+M"),
+    isTargetSupported: target => target.isLocalTab,
+    onClick(event, toolbox) {
+      let browserWindow = toolbox.win.top;
+      ResponsiveUIManager.handleGcliCommand(browserWindow,
+        browserWindow.gBrowser.selectedTab,
+        "resize toggle",
+        null);
+    },
+    isChecked(toolbox) {
+      if (!toolbox.target.tab) {
+        return false;
+      }
+      return ResponsiveUIManager.isActiveForTab(toolbox.target.tab);
+    },
+    setup(toolbox, onChange) {
+      ResponsiveUIManager.on("on", onChange);
+      ResponsiveUIManager.on("off", onChange);
+    },
+    teardown(toolbox, onChange) {
+      ResponsiveUIManager.off("on", onChange);
+      ResponsiveUIManager.off("off", onChange);
+    }
+  },
+  { id: "command-button-screenshot",
+    description: l10n("toolbox.buttons.screenshot"),
+    isTargetSupported: target => target.isLocalTab,
+    onClick(event, toolbox) {
+      // Special case for screenshot button to check for clipboard preference
+      const clipboardEnabled = Services.prefs
+        .getBoolPref("devtools.screenshot.clipboard.enabled");
+      let args = "--fullpage --file";
+      if (clipboardEnabled) {
+        args += " --clipboard";
+      }
+      CommandUtils.executeOnTarget(toolbox.target, "screenshot " + args);
+    }
+  },
+  { id: "command-button-rulers",
+    description: l10n("toolbox.buttons.rulers"),
+    isTargetSupported: target => target.isLocalTab,
+    onClick(event, toolbox) {
+      CommandUtils.executeOnTarget(toolbox.target, "rulers");
+    },
+    autoToggle: true
+  },
+  { id: "command-button-measure",
+    description: l10n("toolbox.buttons.measure"),
+    isTargetSupported: target => target.isLocalTab,
+    onClick(event, toolbox) {
+      CommandUtils.executeOnTarget(toolbox.target, "measure");
+    },
+    autoToggle: true
+  },
 ];
 
 /**
  * Lookup l10n string from a string bundle.
  *
  * @param {string} name
  *        The key to lookup.
  * @param {string} arg
--- a/devtools/client/framework/toolbox.js
+++ b/devtools/client/framework/toolbox.js
@@ -553,41 +553,83 @@ Toolbox.prototype = {
    *
    * @param {Object} options:
    *
    * @property {String} id - The id of the button or command.
    * @property {String} className - An optional additional className for the button.
    * @property {String} description - The value that will display as a tooltip and in
    *                    the options panel for enabling/disabling.
    * @property {Function} onClick - The function to run when the button is activated by
-   *                      click or keyboard shortcut.
+   *                      click or keyboard shortcut. First argument will be the 'click'
+   *                      event, and second argument is the toolbox instance.
    * @property {Boolean} isInStartContainer - Buttons can either be placed at the start
    *                     of the toolbar, or at the end.
+   * @property {Function} setup - Function run immediately to listen for events changing
+   *                      whenever the button is checked or unchecked. The toolbox object
+   *                      is passed as first argument and a callback is passed as second
+   *                       argument, to be called whenever the checked state changes.
+   * @property {Function} teardown - Function run on toolbox close to let a chance to
+   *                      unregister listeners set when `setup` was called and avoid
+   *                      memory leaks. The same arguments than `setup` function are
+   *                      passed to `teardown`.
+   * @property {Function} isTargetSupported - Function to automatically enable/disable
+   *                      the button based on the target. If the target don't support
+   *                      the button feature, this method should return false.
+   * @property {Function} isChecked - Optional function called to known if the button
+   *                      is toggled or not. The function should return true when
+   *                      the button should be displayed as toggled on.
+   * @property {Boolean}  autoToggle - If true, the checked state is going to be
+   *                      automatically toggled on click.
    */
   _createButtonState: function (options) {
-    let isChecked = false;
-    const {id, className, description, onClick, isInStartContainer} = options;
+    let isCheckedValue = false;
+    const { id, className, description, onClick, isInStartContainer, setup, teardown,
+            isTargetSupported, isChecked, autoToggle } = options;
+    const toolbox = this;
     const button = {
       id,
       className,
       description,
-      onClick,
+      onClick(event) {
+        if (typeof onClick == "function") {
+          onClick(event, toolbox);
+        }
+        if (autoToggle) {
+          button.isChecked = !button.isChecked;
+        }
+      },
+      isTargetSupported,
       get isChecked() {
-        return isChecked;
+        if (typeof isChecked == "function") {
+          return isChecked(toolbox);
+        }
+        return isCheckedValue;
       },
       set isChecked(value) {
-        isChecked = value;
+        // Note that if options.isChecked is given, this is ignored
+        isCheckedValue = value;
         this.emit("updatechecked");
       },
       // The preference for having this button visible.
       visibilityswitch: `devtools.${id}.enabled`,
       // The toolbar has a container at the start and end of the toolbar for
       // holding buttons. By default the buttons are placed in the end container.
       isInStartContainer: !!isInStartContainer
     };
+    if (typeof setup == "function") {
+      let onChange = () => {
+        button.emit("updatechecked");
+      };
+      setup(this, onChange);
+      // Save a reference to the cleanup method that will unregister the onChange
+      // callback. Immediately bind the function argument so that we don't have to
+      // also save a reference to them.
+      button.teardown = teardown.bind(options, this, onChange);
+    }
+    button.isVisible = this._commandIsVisible(button);
 
     EventEmitter.decorate(button);
 
     return button;
   },
 
   _buildOptions: function () {
     let selectOptions = (name, event) => {
@@ -1025,74 +1067,60 @@ Toolbox.prototype = {
 
     newTarget.focus();
 
     event.preventDefault();
     event.stopPropagation();
   },
 
   /**
-   * Add buttons to the UI as specified in the devtools.toolbox.toolbarSpec pref
+   * Add buttons to the UI as specified in devtools/client/definitions.js
    */
   _buildButtons: Task.async(function* () {
     // Beyond the normal preference filtering
     this.toolbarButtons = [
       this._buildPickerButton(),
       this._buildFrameButton(),
       yield this._buildNoAutoHideButton()
     ];
 
-    // Build buttons from the GCLI commands only if the GCLI actor is supported and the
-    // target isn't chrome.
-    if (this.target.hasActor("gcli") && !this.target.chrome) {
-      const options = {
-        environment: CommandUtils.createEnvironment(this, "_target")
-      };
-
-      this._requisition = yield CommandUtils.createRequisition(this.target, options);
-      const spec = this.getToolbarSpec();
-      const commandButtons = yield CommandUtils.createCommandButtons(
-        spec, this.target, this.doc, this._requisition, this._createButtonState);
-      this.toolbarButtons = [...this.toolbarButtons, ...commandButtons];
-    }
-
-    // Mutate the objects here with their visibility.
-    this.toolbarButtons.forEach(command => {
-      const definition = ToolboxButtons.find(t => t.id === command.id);
-      command.isTargetSupported = definition.isTargetSupported
-        ? definition.isTargetSupported
-        : target => target.isLocalTab;
-      command.isVisible = this._commandIsVisible(command.id);
+    ToolboxButtons.forEach(definition => {
+      let button = this._createButtonState(definition);
+      this.toolbarButtons.push(button);
     });
 
     this.component.setToolboxButtons(this.toolbarButtons);
   }),
 
   /**
    * Button to select a frame for the inspector to target.
    */
   _buildFrameButton() {
     this.frameButton = this._createButtonState({
       id: "command-button-frames",
       description: L10N.getStr("toolbox.frames.tooltip"),
-      onClick: this.showFramesMenu
+      onClick: this.showFramesMenu,
+      isTargetSupported: target => {
+        return target.activeTab && target.activeTab.traits.frames;
+      }
     });
 
     return this.frameButton;
   },
 
   /**
    * Button that disables/enables auto-hiding XUL pop-ups. When enabled, XUL
    * pop-ups will not automatically close when they lose focus.
    */
   _buildNoAutoHideButton: Task.async(function* () {
     this.autohideButton = this._createButtonState({
       id: "command-button-noautohide",
       description: L10N.getStr("toolbox.noautohide.tooltip"),
-      onClick: this._toggleNoAutohide
+      onClick: this._toggleNoAutohide,
+      isTargetSupported: target => target.chrome
     });
 
     this._isDisableAutohideEnabled().then(enabled => {
       this.autohideButton.isChecked = enabled;
     });
 
     return this.autohideButton;
   }),
@@ -1133,17 +1161,20 @@ Toolbox.prototype = {
    * The element picker button enables the ability to select a DOM node by clicking
    * it on the page.
    */
   _buildPickerButton() {
     this.pickerButton = this._createButtonState({
       id: "command-button-pick",
       description: L10N.getStr("pickButton.tooltip"),
       onClick: this._onPickerClick,
-      isInStartContainer: true
+      isInStartContainer: true,
+      isTargetSupported: target => {
+        return target.activeTab && target.activeTab.traits.frames;
+      }
     });
 
     return this.pickerButton;
   },
 
   /**
    * Apply the current cache setting from devtools.cache.disabled to this
    * toolbox's tab.
@@ -1173,51 +1204,42 @@ Toolbox.prototype = {
     }
   },
 
  /**
   * Get the toolbar spec for toolbox
   */
   getToolbarSpec: function () {
     let spec = CommandUtils.getCommandbarSpec("devtools.toolbox.toolbarSpec");
-    // Special case for screenshot command button to check for clipboard preference
-    const clipboardEnabled = Services.prefs
-      .getBoolPref("devtools.screenshot.clipboard.enabled");
-    if (clipboardEnabled) {
-      for (let i = 0; i < spec.length; i++) {
-        if (spec[i] == "screenshot --fullpage --file") {
-          spec[i] += " --clipboard";
-        }
-      }
-    }
+
     return spec;
   },
 
  /**
   * Return all toolbox buttons (command buttons, plus any others that were
   * added manually).
 
   /**
    * Update the visibility of the buttons.
    */
   updateToolboxButtonsVisibility() {
-    this.toolbarButtons.forEach(command => {
-      command.isVisible = this._commandIsVisible(command.id);
+    this.toolbarButtons.forEach(button => {
+      button.isVisible = this._commandIsVisible(button);
     });
     this.component.setToolboxButtons(this.toolbarButtons);
   },
 
   /**
    * Ensure the visibility of each toolbox button matches the preference value.
    */
-  _commandIsVisible: function (id) {
+  _commandIsVisible: function (button) {
     const {
       isTargetSupported,
       visibilityswitch
-    } = this.toolbarButtons.find(btn => btn.id === id);
+    } = button;
 
     let visible = true;
     try {
       visible = Services.prefs.getBoolPref(visibilityswitch);
     } catch (ex) {
       // Do nothing.
     }
 
@@ -2313,22 +2335,27 @@ Toolbox.prototype = {
 
     // Destroy the preference front
     outstanding.push(this.destroyPreference());
 
     // Detach the thread
     detachThread(this._threadClient);
     this._threadClient = null;
 
+    // Unregister buttons listeners
+    this.toolbarButtons.forEach(button => {
+      if (typeof button.teardown == "function") {
+        // teardown arguments have already been bound in _createButtonState
+        button.teardown();
+      }
+    });
+
     // We need to grab a reference to win before this._host is destroyed.
     let win = this.win;
 
-    if (this._requisition) {
-      CommandUtils.destroyRequisition(this._requisition, this.target);
-    }
     this._telemetry.toolClosed("toolbox");
     this._telemetry.destroy();
 
     // Finish all outstanding tasks (which means finish destroying panels and
     // then destroying the host, successfully or not) before destroying the
     // target.
     deferred.resolve(settleAll(outstanding)
         .catch(console.error)
--- a/devtools/client/inspector/inspector.js
+++ b/devtools/client/inspector/inspector.js
@@ -1750,24 +1750,21 @@ Inspector.prototype = {
 
   /**
    * Initiate gcli screenshot command on selected node.
    */
   screenshotNode: function () {
     const command = Services.prefs.getBoolPref("devtools.screenshot.clipboard.enabled") ?
       "screenshot --file --clipboard --selector" :
       "screenshot --file --selector";
-    CommandUtils.createRequisition(this._target, {
-      environment: CommandUtils.createEnvironment(this, "_target")
-    }).then(requisition => {
-      // Bug 1180314 -  CssSelector might contain white space so need to make sure it is
-      // passed to screenshot as a single parameter.  More work *might* be needed if
-      // CssSelector could contain escaped single- or double-quotes, backslashes, etc.
-      requisition.updateExec(`${command} '${this.selectionCssSelector}'`);
-    });
+    // Bug 1180314 -  CssSelector might contain white space so need to make sure it is
+    // passed to screenshot as a single parameter.  More work *might* be needed if
+    // CssSelector could contain escaped single- or double-quotes, backslashes, etc.
+    CommandUtils.executeOnTarget(this._target,
+      `${command} '${this.selectionCssSelector}'`);
   },
 
   /**
    * Scroll the node into view.
    */
   scrollNodeIntoView: function () {
     if (!this.selection.isNode()) {
       return;
--- a/devtools/client/locales/en-US/startup.properties
+++ b/devtools/client/locales/en-US/startup.properties
@@ -255,8 +255,45 @@ dom.panelLabel=DOM Panel
 dom.commandkey=W
 dom.accesskey=D
 
 # LOCALIZATION NOTE (dom.tooltip):
 # This string is displayed in the tooltip of the tab when the DOM is
 # displayed inside the developer tools window.
 # Keyboard shortcut for DOM panel will be shown inside the brackets.
 dom.tooltip=DOM (%S)
+
+# LOCALIZATION NOTE (toolbox.buttons.splitconsole):
+# This is the tooltip of the button in the toolbox toolbar used to toggle
+# the split console.
+# Keyboard shortcut will be shown inside brackets.
+toolbox.buttons.splitconsole = Toggle split console (%S)
+
+# LOCALIZATION NOTE (toolbox.buttons.responsive):
+# This is the tooltip of the button in the toolbox toolbar that toggles
+# the Responsive mode.
+# Keyboard shortcut will be shown inside brackets.
+toolbox.buttons.responsive = Responsive Design Mode (%S)
+
+# LOCALIZATION NOTE (toolbox.buttons.paintflashing):
+# This is the tooltip of the paintflashing button in the toolbox toolbar
+# that toggles paintflashing.
+toolbox.buttons.paintflashing = Toggle paint flashing
+
+# LOCALIZATION NOTE (toolbox.buttons.scratchpad):
+# This is the tooltip of the button in the toolbox toolbar that opens
+# the scratchpad window
+toolbox.buttons.scratchpad = Scratchpad
+
+# LOCALIZATION NOTE (toolbox.buttons.screenshot):
+# This is the tooltip of the button in the toolbox toolbar that allows you to
+# take a screenshot of the entire page
+toolbox.buttons.screenshot = Take a screenshot of the entire page
+
+# LOCALIZATION NOTE (toolbox.buttons.rulers):
+# This is the tooltip of the button in the toolbox toolbar that toggles the
+# rulers in the page
+toolbox.buttons.rulers = Toggle rulers for the page
+
+# LOCALIZATION NOTE (toolbox.buttons.measure):
+# This is the tooltip of the button in the toolbox toolbar that toggles the
+# measuring tools
+toolbox.buttons.measure = Measure a portion of the page
--- a/devtools/client/menus.js
+++ b/devtools/client/menus.js
@@ -143,21 +143,17 @@ exports.menuitems = [
     checkbox: true
   },
   { id: "menu_eyedropper",
     l10nKey: "eyedropper",
     oncommand(event) {
       let window = event.target.ownerDocument.defaultView;
       let target = TargetFactory.forTab(window.gBrowser.selectedTab);
 
-      CommandUtils.createRequisition(target, {
-        environment: CommandUtils.createEnvironment({target})
-      }).then(requisition => {
-        requisition.updateExec("eyedropper --frommenu");
-      }, e => console.error(e));
+      CommandUtils.executeOnTarget(target, "eyedropper --frommenu");
     },
     checkbox: true
   },
   { id: "menu_scratchpad",
     l10nKey: "scratchpad",
     oncommand() {
       ScratchpadManager.openScratchpad();
     },
--- a/devtools/client/preferences/devtools.js
+++ b/devtools/client/preferences/devtools.js
@@ -25,17 +25,16 @@ pref("devtools.toolbar.visible", false);
 pref("devtools.webide.enabled", true);
 
 // Toolbox preferences
 pref("devtools.toolbox.footer.height", 250);
 pref("devtools.toolbox.sidebar.width", 500);
 pref("devtools.toolbox.host", "bottom");
 pref("devtools.toolbox.previousHost", "side");
 pref("devtools.toolbox.selectedTool", "webconsole");
-pref("devtools.toolbox.toolbarSpec", '["splitconsole", "paintflashing toggle","scratchpad","resize toggle","screenshot --fullpage --file", "rulers", "measure"]');
 pref("devtools.toolbox.sideEnabled", true);
 pref("devtools.toolbox.zoomValue", "1");
 pref("devtools.toolbox.splitconsoleEnabled", false);
 pref("devtools.toolbox.splitconsoleHeight", 100);
 
 // Toolbox Button preferences
 pref("devtools.command-button-pick.enabled", true);
 pref("devtools.command-button-frames.enabled", true);
--- a/devtools/client/shared/developer-toolbar.js
+++ b/devtools/client/shared/developer-toolbar.js
@@ -31,16 +31,38 @@ loader.lazyRequireGetter(this, "gDevTool
 loader.lazyRequireGetter(this, "nodeConstants", "devtools/shared/dom-node-constants");
 loader.lazyRequireGetter(this, "EventEmitter", "devtools/shared/event-emitter");
 
 /**
  * A collection of utilities to help working with commands
  */
 var CommandUtils = {
   /**
+   * Caches requisitions created when calling executeOnTarget:
+   * Target => Requisition Promise
+   */
+  _requisitions: new WeakMap(),
+
+  /**
+   * Utility to execute a command string on a given target
+   */
+  executeOnTarget: Task.async(function* (target, command) {
+    let requisitionPromise = this._requisitions.get(target);
+    if (!requisitionPromise) {
+      requisitionPromise = this.createRequisition(target, {
+        environment: CommandUtils.createEnvironment({ target }, "target")
+      });
+      // Store the promise to avoid races by storing the promise immediately
+      this._requisitions.set(target, requisitionPromise);
+    }
+    let requisition = yield requisitionPromise;
+    requisition.updateExec(command);
+  }),
+
+  /**
    * Utility to ensure that things are loaded in the correct order
    */
   createRequisition: function (target, options) {
     if (!gcliInit) {
       return promise.reject("Unable to load gcli");
     }
     return gcliInit.getSystem(target).then(system => {
       let Requisition = require("gcli/cli").Requisition;
--- a/devtools/client/shared/test/browser_telemetry_button_paintflashing.js
+++ b/devtools/client/shared/test/browser_telemetry_button_paintflashing.js
@@ -39,26 +39,23 @@ function* testButton(toolbox, Telemetry)
 
 function* delayedClicks(toolbox, node, clicks) {
   for (let i = 0; i < clicks; i++) {
     yield new Promise(resolve => {
       // See TOOL_DELAY for why we need setTimeout here
       setTimeout(() => resolve(), TOOL_DELAY);
     });
 
-    // this event will fire once the command execution starts and
-    // the output object is created
-    let clicked = toolbox._requisition.commandOutputManager.onOutput.once();
+    let PaintFlashingCmd = require("devtools/shared/gcli/commands/paintflashing");
+    let clicked = PaintFlashingCmd.eventEmitter.once("changed");
 
     info("Clicking button " + node.id);
     node.click();
 
-    let outputEvent = yield clicked;
-    // promise gets resolved once execution finishes and output is ready
-    yield outputEvent.output.promise;
+    yield clicked;
   }
 }
 
 function checkResults(histIdFocus, Telemetry) {
   let result = Telemetry.prototype.telemetryInfo;
 
   for (let [histId, value] of Object.entries(result)) {
     if (histId.startsWith("DEVTOOLS_INSPECTOR_") ||
--- a/devtools/shared/fronts/inspector.js
+++ b/devtools/shared/fronts/inspector.js
@@ -986,21 +986,17 @@ var InspectorFront = FrontClassWithSpec(
   }, {
     impl: "_getPageStyle"
   }),
 
   pickColorFromPage: custom(Task.async(function* (toolbox, options) {
     if (toolbox) {
       // If the eyedropper was already started using the gcli command, hide it so we don't
       // end up with 2 instances of the eyedropper on the page.
-      let {target} = toolbox;
-      let requisition = yield CommandUtils.createRequisition(target, {
-        environment: CommandUtils.createEnvironment({target})
-      });
-      yield requisition.updateExec("eyedropper --hide");
+      CommandUtils.executeOnTarget(toolbox.target, "eyedropper --hide");
     }
 
     yield this._pickColorFromPage(options);
   }), {
     impl: "_pickColorFromPage"
   })
 });
 
--- a/devtools/shared/gcli/commands/paintflashing.js
+++ b/devtools/shared/gcli/commands/paintflashing.js
@@ -14,16 +14,19 @@ try {
   telemetry = new Telemetry();
 } catch (e) {
   // DevTools Telemetry module only available in Firefox
 }
 
 const EventEmitter = require("devtools/shared/event-emitter");
 const eventEmitter = new EventEmitter();
 
+// exports the event emitter to help test know when this command is toggled
+exports.eventEmitter = eventEmitter;
+
 const gcli = require("gcli/index");
 const l10n = require("gcli/l10n");
 
 const enabledPaintFlashing = new Set();
 
 const isCheckedFor = (tab) =>
   tab ? enabledPaintFlashing.has(getBrowserForTab(tab).outerWindowID) : false;