Bug 1354679 - Show the paused debugger overlay when script gets paused; r=ochameau r=pbro draft
authoryulia <ystartsev@mozilla.com>
Fri, 17 Nov 2017 20:25:04 +0100
changeset 700526 72ff0c02fcdd774a5da5390cad278e970467f51e
parent 675508 53bbdaaa2b8c1819061be26101b075c081b23260
child 740915 7ea916b0810dcf103a25a986d179e78d75b34240
push id89879
push userbmo:ystartsev@mozilla.com
push dateMon, 20 Nov 2017 09:50:04 +0000
reviewersochameau, pbro
bugs1354679
milestone58.0a1
Bug 1354679 - Show the paused debugger overlay when script gets paused; r=ochameau r=pbro Collapsed revision * BASEWIP MozReview-Commit-ID: HwxqLjeDtYu * Bug 1354679 - Show the paused debugger overlay when script gets paused; r=ochameau A previous bug had introduced a new overlay (highlighter) that was aimed at being shown when the debugger would pause. This commit adds the necessary code to the ThreadActor for the overlay to be shown when script execution is paused, and hidden when script execution resumes. This commit also adds a test to cover this feature. MozReview-Commit-ID: 3LT5MbJgbYq * Bug 1354679 - Show the paused debugger overlay when script gets paused; r=ochameau MozReview-Commit-ID: FhKLGEtocTl * Bug 1354679 - Show the paused debugger overlay when script gets paused; r=ochameau r=pbro MozReview-Commit-ID: JZkXtGJ2YIx * Bug 1354679 - Use highlighter actor from client instead of from the server MozReview-Commit-ID: J8DEwNJgpE9
FAKE.txt
devtools/client/framework/attach-thread.js
devtools/client/framework/toolbox-highlighter-utils.js
devtools/client/locales/en-US/debugger.properties
devtools/server/actors/highlighters/paused-debugger.js
devtools/server/actors/script.js
devtools/server/tests/unit/test_pausedOverlay.js
devtools/server/tests/unit/xpcshell.ini
devtools/shared/client/thread-client.js
new file mode 100644
--- a/devtools/client/framework/attach-thread.js
+++ b/devtools/client/framework/attach-thread.js
@@ -5,41 +5,50 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 const {Cc, Ci, Cu} = require("chrome");
 const Services = require("Services");
 const defer = require("devtools/shared/defer");
 
 const {LocalizationHelper} = require("devtools/shared/l10n");
 const L10N = new LocalizationHelper("devtools/client/locales/toolbox.properties");
+const debuggerL10N = new LocalizationHelper("devtools/client/locales/debugger.properties");
 
 function handleThreadState(toolbox, event, packet) {
   // Suppress interrupted events by default because the thread is
   // paused/resumed a lot for various actions.
   if (event !== "paused" || packet.why.type !== "interrupted") {
     // TODO: Bug 1225492, we continue emitting events on the target
     // like we used to, but we should emit these only on the
     // threadClient now.
     toolbox.target.emit("thread-" + event);
   }
 
   if (event === "paused") {
     toolbox.highlightTool("jsdebugger");
 
     if (packet.why.type === "debuggerStatement" ||
        packet.why.type === "breakpoint" ||
-       packet.why.type === "exception") {
+       packet.why.type === "exception")
+    {
       toolbox.raise();
       toolbox.selectTool("jsdebugger");
+
+      // TODO: move this to the frontend so we can make use of the paused reason
+      // directly
+      const reason = debuggerL10N.getStr("defaultPausedReason");
+      toolbox.highlighterUtils.showPausedOverlay(reason);
     }
   } else if (event === "resumed") {
+    toolbox.highlighterUtils.hidePausedOverlay();
     toolbox.unhighlightTool("jsdebugger");
   }
 }
 
