Bug 1570178 - Part 3: Support setting onStep when the frame is not live. r=jimb

We want to be able to set onStep when generators are suspended, and since that
already requires changes, it is easy to expand that and allow setting onStep
even after a frame is entirely dead. This simplifies the implementation,
and better matches user expectations around object properties not throwing
or vanishing due to actions outside their control.

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

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

js/src/debugger/Frame.cpp

@@ -679,25 +679,39 @@ DebuggerFrameImplementation DebuggerFrame::getImplementation(
 /* static */
 bool DebuggerFrame::setOnStepHandler(JSContext* cx, HandleDebuggerFrame frame,
                                      OnStepHandler* handler) {
-  MOZ_ASSERT(frame->isLive());
-
   OnStepHandler* prior = frame->onStepHandler();
   if (handler == prior) {
     return true;
   }
 
   JSFreeOp* fop = cx->defaultFreeOp();
-  AbstractFramePtr referent = DebuggerFrame::getReferent(frame);
+  if (frame->isLive()) {
+    AbstractFramePtr referent = DebuggerFrame::getReferent(frame);
 
-  // Adjust execution observability and step counts on whatever code (JS or
+    // Adjust execution observability and step counts on whatever code (JS or
-  // Wasm) this frame is running.
+    // Wasm) this frame is running.
-  if (handler) {
+    if (handler) {
-    if (!frame->maybeIncrementStepperCounter(cx, referent)) {
+      if (!frame->maybeIncrementStepperCounter(cx, referent)) {
-      return false;
+        return false;
+      }
+    } else {
+      frame->maybeDecrementStepperCounter(cx->runtime()->defaultFreeOp(),
+                                          referent);
+    }
+  } else if (frame->hasGenerator()) {
+    RootedScript script(cx, frame->generatorInfo()->generatorScript());
+
+    if (handler) {
+      if (!frame->maybeIncrementStepperCounter(cx, script)) {
+        return false;
+      }
+    } else {
+      frame->maybeDecrementStepperCounter(cx->runtime()->defaultFreeOp(),
+                                          script);
     }
   } else {
-    frame->maybeDecrementStepperCounter(cx->runtime()->defaultFreeOp(),
+    // If the frame is entirely dead, we still allow setting the onStep
-                                        referent);
+    // handler, but it has no effect.
   }
 
   // Now that the stepper counts and observability are set correctly, we can
@@ -1215,8 +1229,10 @@ bool DebuggerFrame::CallData::ToNative(JSContext* cx, unsigned argc,
                                        Value* vp) {
   CallArgs args = CallArgsFromVp(argc, vp);
 
-  // All accessors/methods require a live frame, except for the live getter.
+  // These methods do not require liveness.
-  bool checkLive = MyMethod != &CallData::liveGetter;
+  bool checkLive = MyMethod != &CallData::liveGetter &&
+                   MyMethod != &CallData::onStepGetter &&
+                   MyMethod != &CallData::onStepSetter;
 
   RootedDebuggerFrame frame(cx,
                             DebuggerFrame::check(cx, args.thisv(), checkLive));

js/src/jit-test/tests/debug/Frame-onStep-assign-function.js

@@ -0,0 +1,49 @@
+// Changing onStep while the function is dead is allowed.
+
+load(libdir + "asserts.js");
+
+const g = newGlobal({ newCompartment: true });
+const dbg = new Debugger(g);
+
+let steps = new Set();
+dbg.onDebuggerStatement = function(frame) {
+  // Setting 'onStep' while alive is allowed.
+  steps.add("debugger 1");
+  assertEq(frame.live, true);
+  frame.onStep = function() {
+    steps.add("onstep 1");
+  };
+
+  dbg.onDebuggerStatement = function() {
+    // Clear the 'onStep' while dead.
+    steps.add("debugger 2");
+    assertEq(frame.live, false);
+
+    // Clearing 'onStep' while dead is allowed.
+    frame.onStep = undefined;
+
+      // Setting 'onStep' while dead is allowed.
+    frame.onStep = function() {
+      steps.add("onstep 2");
+    };
+  };
+};
+
+g.eval(
+  `
+    function myGen() {
+      debugger;
+      print("make sure we have a place to step to inside the frame");
+    }
+
+    const g = myGen();
+
+    debugger;
+  `
+);
+
+assertDeepEq(Array.from(steps), [
+  "debugger 1",
+  "onstep 1",
+  "debugger 2",
+]);

js/src/jit-test/tests/debug/Frame-onStep-assign-generator.js

@@ -0,0 +1,68 @@
+// Changing onStep while the generator is suspended/dead is allowed.
+
+load(libdir + "asserts.js");
+
+const g = newGlobal({ newCompartment: true });
+const dbg = new Debugger(g);
+
+let steps = new Set();
+dbg.onDebuggerStatement = function(frame) {
+  // Setting 'onStep' while alive is allowed.
+  steps.add("debugger 1");
+  assertEq(frame.live, true);
+  frame.onStep = function() {
+    steps.add("onstep 1");
+  };
+
+  dbg.onDebuggerStatement = function() {
+    // Clear the 'onStep' while suspended.
+    steps.add("debugger 2");
+    assertEq(frame.live, false);
+
+    // Clearing 'onStep' while suspended is allowed.
+    frame.onStep = undefined;
+
+      // Setting 'onStep' while suspended is allowed.
+    frame.onStep = function() {
+      steps.add("onstep 2");
+    };
+
+    dbg.onDebuggerStatement = function() {
+      steps.add("debugger 3");
+      assertEq(frame.live, false);
+
+      // Clearing 'onStep' while dead is allowed.
+      frame.onStep = undefined;
+
+        // Setting 'onStep' while dead is allowed.
+      frame.onStep = function() {
+        steps.add("onstep 3");
+      };
+    };
+  };
+};
+
+g.eval(
+  `
+    function* myGen() {
+      debugger;
+      yield;
+    }
+
+    const g = myGen();
+    g.next();
+
+    debugger;
+    g.next();
+
+    debugger;
+  `
+);
+
+assertDeepEq(Array.from(steps), [
+  "debugger 1",
+  "onstep 1",
+  "debugger 2",
+  "onstep 2",
+  "debugger 3",
+]);