Bug 903754 - Remove InterpreterFrames class and use InterpreterActivation instead. r=luke

js/src/jsdbgapi.cpp

@@ -267,8 +267,12 @@ JS_SetInterrupt(JSRuntime *rt, JSInterruptHook hook, void *closure)
 {
     rt->debugHooks.interruptHook = hook;
     rt->debugHooks.interruptHookData = closure;
-    for (InterpreterFrames *f = rt->interpreterFrames; f; f = f->older)
-        f->enableInterruptsUnconditionally();
+
+    for (ActivationIterator iter(rt); !iter.done(); ++iter) {
+        if (iter.activation()->isInterpreter())
+            iter.activation()->asInterpreter()->enableInterruptsUnconditionally();
+    }
+
     return true;
 }
 

js/src/jsscript.cpp

@@ -864,9 +864,10 @@ JSScript::initScriptCounts(JSContext *cx)
     JS_ASSERT(size_t(cursor - base) == bytes);
 
     /* Enable interrupts in any interpreter frames running on this script. */
-    InterpreterFrames *frames;
-    for (frames = cx->runtime()->interpreterFrames; frames; frames = frames->older)
-        frames->enableInterruptsIfRunning(this);
+    for (ActivationIterator iter(cx->runtime()); !iter.done(); ++iter) {
+        if (iter.activation()->isInterpreter())
+            iter.activation()->asInterpreter()->enableInterruptsIfRunning(this);
+    }
 
     return true;
 }
@@ -2611,9 +2612,10 @@ JSScript::ensureHasDebugScript(JSContext *cx)
      * interrupts enabled. The interrupts must stay enabled until the
      * debug state is destroyed.
      */
-    InterpreterFrames *frames;
-    for (frames = cx->runtime()->interpreterFrames; frames; frames = frames->older)
-        frames->enableInterruptsIfRunning(this);
+    for (ActivationIterator iter(cx->runtime()); !iter.done(); ++iter) {
+        if (iter.activation()->isInterpreter())
+            iter.activation()->asInterpreter()->enableInterruptsIfRunning(this);
+    }
 
     return true;
 }

js/src/vm/Interpreter.cpp

@@ -975,30 +975,6 @@ TryNoteIter::settle()
             goto error;                                                       \
     JS_END_MACRO
 
-template<typename T>
-class GenericInterruptEnabler : public InterpreterFrames::InterruptEnablerBase {
-  public:
-    GenericInterruptEnabler(T *variable, T value) : variable(variable), value(value) { }
-    void enable() const { *variable = value; }
-
-  private:
-    T *variable;
-    T value;
-};
-
-inline InterpreterFrames::InterpreterFrames(JSContext *cx, FrameRegs *regs,
-                                            const InterruptEnablerBase &enabler)
-  : context(cx), regs(regs), enabler(enabler)
-{
-    older = cx->runtime()->interpreterFrames;
-    cx->runtime()->interpreterFrames = this;
-}
-
-inline InterpreterFrames::~InterpreterFrames()
-{
-    context->runtime()->interpreterFrames = older;
-}
-
 /*
  * Ensure that the interpreter switch can close call-bytecode cases in the
  * same way as non-call bytecodes.
@@ -1287,10 +1263,19 @@ Interpret(JSContext *cx, RunState &state)
 
 #define CHECK_PCCOUNT_INTERRUPTS() JS_ASSERT_IF(script->hasScriptCounts, switchMask == -1)
 
+    /*
+     * When Debugger puts a script in single-step mode, all js::Interpret
+     * invocations that might be presently running that script must have
+     * interrupts enabled. It's not practical to simply check
+     * script->stepModeEnabled() at each point some callee could have changed
+     * it, because there are so many places js::Interpret could possibly cause
+     * JavaScript to run: each place an object might be coerced to a primitive
+     * or a number, for example. So instead, we expose a simple mechanism to
+     * let Debugger tweak the affected js::Interpret frames when an onStep
+     * handler is added: setting switchMask to -1 will enable interrupts.
+     */
     register int switchMask = 0;
     int switchOp;
-    typedef GenericInterruptEnabler<int> InterruptEnabler;
-    InterruptEnabler interrupts(&switchMask, -1);
 
 # define DO_OP()            goto do_op
 
