Bug 1574190 - Add server support for watchpoints. r=jlast
authorMiriam <bmiriam1230@gmail.com>
Thu, 22 Aug 2019 22:33:08 +0000
changeset 550451 8e22b66d70f9c11f4ac03ff23805dec47e811cf4
parent 550450 22469090e93e0eaefe2018462f8d06dded92ba45
child 550452 c5f9abd08d1216798d34f1785293bff11a1bff28
push id11858
push userrmaries@mozilla.com
push dateThu, 29 Aug 2019 15:29:30 +0000
treeherdermozilla-beta@e9268d2f3233 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjlast
bugs1574190
milestone70.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 1574190 - Add server support for watchpoints. r=jlast Lint and add test. Add server support for watchpoints. Differential Revision: https://phabricator.services.mozilla.com/D42926
devtools/server/actors/object.js
devtools/server/actors/thread.js
devtools/server/tests/unit/test_watchpoint-01.js
devtools/server/tests/unit/xpcshell.ini
devtools/shared/client/object-client.js
devtools/shared/specs/object.js
--- a/devtools/server/actors/object.js
+++ b/devtools/server/actors/object.js
@@ -66,16 +66,17 @@ const proto = {
    *          - decrementGripDepth
    *              Decrement the actor's grip depth
    *          - globalDebugObject
    *              The Debuggee Global Object as given by the ThreadActor
    */
   initialize(
     obj,
     {
+      thread,
       createValueGrip: createValueGripHook,
       sources,
       createEnvironmentActor,
       getGripDepth,
       incrementGripDepth,
       decrementGripDepth,
       getGlobalDebugObject,
     },
@@ -84,31 +85,89 @@ const proto = {
     assert(
       !obj.optimizedOut,
       "Should not create object actors for optimized out values!"
     );
     protocol.Actor.prototype.initialize.call(this, conn);
 
     this.conn = conn;
     this.obj = obj;
+    this.thread = thread;
     this.hooks = {
       createValueGrip: createValueGripHook,
       sources,
       createEnvironmentActor,
       getGripDepth,
       incrementGripDepth,
       decrementGripDepth,
       getGlobalDebugObject,
     };
+    this._originalDescriptors = new Map();
   },
 
   rawValue: function() {
     return this.obj.unsafeDereference();
   },
 
+  addWatchpoint(property, label, watchpointType) {
+    if (this._originalDescriptors.has(property)) {
+      return;
+    }
+    const desc = this.obj.getOwnPropertyDescriptor(property);
+
+    //If there is already a setter or getter, don't add watchpoint.
+    if (desc.set || desc.get) {
+      return;
+    }
+
+    this._originalDescriptors.set(property, desc);
+
+    const pauseAndRespond = () => {
+      const frame = this.thread.dbg.getNewestFrame();
+      this.thread._pauseAndRespond(frame, {
+        type: "watchpoint",
+        message: label,
+      });
+    };
+
+    if (watchpointType === "get") {
+      this.obj.defineProperty(property, {
+        configurable: desc.configurable,
+        enumerable: desc.enumerable,
+        set: this.obj.makeDebuggeeValue(v => {
+          desc.value = v;
+        }),
+        get: this.obj.makeDebuggeeValue(() => {
+          pauseAndRespond();
+        }),
+      });
+    }
+
+    if (watchpointType === "set") {
+      this.obj.defineProperty(property, {
+        configurable: desc.configurable,
+        enumerable: desc.enumerable,
+        set: this.obj.makeDebuggeeValue(v => {
+          desc.value = v;
+          pauseAndRespond();
+        }),
+      });
+    }
+  },
+
+  removeWatchpoint(property) {
+    if (!this._originalDescriptors.has(property)) {
+      return;
+    }
+
+    const desc = this._originalDescriptors.get(property);
+    this._originalDescriptors.delete(property);
+    this.obj.defineProperty(property, desc);
+  },
+
   /**
    * Returns a grip for this actor for returning in a protocol message.
    */
   form: function() {
     const g = {
       type: "object",
       actor: this.actorID,
     };
@@ -723,20 +782,24 @@ const proto = {
     const retval = {
       configurable: desc.configurable,
       enumerable: desc.enumerable,
     };
 
     if ("value" in desc) {
       retval.writable = desc.writable;
       retval.value = this.hooks.createValueGrip(desc.value);
+    } else if (this._originalDescriptors.has(name)) {
+      const value = this._originalDescriptors.get(name).value;
+      retval.value = this.hooks.createValueGrip(value);
     } else {
       if ("get" in desc) {
         retval.get = this.hooks.createValueGrip(desc.get);
       }
+
       if ("set" in desc) {
         retval.set = this.hooks.createValueGrip(desc.set);
       }
     }
     return retval;
   },
 
   /**
--- a/devtools/server/actors/thread.js
+++ b/devtools/server/actors/thread.js
@@ -1620,16 +1620,17 @@ const ThreadActor = ActorClassWithSpec(t
 
     if (this.threadLifetimePool.objectActors.has(value)) {
       return this.threadLifetimePool.objectActors.get(value).form();
     }
 
     const actor = new PauseScopedObjectActor(
       value,
       {
+        thread: this,
         getGripDepth: () => this._gripDepth,
         incrementGripDepth: () => this._gripDepth++,
         decrementGripDepth: () => this._gripDepth--,
         createValueGrip: v => {
           if (this._pausePool) {
             return createValueGrip(v, this._pausePool, this.pauseObjectGrip);
           }
 
new file mode 100644
--- /dev/null
+++ b/devtools/server/tests/unit/test_watchpoint-01.js
@@ -0,0 +1,138 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+/* eslint-disable no-shadow */
+
+"use strict";
+
+/*
+Tests adding set and get watchpoints.
+Tests removing a watchpoint.
+*/
+
+add_task(
+  threadFrontTest(async args => {
+    await testSetWatchpoint(args);
+    await testGetWatchpoint(args);
+    await testRemoveWatchpoint(args);
+  })
+);
+
+async function testSetWatchpoint({ threadFront, debuggee }) {
+  function evaluateTestCode(debuggee) {
+    /* eslint-disable */
+    Cu.evalInSandbox(
+      `                                   // 1
+      function stopMe(obj) {              // 2
+        debugger;                         // 3
+        obj.a = 2;                        // 4
+      }                                   // 
+      stopMe({a: 1})`,                           
+      debuggee,
+      "1.8",
+      "test_watchpoint-01.js",
+    );
+    /* eslint-disable */
+  }
+
+  const packet = await executeOnNextTickAndWaitForPause(
+    () => evaluateTestCode(debuggee),
+    threadFront
+  );
+
+  //Test that we paused on the debugger statement.
+  Assert.equal(packet.frame.where.line, 3);
+
+  //Add set watchpoint.
+  const args = packet.frame.arguments;
+  const obj = args[0];
+  const objClient = threadFront.pauseGrip(obj);
+  await objClient.addWatchpoint("a", "obj.a", "set");
+
+  //Test that watchpoint triggers pause on set.
+  const packet2 = await resumeAndWaitForPause(threadFront);
+  Assert.equal(packet2.frame.where.line, 4);
+  Assert.equal(packet2.why.type, "watchpoint");
+  Assert.equal(obj.preview.ownProperties.a.value, 1);
+  
+  await resume(threadFront);
+}
+
+async function testGetWatchpoint({ threadFront, debuggee }) {
+  function evaluateTestCode(debuggee) {
+    /* eslint-disable */
+    Cu.evalInSandbox(
+      `                                   // 1
+      function stopMe(obj) {              // 2
+        debugger;                         // 3
+        obj.a + 4;                        // 4
+      }                                   // 
+      stopMe({a: 1})`,                           
+      debuggee,
+      "1.8",
+      "test_watchpoint-01.js",
+    );
+    /* eslint-disable */
+  }
+
+  const packet = await executeOnNextTickAndWaitForPause(
+    () => evaluateTestCode(debuggee),
+    threadFront
+  );
+
+  //Test that we paused on the debugger statement.
+  Assert.equal(packet.frame.where.line, 3);
+
+  //Add get watchpoint.
+  const args = packet.frame.arguments;
+  const obj = args[0];
+  const objClient = threadFront.pauseGrip(obj);
+  await objClient.addWatchpoint("a", "obj.a", "get");
+
+  //Test that watchpoint triggers pause on get.
+  const packet2 = await resumeAndWaitForPause(threadFront);
+  Assert.equal(packet2.frame.where.line, 4);
+  Assert.equal(packet2.why.type, "watchpoint");
+  Assert.equal(obj.preview.ownProperties.a.value, 1);
+  
+  await resume(threadFront);
+}
+
+async function testRemoveWatchpoint({ threadFront, debuggee }) {
+  function evaluateTestCode(debuggee) {
+    /* eslint-disable */
+    Cu.evalInSandbox(
+      `                                   // 1
+      function stopMe(obj) {              // 2
+        debugger;                         // 3
+        obj.a = 2;                        // 4
+        debugger;                         // 5
+      }                                   // 
+      stopMe({a: 1})`,                           
+      debuggee,
+      "1.8",
+      "test_watchpoint-01.js",
+    );
+    /* eslint-disable */
+  }
+
+  const packet = await executeOnNextTickAndWaitForPause(
+    () => evaluateTestCode(debuggee),
+    threadFront
+  );
+  
+  //Test that we paused on the debugger statement.
+  Assert.equal(packet.frame.where.line, 3);
+
+  //Add and then remove set watchpoint.
+  const args = packet.frame.arguments;
+  const obj = args[0];
+  const objClient = threadFront.pauseGrip(obj);
+  await objClient.addWatchpoint("a", "obj.a", "set");
+  await objClient.removeWatchpoint("a");
+
+  //Test that we do not pause on set. 
+  const packet2 = await resumeAndWaitForPause(threadFront);
+  Assert.equal(packet2.frame.where.line, 5);
+  
+  await resume(threadFront);
+}
\ No newline at end of file
--- a/devtools/server/tests/unit/xpcshell.ini
+++ b/devtools/server/tests/unit/xpcshell.ini
@@ -210,16 +210,17 @@ skip-if = true # breakpoint sliding is n
 [test_pause_exceptions-04.js]
 [test_longstringactor.js]
 [test_longstringgrips-01.js]
 [test_source-01.js]
 [test_source-02.js]
 [test_source-03.js]
 [test_source-04.js]
 [test_wasm_source-01.js]
+[test_watchpoint-01.js]
 [test_breakpoint-actor-map.js]
 skip-if = true # tests for breakpoint actors are obsolete bug 1524374
 [test_unsafeDereference.js]
 [test_add_actors.js]
 [test_ignore_caught_exceptions.js]
 [test_ignore_no_interface_exceptions.js]
 [test_requestTypes.js]
 reason = bug 937197
--- a/devtools/shared/client/object-client.js
+++ b/devtools/shared/client/object-client.js
@@ -388,11 +388,21 @@ ObjectClient.prototype = {
         if (response.error === "unrecognizedPacketType") {
           const { proxyTarget, proxyHandler } = this._grip;
           return { proxyTarget, proxyHandler };
         }
         return response;
       },
     }
   ),
+  addWatchpoint: DebuggerClient.requester({
+    type: "addWatchpoint",
+    property: arg(0),
+    label: arg(1),
+    watchpointType: arg(2),
+  }),
+  removeWatchpoint: DebuggerClient.requester({
+    type: "removeWatchpoint",
+    property: arg(0),
+  }),
 };
 
 module.exports = ObjectClient;
