Bug 1448880 - Part 5: Tell removeFromFrameMapsAndClearBreakpointsIn() if we are suspending. r=jimb
authorJason Orendorff <jorendorff@mozilla.com>
Tue, 23 Oct 2018 23:36:35 +0000
changeset 491041 27b0121d9e314b26e2c11857b18409f5dabbd75b
parent 491040 61031045a58cad9c96f2051a858031bf37443769
child 491042 6c8949f053e0e9c6216329d9b3f081fc0b6b2936
push id247
push userfmarier@mozilla.com
push dateSat, 27 Oct 2018 01:06:44 +0000
reviewersjimb
bugs1448880
milestone65.0a1
Bug 1448880 - Part 5: Tell removeFromFrameMapsAndClearBreakpointsIn() if we are suspending. r=jimb Since the argument is not used yet, this too is a pure refactoring, with no change in behavior yet. Depends on D6985 Differential Revision: https://phabricator.services.mozilla.com/D6986
js/src/vm/Debugger.cpp
js/src/vm/Debugger.h
--- a/js/src/vm/Debugger.cpp
+++ b/js/src/vm/Debugger.cpp
@@ -1065,19 +1065,36 @@ class MOZ_RAII AutoSetGeneratorRunning
  * the frame is exiting normally or abruptly. Set |cx|'s exception and/or
  * |cx->fp()|'s return value, and return a new success value.
  */
 /* static */ bool
 Debugger::slowPathOnLeaveFrame(JSContext* cx, AbstractFramePtr frame, jsbytecode* pc, bool frameOk)
 {
     mozilla::DebugOnly<Handle<GlobalObject*>> debuggeeGlobal = cx->global();
 
+    // Determine if we are suspending this frame or popping it forever.
+    bool suspending = false;
+    Rooted<GeneratorObject*> genObj(cx);
+    if (frame.isGeneratorFrame()) {
+        // If we're leaving successfully at a yield opcode, we're probably
+        // suspending; the `isClosed()` check detects a debugger forced return
+        // from an `onStep` handler, which looks almost the same.
+        genObj = GetGeneratorObjectForFrame(cx, frame);
+        suspending =
+            frameOk &&
+            pc && (*pc == JSOP_INITIALYIELD || *pc == JSOP_YIELD || *pc == JSOP_AWAIT) &&
+            !genObj->isClosed();
+    }
+
+    bool success = false;
     auto frameMapsGuard = MakeScopeExit([&] {
-        // Clean up all Debugger.Frame instances.
-        removeFromFrameMapsAndClearBreakpointsIn(cx, frame);
+        // Clean up all Debugger.Frame instances on exit. On suspending, pass
+        // the flag that says to leave those frames `.live`. Note that if
+        // suspending && !success, the generator is closed, not suspended.
+        removeFromFrameMapsAndClearBreakpointsIn(cx, frame, suspending && success);
     });
 
     // The onPop handler and associated clean up logic should not run multiple
     // times on the same frame. If slowPathOnLeaveFrame has already been
     // called, the frame will not be present in the Debugger frame maps.
     Rooted<DebuggerFrameVector> frames(cx, DebuggerFrameVector(cx));
     if (!getDebuggerFrames(frame, &frames)) {
         return false;
@@ -1086,23 +1103,16 @@ Debugger::slowPathOnLeaveFrame(JSContext
         return frameOk;
     }
 
     // Save the frame's completion value.
     ResumeMode resumeMode;
     RootedValue value(cx);
     Debugger::resultToCompletion(cx, frameOk, frame.returnValue(), &resumeMode, &value);
 
-    // If we are yielding or awaiting, we'll need to mark the generator as
-    // "running" temporarily.
-    Rooted<GeneratorObject*> genObj(cx);
-    if (frame.isFunctionFrame() && (frame.callee()->isGenerator() || frame.callee()->isAsync())) {
-        genObj = GetGeneratorObjectForFrame(cx, frame);
-    }
-
     // This path can be hit via unwinding the stack due to over-recursion or
     // OOM. In those cases, don't fire the frames' onPop handlers, because
     // invoking JS will only trigger the same condition. See
     // slowPathOnExceptionUnwind.
     if (!cx->isThrowingOverRecursed() && !cx->isThrowingOutOfMemory()) {
         // For each Debugger.Frame, fire its onPop handler, if any.
         for (size_t i = 0; i < frames.length(); i++) {
             HandleDebuggerFrame frameobj = frames[i];
@@ -1146,16 +1156,17 @@ Debugger::slowPathOnLeaveFrame(JSContext
             }
         }
     }
 
     // Establish (resumeMode, value) as our resumption value.
     switch (resumeMode) {
       case ResumeMode::Return:
         frame.setReturnValue(value);
+        success = true;
         return true;
 
       case ResumeMode::Throw:
         cx->setPendingException(value);
         return false;
 
       case ResumeMode::Terminate:
         MOZ_ASSERT(!cx->isExceptionPending());
@@ -7284,17 +7295,18 @@ Debugger::replaceFrameGuts(JSContext* cx
 Debugger::inFrameMaps(AbstractFramePtr frame)
 {
     bool foundAny = false;
     forEachDebuggerFrame(frame, [&](DebuggerFrame* frameobj) { foundAny = true; });
     return foundAny;
 }
 
 /* static */ void
-Debugger::removeFromFrameMapsAndClearBreakpointsIn(JSContext* cx, AbstractFramePtr frame)
+Debugger::removeFromFrameMapsAndClearBreakpointsIn(JSContext* cx, AbstractFramePtr frame,
+                                                   bool suspending)
 {
     forEachDebuggerFrame(frame, [&](DebuggerFrame* frameobj) {
         Debugger* dbg = Debugger::fromChildJSObject(frameobj);
 
         FreeOp* fop = cx->runtime()->defaultFreeOp();
         frameobj->freeFrameIterData(fop);
         DebuggerFrame_maybeDecrementFrameScriptStepModeCount(fop, frame, frameobj);
 
--- a/js/src/vm/Debugger.h
+++ b/js/src/vm/Debugger.h
@@ -784,17 +784,18 @@ class Debugger : private mozilla::Linked
     static bool drainTraceLogger(JSContext* cx, unsigned argc, Value* vp);
 #endif
     static bool adoptDebuggeeValue(JSContext* cx, unsigned argc, Value* vp);
     static bool construct(JSContext* cx, unsigned argc, Value* vp);
     static const JSPropertySpec properties[];
     static const JSFunctionSpec methods[];
     static const JSFunctionSpec static_methods[];
 
-    static void removeFromFrameMapsAndClearBreakpointsIn(JSContext* cx, AbstractFramePtr frame);
+    static void removeFromFrameMapsAndClearBreakpointsIn(JSContext* cx, AbstractFramePtr frame,
+                                                         bool suspending = false);
     static bool updateExecutionObservabilityOfFrames(JSContext* cx, const ExecutionObservableSet& obs,
                                                      IsObserving observing);
     static bool updateExecutionObservabilityOfScripts(JSContext* cx, const ExecutionObservableSet& obs,
                                                       IsObserving observing);
     static bool updateExecutionObservability(JSContext* cx, ExecutionObservableSet& obs,
                                              IsObserving observing);
 
     template <typename FrameFn /* void (DebuggerFrame*) */>