@@ -1345,7 +1330,7 @@ Interpret(JSContext *cx, RunState &state)
     JS_BEGIN_MACRO                                                            \
         script = (s);                                                         \
         if (script->hasAnyBreakpointsOrStepMode() || script->hasScriptCounts) \
-            interrupts.enable();                                              \
+            switchMask = -1; /* Enable interrupts. */                         \
     JS_END_MACRO
 
     FrameRegs regs;
@@ -1364,13 +1349,7 @@ Interpret(JSContext *cx, RunState &state)
 
     JS_ASSERT_IF(entryFrame->isEvalFrame(), state.script()->isActiveEval);
 
-    InterpreterActivation activation(cx, entryFrame, regs);
-
-    /*
-     * Help Debugger find frames running scripts that it has put in
-     * single-step mode.
-     */
-    InterpreterFrames interpreterFrame(cx, &regs, interrupts);
+    InterpreterActivation activation(cx, entryFrame, regs, &switchMask);
 
     /* Copy in hot values that change infrequently. */
     JSRuntime *const rt = cx->runtime();
@@ -1450,7 +1429,7 @@ Interpret(JSContext *cx, RunState &state)
     len = 0;
 
     if (rt->profilingScripts || cx->runtime()->debugHooks.interruptHook)
-        interrupts.enable();
+        switchMask = -1; /* Enable interrupts. */
 
     goto advanceAndDoOp;
 

js/src/vm/Interpreter.h

@@ -315,54 +315,6 @@ TypeOfValue(JSContext *cx, const Value &v);
 extern bool
 HasInstance(JSContext *cx, HandleObject obj, HandleValue v, bool *bp);
 
-/*
- * A linked list of the |FrameRegs regs;| variables belonging to all
- * js::Interpret C++ frames on this thread's stack.
- *
- * Note that this is *not* a list of all JS frames running under the
- * interpreter; that would include inlined frames, whose FrameRegs are
- * saved in various pieces in various places. Rather, this lists each
- * js::Interpret call's live 'regs'; when control returns to that call, it
- * will resume execution with this 'regs' instance.
- *
- * When Debugger puts a script in single-step mode, all js::Interpret
- * invocations that might be presently running that script must have
- * interrupts enabled. It's not practical to simply check
- * script->stepModeEnabled() at each point some callee could have changed
- * it, because there are so many places js::Interpret could possibly cause
- * JavaScript to run: each place an object might be coerced to a primitive
- * or a number, for example. So instead, we simply expose a list of the
- * 'regs' those frames are using, and let Debugger tweak the affected
- * js::Interpret frames when an onStep handler is established.
- *
- * Elements of this list are allocated within the js::Interpret stack
- * frames themselves; the list is headed by this thread's js::ThreadData.
- */
-class InterpreterFrames {
-  public:
-    class InterruptEnablerBase {
-      public:
-        virtual void enable() const = 0;
-    };
-
-    InterpreterFrames(JSContext *cx, FrameRegs *regs, const InterruptEnablerBase &enabler);
-    ~InterpreterFrames();
-
-    /* If this js::Interpret frame is running |script|, enable interrupts. */
-    void enableInterruptsIfRunning(JSScript *script) {
-        if (regs->fp()->script() == script)
-            enabler.enable();
-    }
-    void enableInterruptsUnconditionally() { enabler.enable(); }
-
-    InterpreterFrames *older;
-
-  private:
-    JSContext *context;
-    FrameRegs *regs;
-    const InterruptEnablerBase &enabler;
-};
-
 /* Unwind block and scope chains to match the given depth. */
 extern void
 UnwindScope(JSContext *cx, AbstractFramePtr frame, uint32_t stackDepth);

js/src/vm/Runtime.cpp

@@ -127,7 +127,6 @@ JSRuntime::JSRuntime(JSUseHelperThreads useHelperThreads)
     selfHostingGlobal_(NULL),
     nativeStackBase(0),
     nativeStackQuota(0),
-    interpreterFrames(NULL),
     cxCallback(NULL),
     destroyCompartmentCallback(NULL),
     compartmentNameCallback(NULL),

js/src/vm/Runtime.h

@@ -76,7 +76,6 @@ typedef Rooted<JSLinearString*> RootedLinearString;
 class Activation;
 class ActivationIterator;
 class AsmJSActivation;
-class InterpreterFrames;
 class MathCache;
 class WorkerThreadState;
 