+
 function attachThread(toolbox) {
   let deferred = defer();
 
   let target = toolbox.target;
   let { form: { chromeDebugger, actor } } = target;
 
   // Sourcemaps are always turned off when using the new debugger
   // frontend. This is because it does sourcemapping on the
--- a/devtools/client/framework/toolbox-highlighter-utils.js
+++ b/devtools/client/framework/toolbox-highlighter-utils.js
@@ -84,16 +84,53 @@ exports.getHighlighterUtils = function (
         yield toolbox.initInspector();
         isInspectorInitialized = true;
       }
       return yield generator.apply(null, args);
     });
   };
 
   /**
+   * Show/hide the paused overlays for a collection of tabs
+   * @param {TabActor} tabActor - Optionally focus the content area once the picker is
+   */
+
+  /*
+   * Manage a collection of PausedDebuggerOverlay highlighters and their HighlighterEnvironments indexed
+   * by TabActors (export it so it the paused state can be tested).
+   */
+  const pausedStateOverlays = new Map();
+
+  async function _getPausedStateOverlay(tabActor) {
+    if (pausedStateOverlays.has(tabActor)) {
+      return pausedStateOverlays.get(tabActor).overlay;
+    }
+    const overlay = await exported.getHighlighterByType("PausedDebuggerOverlay");
+    pausedStateOverlays.set(tabActor, { overlay });
+
+    return overlay;
+  }
+
+  exported.showPausedOverlay = async function (reason) {
+    let overlay = await _getPausedStateOverlay(target.activeTab);
+    // get the root node for the tab, where the highlighter will be displayed
+    let nodeFront = await toolbox.inspector.walker.getRootNode();
+    overlay.show(nodeFront, {reason});
+  };
+
+  exported.hidePausedOverlay = function () {
+    const overlayAndEnvironment = pausedStateOverlays.get(target.activeTab);
+    if (!overlayAndEnvironment) {
+      return;
+    }
+
+    overlayAndEnvironment.overlay.hide();
+  };
+
+  /**
    * Start/stop the element picker on the debuggee target.
    * @param {Boolean} doFocus - Optionally focus the content area once the picker is
    *                            activated.
    * @return A promise that resolves when done
    */
   exported.togglePicker = function (doFocus) {
     if (isPicking) {
       return cancelPicker();
--- a/devtools/client/locales/en-US/debugger.properties
+++ b/devtools/client/locales/en-US/debugger.properties
@@ -543,16 +543,21 @@ ignoreExceptions=Ignore exceptions. Clic
 # LOCALIZATION NOTE (pauseOnUncaughtExceptions): The pause on exceptions button
 # tooltip when the debugger will pause on uncaught exceptions.
 pauseOnUncaughtExceptions=Pause on uncaught exceptions. Click to pause on all exceptions
 
 # LOCALIZATION NOTE (pauseOnExceptions): The pause on exceptions button tooltip
 # when the debugger will pause on all exceptions.
 pauseOnExceptions=Pause on all exceptions. Click to ignore exceptions
 
+# LOCALIZATION NOTE (defaultPausedReason): Text displayed inside the content page, in an
+# overlay, when the debugger is paused (so script execution is paused), explaining to the
+# user why the page is paused.
+defaultPausedReason=Paused in debugger
+
 # LOCALIZATION NOTE (loadingText): The text that is displayed in the script
 # editor when the loading process has started but there is no file to display
 # yet.
 loadingText=Loading\u2026
 
 # LOCALIZATION NOTE (errorLoadingText3): The text that is displayed in the debugger
 # viewer when there is an error loading a file
 errorLoadingText3=Error loading this URI: %S
--- a/devtools/server/actors/highlighters/paused-debugger.js
+++ b/devtools/server/actors/highlighters/paused-debugger.js
@@ -71,17 +71,35 @@ PausedDebuggerOverlay.prototype = {
     this.markup.destroy();
     this.env = null;
   },
 
   getElement(id) {
     return this.markup.getElement(this.ID_CLASS_PREFIX + id);
   },
 
+  /**
+   * Show this highlighter. This is the usual show method that all highlighters need to
+   * implement so they can be used via the CustomHighlighterActor mechanism.
+   * @param {DOMNode} node The context node for this highlighter.
+   * @param {Object} options See showOverlay for doc about this.
+   * @return {Boolean}
+   */
   show(node, options = {}) {
+
+    /**
+     * This contains the actual logic to show the highlighter. In comparison to the show
+     * method, it does not need a first DOMNode parameter which makes it simpler to be
+     * called by other server-side modules if needed.
+     * @param {Object} options A config object that can take 2 properties:
+     * - {String} reason The text that will be shown in the toolbar. If not given, the
+     *   toolbar will not be displayed.
+     * - {Boolean} onlyToolbar Set to true to only show the toolbar, and not the overlay.
+     * @return {Boolean}
+     */
     if (this.env.isXUL) {
       return false;
     }
 
     // Show the highlighter's root element.
     let root = this.getElement("root");
     root.removeAttribute("hidden");
 
--- a/devtools/server/actors/script.js
+++ b/devtools/server/actors/script.js
@@ -31,16 +31,19 @@ loader.lazyRequireGetter(this, "findCssS
 loader.lazyRequireGetter(this, "mapURIToAddonID", "devtools/server/actors/utils/map-uri-to-addon-id");
 loader.lazyRequireGetter(this, "BreakpointActor", "devtools/server/actors/breakpoint", true);
 loader.lazyRequireGetter(this, "setBreakpointAtEntryPoints", "devtools/server/actors/breakpoint", true);
 loader.lazyRequireGetter(this, "getSourceURL", "devtools/server/actors/source", true);
 loader.lazyRequireGetter(this, "EnvironmentActor", "devtools/server/actors/environment", true);
 loader.lazyRequireGetter(this, "FrameActor", "devtools/server/actors/frame", true);
 loader.lazyRequireGetter(this, "EventEmitter", "devtools/shared/event-emitter");
 
+loader.lazyGetter(this, "l10n",
+  () => Services.strings.createBundle("chrome://devtools-shared/locale/debugger.properties"));
+
 /**
  * A BreakpointActorMap is a map from locations to instances of BreakpointActor.
  */
 function BreakpointActorMap() {
   this._size = 0;
   this._actors = {};
 }
 
@@ -1234,16 +1237,17 @@ const ThreadActor = ActorClassWithSpec(t
                message: "Evaluation frame not found" };
     }
 
     if (!frame.environment) {
       return { error: "notDebuggee",
                message: "cannot access the environment of this frame." };
     }
 
+    // TODO: this is not being used, paused takes no arguements
     let youngest = this.youngestFrame;
 
     // Put ourselves back in the running state and inform the client.
     let resumedPacket = this._resumed();
     this.conn.send(resumedPacket);
 
     // Run the expression.
     // XXX: test syntax errors
@@ -2051,16 +2055,17 @@ const ThreadActor = ActorClassWithSpec(t
 });
 
 Object.assign(ThreadActor.prototype.requestTypes, {
   "attach": ThreadActor.prototype.onAttach,
   "detach": ThreadActor.prototype.onDetach,
   "reconfigure": ThreadActor.prototype.onReconfigure,
   "resume": ThreadActor.prototype.onResume,
   "clientEvaluate": ThreadActor.prototype.onClientEvaluate,
+  "clientPaused": ThreadActor.prototype.onClientPaused,
   "frames": ThreadActor.prototype.onFrames,
   "interrupt": ThreadActor.prototype.onInterrupt,
   "eventListeners": ThreadActor.prototype.onEventListeners,
   "releaseMany": ThreadActor.prototype.onReleaseMany,
   "sources": ThreadActor.prototype.onSources,
   "threadGrips": ThreadActor.prototype.onThreadGrips,
   "prototypesAndProperties": ThreadActor.prototype.onPrototypesAndProperties
 });
new file mode 100644
--- /dev/null
+++ b/devtools/server/tests/unit/test_pausedOverlay.js
@@ -0,0 +1,74 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+/* eslint-disable no-shadow, max-nested-callbacks */
+
+"use strict";
+
+const { pausedStateOverlays } = require("devtools/server/actors/script");
+
+/**
+ * Check that the paused overlay appears when the debugger pauses.
+ */
+var gDebuggee;
+var gClient;
+var gThreadClient;
+var gTabClient;
+
+// We use a mock overlay here, this test doesn't need to test the overlay itself, only the
+// fact that it gets shown and hidden at the right times.
+var isOverlayVisible = false;
+var mockOverlay = {
+  environment: {},
+  overlay: {
+    show: () => {
+      isOverlayVisible = true;
+    },
+    hide: () => {
+      isOverlayVisible = false;
+    }
+  }
+};
+
+
+add_task(async function() {
+  do_test_pending();
+
+  Task.spawn(function* () {
+    initTestDebuggerServer(DebuggerServer);
+    gDebuggee = addTestGlobal("test-overlay", DebuggerServer);
+    gClient = new DebuggerClient(DebuggerServer.connectPipe());
+    yield gClient.connect();
+
+    let [, tabClient, threadClient] = yield attachTestTabAndResume(gClient, "test-overlay");
+    gThreadClient = threadClient;
+    gTabClient = tabClient;
+
+    // Inject a mock overlay here. We need the TabActor reference in order to inject the
+    // mock into the pausedStateOverlays map (go over the transport boundary to do this).
+    pausedStateOverlays.set(
+      gClient._transport._serverConnection.getActor(gTabClient._actor), mockOverlay);
+
+    yield test_client_paused();
+
+    yield gClient.close();
+    do_test_finished();
+  });
+});
+
+function* test_client_paused() {
+  ok(!isOverlayVisible, "The overlay is hidden at first");
+
+  // this is triggering a pause, this may be triggered in any way
+  // such as breakpoint, exception, etc
+  yield executeOnNextTickAndWaitForPause(() => {
+    Cu.evalInSandbox("debugger;", gDebuggee);
+  }, gClient);
+  // we only show the paused overlay if the client determines this is an
+  // important pause for the user
+  yield gThreadClient.clientPaused();
+
+  ok(isOverlayVisible, "The overlay is visible on client paused");
+
+  yield gThreadClient.resume();
+  ok(!isOverlayVisible, "The overlay is hidden on resume");
+}
--- a/devtools/server/tests/unit/xpcshell.ini
+++ b/devtools/server/tests/unit/xpcshell.ini
@@ -62,16 +62,17 @@ support-files =
 [test_frameactor-02.js]
 [test_frameactor-03.js]
 [test_frameactor-04.js]
 [test_frameactor-05.js]
 [test_frameactor_wasm-01.js]
 [test_framearguments-01.js]
 [test_getRuleText.js]
 [test_getTextAtLineColumn.js]
+[test_pausedOverlay.js]
 [test_pauselifetime-01.js]
 [test_pauselifetime-02.js]
 [test_pauselifetime-03.js]
 [test_pauselifetime-04.js]
 [test_threadlifetime-01.js]
 [test_threadlifetime-02.js]
 [test_threadlifetime-03.js]
 [test_threadlifetime-04.js]
--- a/devtools/shared/client/thread-client.js
+++ b/devtools/shared/client/thread-client.js
@@ -270,16 +270,24 @@ ThreadClient.prototype = {
         onResponse(response);
         return response;
       }
       return this.resume(onResponse);
     });
   },
 
   /**
+   * Notify the server that the client needs a pause overlay to be shown.
+   *
+   */
+  clientPaused: DebuggerClient.requester({
+    type: "clientPaused"
+  }),
+
+  /**
    * Send a clientEvaluate packet to the debuggee. Response
    * will be a resume packet.
    *
    * @param string frame
    *        The actor ID of the frame where the evaluation should take place.
    * @param string expression
    *        The expression that will be evaluated in the scope of the frame
    *        above.