Bug 1571704 - Make ThreadActor.frames return Frame fronts instead of JSON objects. r=nchevobbe

Differential Revision: https://phabricator.services.mozilla.com/D51004

--HG--
extra : moz-landing-system : lando

devtools/client/debugger/src/client/firefox/commands.js

@@ -342,8 +342,8 @@ async function getFrameScopes(frame: Frame): Promise<*> {
     return frame.scope;
   }
 
-  const sourceThreadFront = lookupThreadFront(frame.thread);
-  return sourceThreadFront.getEnvironment(frame.id);
+  const frameFront = lookupThreadFront(frame.thread).get(frame.id);
+  return frameFront.getEnvironment();
 }
 
 function pauseOnExceptions(

devtools/client/debugger/src/client/firefox/create.js

@@ -8,8 +8,8 @@
 import type { Frame, ThreadId, GeneratedSourceData, Thread } from "../../types";
 import type {
   PausedPacket,
-  FramesResponse,
   FramePacket,
+  FrameFront,
   SourcePayload,
   ThreadFront,
   Target,
@@ -32,7 +32,7 @@ export function prepareSourcePayload(
 
 export function createFrame(
   thread: ThreadId,
-  frame: FramePacket,
+  frame: FramePacket | FrameFront,
   index: number = 0
 ): ?Frame {
   if (!frame) {
@@ -45,8 +45,10 @@ export function createFrame(
     column: frame.where.column,
   };
 
+  const id = frame.data ? frame.actorID : frame.actor;
+
   return {
-    id: frame.actor,
+    id,
     thread,
     displayName: frame.displayName,
     location,
@@ -65,16 +67,16 @@ export function makeSourceId(source: SourcePayload) {
 export function createPause(
   thread: string,
   packet: PausedPacket,
-  response: FramesResponse
+  frames: FramePacket[]
 ): any {
   // NOTE: useful when the debugger is already paused
-  const frame = packet.frame || response.frames[0];
+  const frame = packet.frame || frames[0];
 
   return {
     ...packet,
     thread,
     frame: createFrame(thread, frame),
-    frames: response.frames.map((currentFrame, i) =>
+    frames: frames.map((currentFrame, i) =>
       createFrame(thread, currentFrame, i)
     ),
   };

devtools/client/debugger/src/client/firefox/events.js

@@ -84,17 +84,13 @@ async function paused(threadFront: ThreadFront, packet: PausedPacket) {
     return;
   }
 
-  // NOTE: this happens if we fetch frames and then immediately navigate
-  if (!response.hasOwnProperty("frames")) {
+  if (!response || why.type == "alreadyPaused") {
     return;
   }
 
-  if (why.type != "alreadyPaused") {
-    const pause = createPause(threadFront.actor, packet, response);
-    await sourceQueue.flush();
-    actions.paused(pause);
-  }
-
+  const pause = createPause(threadFront.actor, packet, response.frames);
+  await sourceQueue.flush();
+  actions.paused(pause);
   recordEvent("pause", { reason: why.type });
 }
 

devtools/client/debugger/src/client/firefox/types.js

@@ -18,7 +18,6 @@ import type {
   Script,
   Pause,
   PendingLocation,
-  Frame,
   SourceId,
   QueuedSourceData,
   Range,
@@ -66,21 +65,34 @@ type URL = string;
  * @static
  */
 
+export type FramePacket = {|
+  +actor: ActorId,
+  +arguments: any[],
+  +displayName: string,
+  +environment: any,
+  +this: any,
+  +depth?: number,
+  +oldest?: boolean,
+  +type: "pause" | "call",
+  +where: ServerLocation,
+|};
+
+type ServerLocation = {| actor: string, line: number, column: number |};
+
 /**
  * Frame Packet
  * @memberof firefox/packets
  * @static
  */
-export type FramePacket = {
-  actor: ActorId,
-  arguments: any[],
+export type FrameFront = {
+  +typeName: "frame",
+  +data: FramePacket,
+  +getEnvironment: () => Promise<*>,
+  +where: ServerLocation,
+  actorID: string,
   displayName: string,
-  environment: any,
   this: any,
-  depth?: number,
-  oldest?: boolean,
-  type: "pause" | "call",
-  where: {| actor: string, line: number, column: number |},
+  environment: any,
 };
 
 /**
@@ -144,10 +156,6 @@ export type PausedPacket = {
  * @memberof firefox
  * @static
  */
-export type FramesResponse = {
-  frames: FramePacket[],
-  from: ActorId,
-};
 
 export type TabPayload = {
   actor: ActorId,
@@ -359,6 +367,7 @@ export type ObjectFront = {
  * @static
  */
 export type ThreadFront = {
+  getFrames: (number, number) => Promise<{| frames: FramePacket[] |}>,
   resume: Function => Promise<*>,
   stepIn: Function => Promise<*>,
   stepOver: Function => Promise<*>,
@@ -376,8 +385,6 @@ export type ThreadFront = {
   removeXHRBreakpoint: (path: string, method: string) => Promise<boolean>,
   interrupt: () => Promise<*>,
   eventListeners: () => Promise<*>,
-  getFrames: (number, number) => FramesResponse,
-  getEnvironment: (frame: Frame) => Promise<*>,
   on: (string, Function) => void,
   getSources: () => Promise<SourcesPacket>,
   reconfigure: ({ observeAsmJS: boolean }) => Promise<*>,
@@ -393,6 +400,7 @@ export type ThreadFront = {
   detach: () => Promise<void>,
   timeWarp: Function => Promise<*>,
   fetchAncestorFramePositions: Function => Promise<*>,
+  get: string => FrameFront,
 };
 
 export type Panel = {|

devtools/client/debugger/test/mochitest/helpers.js

@@ -1869,7 +1869,7 @@ async function evaluateInTopFrame(dbg, text) {
   const consoleFront = await dbg.toolbox.target.getFront("console");
   const { frames } = await threadFront.getFrames(0, 1);
   ok(frames.length == 1, "Got one frame");
-  const options = { thread: threadFront.actor, frameActor: frames[0].actor };
+  const options = { thread: threadFront.actor, frameActor: frames[0].actorID };
   const response = await consoleFront.evaluateJS(text, options);
   return response.result.type == "undefined" ? undefined : response.result;
 }

devtools/server/actors/frame.js

@@ -30,9 +30,10 @@ const FrameActor = ActorClassWithSpec(frameSpec, {
    * @param threadActor ThreadActor
    *        The parent thread actor for this frame.
    */
-  initialize: function(frame, threadActor) {
+  initialize: function(frame, threadActor, depth) {
     this.frame = frame;
     this.threadActor = threadActor;
+    this.depth = depth;
   },
 
   /**
@@ -81,7 +82,14 @@ const FrameActor = ActorClassWithSpec(frameSpec, {
    */
   form: function() {
     const threadActor = this.threadActor;
-    const form = { actor: this.actorID, type: this.frame.type };
+    const form = {
+      actor: this.actorID,
+      type: this.frame.type,
+    };
+
+    if (this.depth) {
+      form.depth = this.depth;
+    }
 
     // NOTE: ignoreFrameEnvironment lets the client explicitly avoid
     // populating form environments on pause.
@@ -102,6 +110,7 @@ const FrameActor = ActorClassWithSpec(frameSpec, {
 
     form.displayName = formatDisplayName(this.frame);
     form.arguments = this._args();
+
     if (this.frame.script) {
       const location = this.threadActor.sources.getFrameLocation(this.frame);
       form.where = {

devtools/server/actors/thread.js

@@ -1318,7 +1318,7 @@ const ThreadActor = ActorClassWithSpec(threadSpec, {
     return stepFrame;
   },
 
-  onFrames: function(request) {
+  frames: function(request) {
     if (this.state !== "paused") {
       return {
         error: "wrongState",
@@ -1342,27 +1342,15 @@ const ThreadActor = ActorClassWithSpec(threadSpec, {
     // frames if count is not defined.
     const frames = [];
     for (; frame && (!count || i < start + count); i++, frame = frame.older) {
-      const form = this._createFrameActor(frame).form();
-      form.depth = i;
-
-      let frameItem = null;
-
-      const frameSourceActor = this.sources.createSourceActor(
-        frame.script.source
-      );
-      if (frameSourceActor) {
-        form.where = {
-          actor: frameSourceActor.actorID,
-          line: form.where.line,
-          column: form.where.column,
-        };
-        frameItem = form;
+      const sourceActor = this.sources.createSourceActor(frame.script.source);
+      if (!sourceActor) {
+        continue;
       }
-      frames.push(frameItem);
+      const frameActor = this._createFrameActor(frame, i);
+      frames.push(frameActor);
     }
 
-    // Filter null values because createSourceActor can be falsey
-    return { frames: frames.filter(x => !!x) };
+    return { frames };
   },
 
   addAllSources() {
@@ -1533,6 +1521,7 @@ const ThreadActor = ActorClassWithSpec(threadSpec, {
     const packet = {
       actor: this._pauseActor.actorID,
     };
+
     if (frame) {
       packet.frame = this._createFrameActor(frame).form();
     }
@@ -1630,12 +1619,12 @@ const ThreadActor = ActorClassWithSpec(threadSpec, {
     return popped;
   },
 
-  _createFrameActor: function(frame) {
+  _createFrameActor: function(frame, depth) {
     if (frame.actor) {
       return frame.actor;
     }
 
-    const actor = new FrameActor(frame, this);
+    const actor = new FrameActor(frame, this, depth);
     this._frameActors.push(actor);
     this._framePool.addActor(actor);
     frame.actor = actor;
@@ -2200,7 +2189,6 @@ Object.assign(ThreadActor.prototype.requestTypes, {
   detach: ThreadActor.prototype.onDetach,
   reconfigure: ThreadActor.prototype.onReconfigure,
   resume: ThreadActor.prototype.onResume,
-  frames: ThreadActor.prototype.onFrames,
   interrupt: ThreadActor.prototype.onInterrupt,
   sources: ThreadActor.prototype.onSources,
   skipBreakpoints: ThreadActor.prototype.onSkipBreakpoints,

devtools/server/tests/unit/test_frameactor-05.js

@@ -42,7 +42,7 @@ function test_pause_frame() {
       // youngest actors should be popped..
       const expectPopped = frameResponse.frames
         .slice(0, 3)
-        .map(frame => frame.actor);
+        .map(frame => frame.actorID);
       expectPopped.sort();
 
       gThreadFront.once("paused", function(pausePacket) {

devtools/shared/fronts/frame.js

@@ -0,0 +1,26 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+"use strict";
+
+const { frameSpec } = require("devtools/shared/specs/frame");
+const {
+  FrontClassWithSpec,
+  registerFront,
+} = require("devtools/shared/protocol");
+
+class FrameFront extends FrontClassWithSpec(frameSpec) {
+  form(json) {
+    this.displayName = json.displayName;
+    this.arguments = json.arguments;
+    this.type = json.type;
+    this.where = json.where;
+    this.environment = json.environment;
+    this.this = json.this;
+    this.data = json;
+  }
+}
+
+module.exports = FrameFront;
+registerFront(FrameFront);

devtools/shared/fronts/moz.build

@@ -22,6 +22,7 @@ DevToolsModules(
     'device.js',
     'emulation.js',
     'environment.js',
+    'frame.js',
     'framerate.js',
     'highlighters.js',
     'inspector.js',

devtools/shared/fronts/thread.js

@@ -9,9 +9,11 @@ const {
   FrontClassWithSpec,
   registerFront,
 } = require("devtools/shared/protocol");
+
 const { threadSpec } = require("devtools/shared/specs/thread");
 
 loader.lazyRequireGetter(this, "ObjectFront", "devtools/shared/fronts/object");
+loader.lazyRequireGetter(this, "FrameFront", "devtools/shared/fronts/frame");
 loader.lazyRequireGetter(
   this,
   "SourceFront",
@@ -65,6 +67,10 @@ class ThreadFront extends FrontClassWithSpec(threadSpec) {
     }
   }
 
+  getFrames(start, count) {
+    return super.frames(start, count);
+  }
+
   /**
    * Resume a paused thread. If the optional limit parameter is present, then
    * the thread will also pause when that limit is reached.
@@ -211,19 +217,6 @@ class ThreadFront extends FrontClassWithSpec(threadSpec) {
     return { sources };
   }
 
-  /**
-   * Request frames from the callstack for the current thread.
-   *
-   * @param start integer
-   *        The number of the youngest stack frame to return (the youngest
-   *        frame is 0).
-   * @param count integer
-   *        The maximum number of frames to return, or null to return all
-   *        frames.
-   */
-  getFrames(start, count) {
-    return super.frames(start, count);
-  }
   /**
    * attach to the thread actor.
    */
@@ -249,15 +242,6 @@ class ThreadFront extends FrontClassWithSpec(threadSpec) {
     await this.destroy();
   }
 
-  /**
-   * Request the frame environment.
-   *
-   * @param frameId string
-   */
-  getEnvironment(frameId) {
-    return this.client.request({ to: frameId, type: "getEnvironment" });
-  }
-
   /**
    * Return a ObjectFront object for the given object grip.
    *
@@ -287,6 +271,14 @@ class ThreadFront extends FrontClassWithSpec(threadSpec) {
     this[gripCacheName] = {};
   }
 
+  _clearFrameFronts() {
+    for (const front of this.poolChildren()) {
+      if (front instanceof FrameFront) {
+        this.unmanage(front);
+      }
+    }
+  }
+
   /**
    * Invalidate pause-lifetime grip clients and clear the list of current grip
    * clients.
@@ -328,6 +320,7 @@ class ThreadFront extends FrontClassWithSpec(threadSpec) {
     // when it's initialized
     this._lastPausePacket = packet;
     this._clearPauseGrips();
+    this._clearFrameFronts();
   }
 
   getLastPausePacket() {

devtools/shared/specs/index.js

@@ -82,11 +82,10 @@ const Types = (exports.__TypesForTests = [
     spec: "devtools/shared/specs/environment",
     front: "devtools/shared/fronts/environment",
   },
-  /* frame has old fashion client and no front */
   {
     types: ["frame"],
     spec: "devtools/shared/specs/frame",
-    front: null,
+    front: "devtools/shared/fronts/frame",
   },
   {
     types: ["framerate"],

devtools/shared/specs/thread.js

@@ -15,10 +15,16 @@ types.addDictType("available-breakpoint-group", {
   name: "string",
   events: "array:available-breakpoint-event",
 });
+
 types.addDictType("available-breakpoint-event", {
   id: "string",
   name: "string",
 });
+
+types.addDictType("thread.frames", {
+  frames: "array:frame",
+});
+
 types.addDictType("paused-reason", {
   type: "string",
 
@@ -102,7 +108,7 @@ const threadSpec = generateActorSpec({
         start: Arg(0, "number"),
         count: Arg(1, "number"),
       },
-      response: RetVal("json"),
+      response: RetVal("thread.frames"),
     },
     interrupt: {
       request: {