@@ -917,12 +916,6 @@ struct JSRuntime : public JS::shadow::Runtime,
     /* The native stack size limit that runtime should not exceed. */
     size_t              nativeStackQuota;
 
-    /*
-     * Frames currently running in js::Interpret. See InterpreterFrames for
-     * details.
-     */
-    js::InterpreterFrames *interpreterFrames;
-
     /* Context create/destroy callback. */
     JSContextCallback   cxCallback;
     void               *cxCallbackData;

js/src/vm/Stack-inl.h

@@ -833,11 +833,12 @@ Activation::~Activation()
     cx_->mainThread().activation_ = prev_;
 }
 
-InterpreterActivation::InterpreterActivation(JSContext *cx, StackFrame *entry, FrameRegs &regs)
+InterpreterActivation::InterpreterActivation(JSContext *cx, StackFrame *entry, FrameRegs &regs,
+                                             int *const switchMask)
   : Activation(cx, Interpreter),
     entry_(entry),
-    current_(entry),
-    regs_(regs)
+    regs_(regs),
+    switchMask_(switchMask)
 #ifdef DEBUG
   , oldFrameCount_(cx_->runtime()->interpreterStack().frameCount_)
 #endif
@@ -846,8 +847,8 @@ InterpreterActivation::InterpreterActivation(JSContext *cx, StackFrame *entry, F
 InterpreterActivation::~InterpreterActivation()
 {
     // Pop all inline frames.
-    while (current_ != entry_)
-        popInlineFrame(current_);
+    while (regs_.fp() != entry_)
+        popInlineFrame(regs_.fp());
 
     JS_ASSERT(oldFrameCount_ == cx_->runtime()->interpreterStack().frameCount_);
     JS_ASSERT_IF(oldFrameCount_ == 0, cx_->runtime()->interpreterStack().allocator_.used() == 0);
@@ -860,18 +861,15 @@ InterpreterActivation::pushInlineFrame(const CallArgs &args, HandleScript script
     if (!cx_->runtime()->interpreterStack().pushInlineFrame(cx_, regs_, args, script, initial))
         return false;
     JS_ASSERT(regs_.fp()->script()->compartment() == compartment_);
-    current_ = regs_.fp();
     return true;
 }
 
 inline void
 InterpreterActivation::popInlineFrame(StackFrame *frame)
 {
-    JS_ASSERT(current_ == frame);
-    JS_ASSERT(current_ != entry_);
-
-    current_ = frame->prev();
-    JS_ASSERT(current_);
+    (void)frame; // Quell compiler warning.
+    JS_ASSERT(regs_.fp() == frame);
+    JS_ASSERT(regs_.fp() != entry_);
 
     cx_->runtime()->interpreterStack().popInlineFrame(regs_);
 }

js/src/vm/Stack.h

@@ -1226,15 +1226,16 @@ class InterpreterActivation : public Activation
     friend class js::InterpreterFrameIterator;
 
     StackFrame *const entry_; // Entry frame for this activation.
-    StackFrame *current_;     // The most recent frame.
     FrameRegs &regs_;
+    int *const switchMask_; // For debugger interrupts, see js::Interpret.
 
 #ifdef DEBUG
     size_t oldFrameCount_;
 #endif
 
   public:
-    inline InterpreterActivation(JSContext *cx, StackFrame *entry, FrameRegs &regs);
+    inline InterpreterActivation(JSContext *cx, StackFrame *entry, FrameRegs &regs,
+                                 int *const switchMask);
     inline ~InterpreterActivation();
 
     inline bool pushInlineFrame(const CallArgs &args, HandleScript script,
@@ -1242,12 +1243,20 @@ class InterpreterActivation : public Activation
     inline void popInlineFrame(StackFrame *frame);
 
     StackFrame *current() const {
-        JS_ASSERT(current_);
-        return current_;
+        return regs_.fp();
     }
     FrameRegs &regs() const {
         return regs_;
     }
+
+    // If this js::Interpret frame is running |script|, enable interrupts.
+    void enableInterruptsIfRunning(JSScript *script) {
+        if (regs_.fp()->script() == script)
+            enableInterruptsUnconditionally();
+    }
+    void enableInterruptsUnconditionally() {
+        *switchMask_ = -1;
+    }
 };
 
 // Iterates over a runtime's activation list.