--- a/devtools/shared/specs/object.js
+++ b/devtools/shared/specs/object.js
@@ -104,17 +104,16 @@ types.addDictType("object.originalSource
 
 types.addDictType("object.proxySlots", {
   proxyTarget: "object.descriptor",
   proxyHandler: "object.descriptor",
 });
 
 const objectSpec = generateActorSpec({
   typeName: "obj",
-
   methods: {
     allocationStack: {
       request: {},
       response: {
         allocationStack: RetVal("array:object.originalSourceLocation"),
       },
     },
     decompile: {
@@ -202,16 +201,31 @@ const objectSpec = generateActorSpec({
       response: {
         rejectionStack: RetVal("array:object.originalSourceLocation"),
       },
     },
     proxySlots: {
       request: {},
       response: RetVal("object.proxySlots"),
     },
+    addWatchpoint: {
+      request: {
+        property: Arg(0, "string"),
+        label: Arg(1, "string"),
+        watchpointType: Arg(2, "string"),
+      },
+      response: {},
+    },
+    removeWatchpoint: {
+      request: {
+        property: Arg(0, "string"),
+      },
+      response: {},
+    },
+
     release: { release: true },
     scope: {
       request: {},
       response: RetVal("object.scope"),
     },
     // Needed for the PauseScopedObjectActor which extends the ObjectActor.
     threadGrip: {
       request: {},