Bug 1544694 - Convert Source Client to a Front; r=ochameau,jdescottes
authoryulia <ystartsev@mozilla.com>
Fri, 26 Apr 2019 12:51:07 +0000
changeset 530296 27e6d1a417e2e5b67996c0625868a147b051b7ab
parent 530295 9c36551d262a0581a34d7a450bd2284d76f3e8cf
child 530297 96682cfce65c88f4075349abf2aec0d766601dde
push id11265
push userffxbld-merge
push dateMon, 13 May 2019 10:53:39 +0000
treeherdermozilla-beta@77e0fe8dbdd3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersochameau, jdescottes
bugs1544694
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1544694 - Convert Source Client to a Front; r=ochameau,jdescottes Differential Revision: https://phabricator.services.mozilla.com/D27708
devtools/client/debugger/src/client/firefox/commands.js
devtools/server/actors/source.js
devtools/server/tests/unit/head_dbg.js
devtools/server/tests/unit/test_blackboxing-04.js
devtools/server/tests/unit/test_breakpoint-09.js
devtools/server/tests/unit/test_source-02.js
devtools/shared/client/source-client.js
devtools/shared/client/thread-client.js
devtools/shared/specs/source.js
--- a/devtools/client/debugger/src/client/firefox/commands.js
+++ b/devtools/client/debugger/src/client/firefox/commands.js
@@ -460,17 +460,17 @@ async function getBreakpointPositions(
   actors: Array<SourceActor>,
   range: ?Range
 ): Promise<{ [string]: number[] }> {
   const sourcePositions = {};
 
   for (const { thread, actor } of actors) {
     const sourceThreadClient = lookupThreadClient(thread);
     const sourceClient = sourceThreadClient.source({ actor });
-    const { positions } = await sourceClient.getBreakpointPositionsCompressed(
+    const positions = await sourceClient.getBreakpointPositionsCompressed(
       range
     );
 
     for (const line of Object.keys(positions)) {
       let columns = positions[line];
       const existing = sourcePositions[line];
       if (existing) {
         columns = [...new Set([...existing, ...columns])];
--- a/devtools/server/actors/source.js
+++ b/devtools/server/actors/source.js
@@ -319,17 +319,21 @@ const SourceActor = ActorClassWithSpec(s
         compressed[line] = [];
       }
       compressed[line].push(column);
     }
     return compressed;
   },
 
   /**
-   * Handler for the "source" packet.
+   * Handler for the "onSource" packet.
+   * @return Object
+   *         The return of this function contains a field `contentType`, and
+   *         a field `source`. `source` can either be an arrayBufferActor grip,
+   *         or a LongStringActor grip.
    */
   onSource: function() {
     return Promise.resolve(this._init)
       .then(this._getSourceText)
       .then(({ content, contentType }) => {
         if (typeof content === "object" && content && content.constructor &&
             content.constructor.name === "ArrayBuffer") {
           return {
--- a/devtools/server/tests/unit/head_dbg.js
+++ b/devtools/server/tests/unit/head_dbg.js
@@ -741,31 +741,31 @@ function getFrames(threadClient, first, 
 /**
  * Black box the specified source.
  *
  * @param SourceClient sourceClient
  * @returns Promise
  */
 async function blackBox(sourceClient, range = null) {
   dumpn("Black boxing source: " + sourceClient.actor);
-  const { error, pausedInSource } = await sourceClient.blackBox(range);
-  Assert.ok(!error, "Should not get an error: " + error);
-  return {error, pausedInSource};
+  const pausedInSource = await sourceClient.blackBox(range);
+  ok(true, "blackBox didn't throw");
+  return pausedInSource;
 }
 
 /**
  * Stop black boxing the specified source.
  *
  * @param SourceClient sourceClient
  * @returns Promise
  */
 async function unBlackBox(sourceClient, range = null) {
   dumpn("Un-black boxing source: " + sourceClient.actor);
-  const {error} = await sourceClient.unblackBox(range);
-  Assert.ok(!error, "Should not get an error: " + error);
+  await sourceClient.unblackBox(range);
+  ok(true, "unblackBox didn't throw");
 }
 
 /**
  * Perform a "source" RDP request with the given SourceClient to get the source
  * content and content type.
  *
  * @param SourceClient sourceClient
  * @returns Promise
--- a/devtools/server/tests/unit/test_blackboxing-04.js
+++ b/devtools/server/tests/unit/test_blackboxing-04.js
@@ -66,14 +66,14 @@ function test_black_box() {
 
 function test_black_box_paused() {
   gThreadClient.getSources(async function({error, sources}) {
     Assert.ok(!error, "Should not get an error: " + error);
     const sourceClient = gThreadClient.source(
       sources.filter(s => s.url == BLACK_BOXED_URL)[0]
     );
 
-    const {pausedInSource} = await blackBox(sourceClient);
+    const pausedInSource = await blackBox(sourceClient);
     Assert.ok(pausedInSource,
       "We should be notified that we are currently paused in this source");
     finishClient(gClient);
   });
 }
--- a/devtools/server/tests/unit/test_breakpoint-09.js
+++ b/devtools/server/tests/unit/test_breakpoint-09.js
@@ -17,17 +17,17 @@ add_task(threadClientTest(({ threadClien
         packet.frame.where.actor
       );
       const location = { sourceUrl: source.url, line: debuggee.line0 + 2 };
 
       threadClient.setBreakpoint(location, {});
       threadClient.addOneTimeListener("paused", function(event, packet) {
         // Check the return value.
         Assert.equal(packet.type, "paused");
-        Assert.equal(packet.frame.where.actor, source.actor);
+        Assert.equal(packet.frame.where.actor, source.actorID);
         Assert.equal(packet.frame.where.line, location.line);
         Assert.equal(packet.why.type, "breakpoint");
         // Check that the breakpoint worked.
         Assert.equal(debuggee.a, undefined);
 
         // Remove the breakpoint.
         threadClient.removeBreakpoint(location);
         done = true;
--- a/devtools/server/tests/unit/test_source-02.js
+++ b/devtools/server/tests/unit/test_source-02.js
@@ -56,19 +56,18 @@ function test_source() {
         return s.url === SOURCE_URL;
       })[0];
 
       Assert.ok(!!source);
 
       const sourceClient = gThreadClient.source(source);
       response = await sourceClient.getBreakpointPositions();
       Assert.ok(!!response);
-      Assert.ok(!!response.positions);
       Assert.deepEqual(
-        response.positions,
+        response,
         [{
           line: 2,
           column: 2,
         }, {
           line: 3,
           column: 14,
         }, {
           line: 3,
@@ -82,19 +81,18 @@ function test_source() {
         }, {
           line: 6,
           column: 0,
         }]
       );
 
       response = await sourceClient.getBreakpointPositionsCompressed();
       Assert.ok(!!response);
-      Assert.ok(!!response.positions);
       Assert.deepEqual(
-        response.positions,
+        response,
         {
           2: [2],
           3: [14, 17, 24],
           4: [4],
           6: [0],
         }
       );
 
--- a/devtools/shared/client/source-client.js
+++ b/devtools/shared/client/source-client.js
@@ -1,96 +1,66 @@
 /* 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 {arg, DebuggerClient} = require("devtools/shared/client/debugger-client");
+const { sourceSpec } = require("devtools/shared/specs/source");
+const { FrontClassWithSpec, registerFront } = require("devtools/shared/protocol");
 
 /**
- * A SourceClient provides a way to access the source text of a script.
+ * A SourceFront provides a way to access the source text of a script.
  *
- * @param client ThreadClient
- *        The thread client parent.
+ * @param client DebuggerClient
+ *        The Debugger Client instance.
  * @param form Object
  *        The form sent across the remote debugging protocol.
+ * @param activeThread ThreadClient
+ *        The thread client parent. Used until the SourceFront marshalls LongStringFront
+ *        and ArrayBuffer.
  */
-function SourceClient(client, form) {
-  this._form = form;
-  this._activeThread = client;
-  this._client = client.client;
-}
-
-SourceClient.prototype = {
-  get actor() {
-    return this._form.actor;
-  },
-  get url() {
-    return this._form.url;
-  },
-
-  /**
-   * Black box this SourceClient's source.
-   */
-  blackBox: DebuggerClient.requester(
-    {
-      type: "blackbox",
-      range: arg(0),
-    },
-    {
-      telemetry: "BLACKBOX",
-    },
-  ),
+class SourceClient extends FrontClassWithSpec(sourceSpec) {
+  constructor(client, form, activeThread) {
+    super(client);
+    this._url = form.url;
+    this._activeThread = activeThread;
+    // this is here for the time being, until the source front is managed
+    // via protocol.js marshalling
+    this.actorID = form.actor;
+    this.manage(this);
+  }
 
-  /**
-   * Un-black box this SourceClient's source.
-   */
-  unblackBox: DebuggerClient.requester(
-    {
-      type: "unblackbox",
-      range: arg(0),
-    },
-    {
-      telemetry: "UNBLACKBOX",
-    },
-  ),
+  get actor() {
+    return this.actorID;
+  }
+
+  get url() {
+    return this._url;
+  }
 
-  getBreakpointPositions: function(query) {
-    const packet = {
-      to: this._form.actor,
-      type: "getBreakpointPositions",
-      query,
-    };
-    return this._client.request(packet);
-  },
+  // Alias for source.blackbox to avoid changing protocol.js packets
+  blackBox(range) {
+    return this.blackbox(range);
+  }
 
-  getBreakpointPositionsCompressed: function(query) {
-    const packet = {
-      to: this._form.actor,
-      type: "getBreakpointPositionsCompressed",
-      query,
-    };
-    return this._client.request(packet);
-  },
+  // Alias for source.unblackbox to avoid changing protocol.js packets
+  unblackBox() {
+    return this.unblackbox();
+  }
 
   /**
    * Get a long string grip for this SourceClient's source.
    */
-  source: function() {
-    const packet = {
-      to: this._form.actor,
-      type: "source",
-    };
-    return this._client.request(packet).then(response => {
-      return this._onSourceResponse(response);
-    });
-  },
+  async source() {
+    const response = await this.onSource();
+    return this._onSourceResponse(response);
+  }
 
-  _onSourceResponse: function(response) {
+  _onSourceResponse(response) {
     if (typeof response.source === "string") {
       return response;
     }
 
     const { contentType, source } = response;
     if (source.type === "arrayBuffer") {
       const arrayBuffer = this._activeThread.threadArrayBuffer(source);
       return arrayBuffer.slice(0, arrayBuffer.length).then(function(resp) {
@@ -118,21 +88,13 @@ SourceClient.prototype = {
       }
 
       const newResponse = {
         source: resp.substring,
         contentType: contentType,
       };
       return newResponse;
     });
-  },
+  }
+}
 
-  setPausePoints: function(pausePoints) {
-    const packet = {
-      to: this._form.actor,
-      type: "setPausePoints",
-      pausePoints,
-    };
-    return this._client.request(packet);
-  },
-};
-
-module.exports = SourceClient;
+exports.SourceClient = SourceClient;
+registerFront(SourceClient);
--- a/devtools/shared/client/thread-client.js
+++ b/devtools/shared/client/thread-client.js
@@ -7,17 +7,17 @@
 
 const {arg, DebuggerClient} = require("devtools/shared/client/debugger-client");
 const eventSource = require("devtools/shared/client/event-source");
 const {ThreadStateTypes} = require("devtools/shared/client/constants");
 
 loader.lazyRequireGetter(this, "ArrayBufferClient", "devtools/shared/client/array-buffer-client");
 loader.lazyRequireGetter(this, "LongStringClient", "devtools/shared/client/long-string-client");
 loader.lazyRequireGetter(this, "ObjectClient", "devtools/shared/client/object-client");
-loader.lazyRequireGetter(this, "SourceClient", "devtools/shared/client/source-client");
+loader.lazyRequireGetter(this, "SourceClient", "devtools/shared/client/source-client", true);
 
 /**
  * Creates a thread client for the remote debugging protocol server. This client
  * is a front to the thread actor created in the server side, hiding the
  * protocol details in a traditional JavaScript API.
  *
  * @param client DebuggerClient
  * @param actor string
@@ -539,17 +539,17 @@ ThreadClient.prototype = {
   /**
    * Return an instance of SourceClient for the given source actor form.
    */
   source: function(form) {
     if (form.actor in this._threadGrips) {
       return this._threadGrips[form.actor];
     }
 
-    this._threadGrips[form.actor] = new SourceClient(this, form);
+    this._threadGrips[form.actor] = new SourceClient(this.client, form, this);
     return this._threadGrips[form.actor];
   },
 
   events: ["newSource", "progress"],
 };
 
 eventSource(ThreadClient.prototype);
 
--- a/devtools/shared/specs/source.js
+++ b/devtools/shared/specs/source.js
@@ -34,16 +34,18 @@ const sourceSpec = generateActorSpec({
       request: {
         query: Arg(0, "nullable:breakpointquery"),
       },
       response: {
         positions: RetVal("json"),
       },
     },
     onSource: {
+      // we are sending the type "source" to be compatible
+      // with FF67 and older
       request: { type: "source" },
       response: RetVal("json"),
     },
     setPausePoints: {
       request: {
         pausePoints: Arg(0, "json"),
       },
     },