Bug 1450974 - Refactor the thread actor's stepping hooks. r=jimb draft
authorJason Laster <jason.laster.11@gmail.com>
Tue, 03 Apr 2018 09:43:52 -0400
changeset 777517 0410214f28ee58ac622b11667ac083c56bfd5f3e
parent 776626 4b1dc5e311b92a54befa20bc4af71668fca6048f
push id105236
push userbmo:jlaster@mozilla.com
push dateWed, 04 Apr 2018 21:55:31 +0000
reviewersjimb
bugs1450974
milestone61.0a1
Bug 1450974 - Refactor the thread actor's stepping hooks. r=jimb MozReview-Commit-ID: 4oRRjYtITTh
devtools/server/actors/thread.js
--- a/devtools/server/actors/thread.js
+++ b/devtools/server/actors/thread.js
@@ -342,95 +342,93 @@ const ThreadActor = ActorClassWithSpec(t
    * @param Debugger.Frame frame
    *        The newest debuggee frame in the stack.
    * @param object reason
    *        An object with a 'type' property containing the reason for the pause.
    * @param function onPacket
    *        Hook to modify the packet before it is sent. Feel free to return a
    *        promise.
    */
-  _pauseAndRespond: function(frame, reason, onPacket = function(k) {
-    return k;
-  }) {
+  _pauseAndRespond: async function(frame, reason, onPacket = k => k) {
     try {
       let packet = this._paused(frame);
       if (!packet) {
         return undefined;
       }
       packet.why = reason;
 
       let generatedLocation = this.sources.getFrameLocation(frame);
-      this.sources.getOriginalLocation(generatedLocation)
-                  .then((originalLocation) => {
-                    if (!originalLocation.originalSourceActor) {
-          // The only time the source actor will be null is if there
-          // was a sourcemap and it tried to look up the original
-          // location but there was no original URL. This is a strange
-          // scenario so we simply don't pause.
-                      DevToolsUtils.reportException(
-            "ThreadActor",
-            new Error("Attempted to pause in a script with a sourcemap but " +
-                      "could not find original location.")
-          );
+      const originalLocation = await this.sources.getOriginalLocation(generatedLocation);
 
-                      return undefined;
-                    }
+      // The only time the source actor will be null is if there
+      // was a sourcemap and it tried to look up the original
+      // location but there was no original URL. This is a strange
+      // scenario so we simply don't pause.
+      if (!originalLocation.originalSourceActor) {
+        DevToolsUtils.reportException("ThreadActor", new Error(
+          "Attempted to pause in a script with a sourcemap but" +
+          " could not find original location."
+        ));
+
+        return undefined;
+      }
 
-                    packet.frame.where = {
-                      source: originalLocation.originalSourceActor.form(),
-                      line: originalLocation.originalLine,
-                      column: originalLocation.originalColumn
-                    };
-                    Promise.resolve(onPacket(packet))
-          .catch(error => {
-            reportError(error);
-            return {
-              error: "unknownError",
-              message: error.message + "\n" + error.stack
-            };
-          })
-          .then(pkt => {
-            this.conn.send(pkt);
-          });
+      packet.frame.where = {
+        source: originalLocation.originalSourceActor.form(),
+        line: originalLocation.originalLine,
+        column: originalLocation.originalColumn
+      };
 
-                    return undefined;
-                  });
+      Promise.resolve(onPacket(packet))
+      .catch(error => {
+        reportError(error);
+        return {
+          error: "unknownError",
+          message: error.message + "\n" + error.stack
+        };
+      })
+      .then(pkt => this.conn.send(pkt));
 
       this._pushThreadPause();
     } catch (e) {
       reportError(e, "Got an exception during TA__pauseAndRespond: ");
     }
 
     // If the browser tab has been closed, terminate the debuggee script
     // instead of continuing. Executing JS after the content window is gone is
     // a bad idea.
     return this._tabClosed ? null : undefined;
   },
 
   _makeOnEnterFrame: function({ pauseAndRespond }) {
     return frame => {
       const generatedLocation = this.sources.getFrameLocation(frame);
       let { originalSourceActor } = this.unsafeSynchronize(
-        this.sources.getOriginalLocation(generatedLocation));
+        this.sources.getOriginalLocation(generatedLocation)
+      );
+
       let url = originalSourceActor.url;
 
-      return this.sources.isBlackBoxed(url)
-        ? undefined
-        : pauseAndRespond(frame);
+      if (this.sources.isBlackBoxed(url)) {
+        return undefined;
+      }
+
+      return pauseAndRespond(frame);
     };
   },
 
   _makeOnPop: function({ thread, pauseAndRespond, createValueGrip: createValueGripHook,
                           startLocation }) {
     const result = function(completion) {
       // onPop is called with 'this' set to the current frame.
-
       const generatedLocation = thread.sources.getFrameLocation(this);
       const { originalSourceActor } = thread.unsafeSynchronize(
-        thread.sources.getOriginalLocation(generatedLocation));
+        thread.sources.getOriginalLocation(generatedLocation)
+      );
+
       const url = originalSourceActor.url;
 
       if (thread.sources.isBlackBoxed(url)) {
         return undefined;
       }
 
       // Note that we're popping this frame; we need to watch for
       // subsequent step events on its caller.
@@ -459,22 +457,20 @@ const ThreadActor = ActorClassWithSpec(t
     // frame, if we did we'd also have to find the appropriate spot to
     // clear it.
     result.originalLocation = startLocation;
 
     return result;
   },
 
   _makeOnStep: function({ thread, pauseAndRespond, startFrame,
-                           startLocation, steppingType }) {
+                          startLocation, steppingType }) {
     // Breaking in place: we should always pause.
     if (steppingType === "break") {
-      return function() {
-        return pauseAndRespond(this);
-      };
+      return () => pauseAndRespond(this);
     }
 
     // Otherwise take what a "step" means into consideration.
     return function() {
       // onStep is called with 'this' set to the current frame.
 
       // Only allow stepping stops at entry points for the line, when
       // the stepping occurs in a single frame.  The "same frame"
@@ -483,18 +479,19 @@ const ThreadActor = ActorClassWithSpec(t
       // is a call "a(b())" and the user steps into b, then this
       // condition makes it possible to step out of b and into a.
       if (this === startFrame &&
           !this.script.getOffsetLocation(this.offset).isEntryPoint) {
         return undefined;
       }
 
       const generatedLocation = thread.sources.getFrameLocation(this);
-      const newLocation = thread.unsafeSynchronize(thread.sources.getOriginalLocation(
-        generatedLocation));
+      const newLocation = thread.unsafeSynchronize(
+        thread.sources.getOriginalLocation(generatedLocation)
+      );
 
       // Cases when we should pause because we have executed enough to consider
       // a "step" to have occured:
       //
       // 1.1. We change frames.
       // 1.2. We change URLs (can happen without changing frames thanks to
       //      source mapping).
       // 1.3. The source has pause points and we change locations.
@@ -553,21 +550,22 @@ const ThreadActor = ActorClassWithSpec(t
    * Define the JS hook functions for stepping.
    */
   _makeSteppingHooks: function(startLocation, steppingType) {
     // Bind these methods and state because some of the hooks are called
     // with 'this' set to the current frame. Rather than repeating the
     // binding in each _makeOnX method, just do it once here and pass it
     // in to each function.
     const steppingHookState = {
-      pauseAndRespond: (frame, onPacket = k=>k) => {
-        return this._pauseAndRespond(frame, { type: "resumeLimit" }, onPacket);
-      },
-      createValueGrip: v => createValueGrip(v, this._pausePool,
-        this.objectGrip),
+      pauseAndRespond: (frame, onPacket = k=>k) => this._pauseAndRespond(
+        frame,
+        { type: "resumeLimit" },
+        onPacket
+      ),
+      createValueGrip: v => createValueGrip(v, this._pausePool, this.objectGrip),
       thread: this,
       startFrame: this.youngestFrame,
       startLocation: startLocation,
       steppingType: steppingType
     };
 
     return {
       onEnterFrame: this._makeOnEnterFrame(steppingHookState),
@@ -580,51 +578,52 @@ const ThreadActor = ActorClassWithSpec(t
    * Handle attaching the various stepping hooks we need to attach when we
    * receive a resume request with a resumeLimit property.
    *
    * @param Object request
    *        The request packet received over the RDP.
    * @returns A promise that resolves to true once the hooks are attached, or is
    *          rejected with an error packet.
    */
-  _handleResumeLimit: function(request) {
+  _handleResumeLimit: async function(request) {
     let steppingType = request.resumeLimit.type;
     if (!["break", "step", "next", "finish"].includes(steppingType)) {
-      return Promise.reject({ error: "badParameterType",
-                              message: "Unknown resumeLimit type" });
+      return Promise.reject({
+        error: "badParameterType",
+        message: "Unknown resumeLimit type"
+      });
     }
 
     const generatedLocation = this.sources.getFrameLocation(this.youngestFrame);
-    return this.sources.getOriginalLocation(generatedLocation)
-      .then(originalLocation => {
-        const { onEnterFrame, onPop, onStep } = this._makeSteppingHooks(originalLocation,
-                                                                        steppingType);
+    const originalLocation = await this.sources.getOriginalLocation(generatedLocation);
+    const { onEnterFrame, onPop, onStep } = this._makeSteppingHooks(
+      originalLocation,
+      steppingType
+    );
 
-        // Make sure there is still a frame on the stack if we are to continue
-        // stepping.
-        let stepFrame = this._getNextStepFrame(this.youngestFrame);
-        if (stepFrame) {
-          switch (steppingType) {
-            case "step":
-              this.dbg.onEnterFrame = onEnterFrame;
-              // Fall through.
-            case "break":
-            case "next":
-              if (stepFrame.script) {
-                stepFrame.onStep = onStep;
-              }
-              stepFrame.onPop = onPop;
-              break;
-            case "finish":
-              stepFrame.onPop = onPop;
+    // Make sure there is still a frame on the stack if we are to continue stepping.
+    let stepFrame = this._getNextStepFrame(this.youngestFrame);
+    if (stepFrame) {
+      switch (steppingType) {
+        case "step":
+          this.dbg.onEnterFrame = onEnterFrame;
+          // Fall through.
+        case "break":
+        case "next":
+          if (stepFrame.script) {
+            stepFrame.onStep = onStep;
           }
-        }
+          stepFrame.onPop = onPop;
+          break;
+        case "finish":
+          stepFrame.onPop = onPop;
+      }
+    }
 
-        return true;
-      });
+    return true;
   },
 
   /**
    * Clear the onStep and onPop hooks from the given frame and all of the frames
    * below it.
    *
    * @param Debugger.Frame aFrame
    *        The frame we want to clear the stepping hooks from.
@@ -1339,24 +1338,33 @@ const ThreadActor = ActorClassWithSpec(t
    * Return a protocol completion value representing the given
    * Debugger-provided completion value.
    */
   createProtocolCompletionValue: function(completion) {
     let protoValue = {};
     if (completion == null) {
       protoValue.terminated = true;
     } else if ("return" in completion) {
-      protoValue.return = createValueGrip(completion.return,
-        this._pausePool, this.objectGrip);
+      protoValue.return = createValueGrip(
+        completion.return,
+        this._pausePool,
+        this.objectGrip
+      );
     } else if ("throw" in completion) {
-      protoValue.throw = createValueGrip(completion.throw,
-        this._pausePool, this.objectGrip);
+      protoValue.throw = createValueGrip(
+        completion.throw,
+        this._pausePool,
+        this.objectGrip
+      );
     } else {
-      protoValue.return = createValueGrip(completion.yield,
-        this._pausePool, this.objectGrip);
+      protoValue.return = createValueGrip(
+        completion.yield,
+        this._pausePool,
+        this.objectGrip
+      );
     }
     return protoValue;
   },
 
   /**
    * Create a grip for the given debuggee object.
    *
    * @param value Debugger.Object