Bug 1276162 part 2 - Remove ContextOption and make frame iterators always see all frames. r=luke
authorJan de Mooij <jdemooij@mozilla.com>
Sat, 28 May 2016 10:34:11 +0200
changeset 340441 54abdc6d524e87f3714d90ee2df276caa999b8f5
parent 340440 d04a27a81ae928cd3535ffb34595ba5d2248a1e8
child 340442 511ad37f594b9984c881724abba96821ccd91716
push id1183
push userraliiev@mozilla.com
push dateMon, 05 Sep 2016 20:01:49 +0000
treeherdermozilla-release@3148731bed45 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersluke
bugs1276162
milestone49.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 1276162 part 2 - Remove ContextOption and make frame iterators always see all frames. r=luke
js/src/frontend/TokenStream.cpp
js/src/vm/Debugger.cpp
js/src/vm/SavedStacks.cpp
js/src/vm/Stack.cpp
js/src/vm/Stack.h
--- a/js/src/frontend/TokenStream.cpp
+++ b/js/src/frontend/TokenStream.cpp
@@ -631,17 +631,16 @@ TokenStream::reportCompileErrorNumberVA(
         err.report.lineno = srcCoords.lineNum(offset);
         err.report.column = srcCoords.columnIndex(offset);
     }
 
     // If we have no location information, try to get one from the caller.
     bool callerFilename = false;
     if (offset != NoOffset && !err.report.filename && cx->isJSContext()) {
         NonBuiltinFrameIter iter(cx->asJSContext(),
-                                 FrameIter::ALL_CONTEXTS,
                                  FrameIter::FOLLOW_DEBUGGER_EVAL_PREV_LINK,
                                  cx->compartment()->principals());
         if (!iter.done() && iter.filename()) {
             callerFilename = true;
             err.report.filename = iter.filename();
             err.report.lineno = iter.computeLine(&err.report.column);
         }
     }
--- a/js/src/vm/Debugger.cpp
+++ b/js/src/vm/Debugger.cpp
@@ -2204,17 +2204,17 @@ Debugger::updateExecutionObservabilityOf
         jit::JitContext jctx(cx, nullptr);
         if (!jit::RecompileOnStackBaselineScriptsForDebugMode(cx, obs, observing)) {
             ReportOutOfMemory(cx);
             return false;
         }
     }
 
     AbstractFramePtr oldestEnabledFrame;
-    for (ScriptFrameIter iter(cx, ScriptFrameIter::ALL_CONTEXTS);
+    for (ScriptFrameIter iter(cx);
          !iter.done();
          ++iter)
     {
         if (obs.shouldMarkAsDebuggee(iter)) {
             if (observing) {
                 if (!iter.abstractFramePtr().isDebuggee()) {
                     oldestEnabledFrame = iter.abstractFramePtr();
                     oldestEnabledFrame.setIsDebuggee();
@@ -2510,17 +2510,17 @@ Debugger::updateObservesCoverageOnDebugg
         // dangling pointers to freed PCCounts.
         if (!obs.add(comp))
             return false;
     }
 
     // If any frame on the stack belongs to the debuggee, then we cannot update
     // the ScriptCounts, because this would imply to invalidate a Debugger.Frame
     // to recompile it with/without ScriptCount support.
-    for (ScriptFrameIter iter(cx, ScriptFrameIter::ALL_CONTEXTS);
+    for (ScriptFrameIter iter(cx);
          !iter.done();
          ++iter)
     {
         if (obs.shouldMarkAsDebuggee(iter)) {
             JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_DEBUG_NOT_IDLE);
             return false;
         }
     }
@@ -7004,18 +7004,17 @@ CheckThisFrame(JSContext* cx, const Call
 #define THIS_FRAME_ITER(cx, argc, vp, fnname, args, thisobj, maybeIter, iter)  \
     THIS_FRAME_THISOBJ(cx, argc, vp, fnname, args, thisobj);                   \
     Maybe<ScriptFrameIter> maybeIter;                                          \
     {                                                                          \
         AbstractFramePtr f = AbstractFramePtr::FromRaw(thisobj->getPrivate()); \
         if (f.isScriptFrameIterData()) {                                       \
             maybeIter.emplace(*(ScriptFrameIter::Data*)(f.raw()));             \
         } else {                                                               \
-            maybeIter.emplace(cx, ScriptFrameIter::ALL_CONTEXTS,               \
-                              ScriptFrameIter::IGNORE_DEBUGGER_EVAL_PREV_LINK); \
+            maybeIter.emplace(cx, ScriptFrameIter::IGNORE_DEBUGGER_EVAL_PREV_LINK); \
             ScriptFrameIter& iter = *maybeIter;                                \
             while (!iter.hasUsableAbstractFramePtr() || iter.abstractFramePtr() != f) \
                 ++iter;                                                        \
             AbstractFramePtr data = iter.copyDataAsAbstractFramePtr();         \
             if (!data)                                                         \
                 return false;                                                  \
             thisobj->setPrivate(data.raw());                                   \
         }                                                                      \
--- a/js/src/vm/SavedStacks.cpp
+++ b/js/src/vm/SavedStacks.cpp
@@ -1078,17 +1078,17 @@ SavedStacks::saveCurrentStack(JSContext*
         !cx->global() ||
         !cx->global()->isStandardClassResolved(JSProto_Object))
     {
         frame.set(nullptr);
         return true;
     }
 
     AutoSPSEntry psuedoFrame(cx->runtime(), "js::SavedStacks::saveCurrentStack");
-    FrameIter iter(cx, FrameIter::ALL_CONTEXTS);
+    FrameIter iter(cx);
     return insertFrames(cx, iter, frame, maxFrameCount);
 }
 
 bool
 SavedStacks::copyAsyncStack(JSContext* cx, HandleObject asyncStack, HandleString asyncCause,
                             MutableHandleSavedFrame adoptedStack, unsigned maxFrameCount)
 {
     MOZ_ASSERT(initialized());
--- a/js/src/vm/Stack.cpp
+++ b/js/src/vm/Stack.cpp
@@ -512,24 +512,16 @@ FrameIter::settleOnActivation()
     while (true) {
         if (data_.activations_.done()) {
             data_.state_ = DONE;
             return;
         }
 
         Activation* activation = data_.activations_.activation();
 
-        // Skip activations from another context if needed.
-        MOZ_ASSERT(activation->cx());
-        MOZ_ASSERT(data_.cx_);
-        if (data_.contextOption_ == CURRENT_CONTEXT && activation->cx() != data_.cx_) {
-            ++data_.activations_;
-            continue;
-        }
-
         // If the caller supplied principals, only show activations which are subsumed (of the same
         // origin or of an origin accessible) by these principals.
         if (data_.principals_) {
             JSContext* cx = data_.cx_;
             if (JSSubsumesOp subsumes = cx->runtime()->securityCallbacks->subsumes) {
                 if (!subsumes(data_.principals_, activation->compartment()->principals())) {
                     ++data_.activations_;
                     continue;
@@ -585,69 +577,56 @@ FrameIter::settleOnActivation()
 
         MOZ_ASSERT(!data_.interpFrames_.frame()->runningInJit());
         data_.pc_ = data_.interpFrames_.pc();
         data_.state_ = INTERP;
         return;
     }
 }
 
-FrameIter::Data::Data(JSContext* cx, ContextOption contextOption,
-                      DebuggerEvalOption debuggerEvalOption, JSPrincipals* principals)
+FrameIter::Data::Data(JSContext* cx, DebuggerEvalOption debuggerEvalOption,
+                      JSPrincipals* principals)
   : cx_(cx),
-    contextOption_(contextOption),
     debuggerEvalOption_(debuggerEvalOption),
     principals_(principals),
     pc_(nullptr),
     interpFrames_(nullptr),
     activations_(cx->runtime()),
     jitFrames_(),
     ionInlineFrameNo_(0),
     wasmFrames_()
 {
 }
 
 FrameIter::Data::Data(const FrameIter::Data& other)
   : cx_(other.cx_),
-    contextOption_(other.contextOption_),
     debuggerEvalOption_(other.debuggerEvalOption_),
     principals_(other.principals_),
     state_(other.state_),
     pc_(other.pc_),
     interpFrames_(other.interpFrames_),
     activations_(other.activations_),
     jitFrames_(other.jitFrames_),
     ionInlineFrameNo_(other.ionInlineFrameNo_),
     wasmFrames_(other.wasmFrames_)
 {
 }
 
-FrameIter::FrameIter(JSContext* cx)
-  : data_(cx, CURRENT_CONTEXT, FOLLOW_DEBUGGER_EVAL_PREV_LINK, nullptr),
+FrameIter::FrameIter(JSContext* cx, DebuggerEvalOption debuggerEvalOption)
+  : data_(cx, debuggerEvalOption, nullptr),
     ionInlineFrames_(cx, (js::jit::JitFrameIterator*) nullptr)
 {
     // settleOnActivation can only GC if principals are given.
     JS::AutoSuppressGCAnalysis nogc;
     settleOnActivation();
 }
 
-FrameIter::FrameIter(JSContext* cx, ContextOption contextOption,
-                     DebuggerEvalOption debuggerEvalOption)
-  : data_(cx, contextOption, debuggerEvalOption, nullptr),
-    ionInlineFrames_(cx, (js::jit::JitFrameIterator*) nullptr)
-{
-    // settleOnActivation can only GC if principals are given.
-    JS::AutoSuppressGCAnalysis nogc;
-    settleOnActivation();
-}
-
-FrameIter::FrameIter(JSContext* cx, ContextOption contextOption,
-                     DebuggerEvalOption debuggerEvalOption,
+FrameIter::FrameIter(JSContext* cx, DebuggerEvalOption debuggerEvalOption,
                      JSPrincipals* principals)
-  : data_(cx, contextOption, debuggerEvalOption, principals),
+  : data_(cx, debuggerEvalOption, principals),
     ionInlineFrames_(cx, (js::jit::JitFrameIterator*) nullptr)
 {
     settleOnActivation();
 }
 
 FrameIter::FrameIter(const FrameIter& other)
   : data_(other.data_),
     ionInlineFrames_(other.data_.cx_,
@@ -720,30 +699,25 @@ FrameIter::operator++()
         MOZ_CRASH("Unexpected state");
       case INTERP:
         if (interpFrame()->isDebuggerEvalFrame() &&
             interpFrame()->evalInFramePrev() &&
             data_.debuggerEvalOption_ == FOLLOW_DEBUGGER_EVAL_PREV_LINK)
         {
             AbstractFramePtr eifPrev = interpFrame()->evalInFramePrev();
 
-            // Eval-in-frame can cross contexts.
-            ContextOption prevContextOption = data_.contextOption_;
-            data_.contextOption_ = ALL_CONTEXTS;
-
             popInterpreterFrame();
 
             while (!hasUsableAbstractFramePtr() || abstractFramePtr() != eifPrev) {
                 if (data_.state_ == JIT)
                     popJitFrame();
                 else
                     popInterpreterFrame();
             }
 
-            data_.contextOption_ = prevContextOption;
             data_.cx_ = data_.activations_->cx();
             break;
         }
         popInterpreterFrame();
         break;
       case JIT:
         popJitFrame();
         break;
--- a/js/src/vm/Stack.h
+++ b/js/src/vm/Stack.h
@@ -1700,67 +1700,61 @@ class WasmActivation : public Activation
     void* resumePC() const { return resumePC_; }
 };
 
 // A FrameIter walks over the runtime's stack of JS script activations,
 // abstracting over whether the JS scripts were running in the interpreter or
 // different modes of compiled code.
 //
 // FrameIter is parameterized by what it includes in the stack iteration:
-//  - The ContextOption determines whether the iteration will view frames from
-//    all JSContexts or just the given JSContext. (Hopefully this will go away.)
 //  - When provided, the optional JSPrincipal argument will cause FrameIter to
 //    only show frames in globals whose JSPrincipals are subsumed (via
 //    JSSecurityCallbacks::subsume) by the given JSPrincipal.
 //
 // Additionally, there are derived FrameIter types that automatically skip
 // certain frames:
 //  - ScriptFrameIter only shows frames that have an associated JSScript
 //    (currently everything other than asm.js stack frames). When !hasScript(),
 //    clients must stick to the portion of the
 //    interface marked below.
 //  - NonBuiltinScriptFrameIter additionally filters out builtin (self-hosted)
 //    scripts.
 class FrameIter
 {
   public:
-    enum ContextOption { CURRENT_CONTEXT, ALL_CONTEXTS };
     enum DebuggerEvalOption { FOLLOW_DEBUGGER_EVAL_PREV_LINK,
                               IGNORE_DEBUGGER_EVAL_PREV_LINK };
     enum State { DONE, INTERP, JIT, WASM };
 
     // Unlike ScriptFrameIter itself, ScriptFrameIter::Data can be allocated on
     // the heap, so this structure should not contain any GC things.
     struct Data
     {
         JSContext * cx_;
-        ContextOption       contextOption_;
         DebuggerEvalOption  debuggerEvalOption_;
         JSPrincipals *      principals_;
 
         State               state_;
 
         jsbytecode *        pc_;
 
         InterpreterFrameIterator interpFrames_;
         ActivationIterator activations_;
 
         jit::JitFrameIterator jitFrames_;
         unsigned ionInlineFrameNo_;
         wasm::FrameIterator wasmFrames_;
 
-        Data(JSContext* cx, ContextOption contextOption,
-             DebuggerEvalOption debuggerEvalOption, JSPrincipals* principals);
+        Data(JSContext* cx, DebuggerEvalOption debuggerEvalOption, JSPrincipals* principals);
         Data(const Data& other);
     };
 
-    explicit FrameIter(JSContext* cx);
-    FrameIter(JSContext* cx, ContextOption,
-              DebuggerEvalOption = FOLLOW_DEBUGGER_EVAL_PREV_LINK);
-    FrameIter(JSContext* cx, ContextOption, DebuggerEvalOption, JSPrincipals*);
+    explicit FrameIter(JSContext* cx,
+                       DebuggerEvalOption = FOLLOW_DEBUGGER_EVAL_PREV_LINK);
+    FrameIter(JSContext* cx, DebuggerEvalOption, JSPrincipals*);
     FrameIter(const FrameIter& iter);
     MOZ_IMPLICIT FrameIter(const Data& data);
     MOZ_IMPLICIT FrameIter(AbstractFramePtr frame);
 
     bool done() const { return data_.state_ == DONE; }
 
     // -------------------------------------------------------
     // The following functions can only be called when !done()
@@ -1888,35 +1882,27 @@ class FrameIter
 class ScriptFrameIter : public FrameIter
 {
     void settle() {
         while (!done() && !hasScript())
             FrameIter::operator++();
     }
 
   public:
-    explicit ScriptFrameIter(JSContext* cx)
-      : FrameIter(cx)
+    explicit ScriptFrameIter(JSContext* cx,
+                             DebuggerEvalOption debuggerEvalOption = FOLLOW_DEBUGGER_EVAL_PREV_LINK)
+      : FrameIter(cx, debuggerEvalOption)
     {
         settle();
     }
 
     ScriptFrameIter(JSContext* cx,
-                    ContextOption cxOption,
-                    DebuggerEvalOption debuggerEvalOption = FOLLOW_DEBUGGER_EVAL_PREV_LINK)
-      : FrameIter(cx, cxOption, debuggerEvalOption)
-    {
-        settle();
-    }
-
-    ScriptFrameIter(JSContext* cx,
-                    ContextOption cxOption,
                     DebuggerEvalOption debuggerEvalOption,
                     JSPrincipals* prin)
-      : FrameIter(cx, cxOption, debuggerEvalOption, prin)
+      : FrameIter(cx, debuggerEvalOption, prin)
     {
         settle();
     }
 
     ScriptFrameIter(const ScriptFrameIter& iter) : FrameIter(iter) { settle(); }
     explicit ScriptFrameIter(const FrameIter::Data& data) : FrameIter(data) { settle(); }
     explicit ScriptFrameIter(AbstractFramePtr frame) : FrameIter(frame) { settle(); }
 
@@ -1938,43 +1924,34 @@ SelfHostedFramesVisible()
 #endif
 
 /* A filtering of the FrameIter to only stop at non-self-hosted scripts. */
 class NonBuiltinFrameIter : public FrameIter
 {
     void settle();
 
   public:
-    explicit NonBuiltinFrameIter(JSContext* cx)
-      : FrameIter(cx)
+    explicit NonBuiltinFrameIter(JSContext* cx,
+                                 FrameIter::DebuggerEvalOption debuggerEvalOption =
+                                 FrameIter::FOLLOW_DEBUGGER_EVAL_PREV_LINK)
+      : FrameIter(cx, debuggerEvalOption)
     {
         settle();
     }
 
     NonBuiltinFrameIter(JSContext* cx,
-                        FrameIter::ContextOption contextOption,
-                        FrameIter::DebuggerEvalOption debuggerEvalOption =
-                        FrameIter::FOLLOW_DEBUGGER_EVAL_PREV_LINK)
-      : FrameIter(cx, contextOption, debuggerEvalOption)
-    {
-        settle();
-    }
-
-    NonBuiltinFrameIter(JSContext* cx,
-                        FrameIter::ContextOption contextOption,
                         FrameIter::DebuggerEvalOption debuggerEvalOption,
                         JSPrincipals* principals)
-      : FrameIter(cx, contextOption, debuggerEvalOption, principals)
+      : FrameIter(cx, debuggerEvalOption, principals)
     {
         settle();
     }
 
     NonBuiltinFrameIter(JSContext* cx, JSPrincipals* principals)
-        : FrameIter(cx, FrameIter::ALL_CONTEXTS, FrameIter::FOLLOW_DEBUGGER_EVAL_PREV_LINK,
-                    principals)
+      : FrameIter(cx, FrameIter::FOLLOW_DEBUGGER_EVAL_PREV_LINK, principals)
     {
         settle();
     }
 
     explicit NonBuiltinFrameIter(const FrameIter::Data& data)
       : FrameIter(data)
     {}
 
@@ -1986,36 +1963,28 @@ class NonBuiltinFrameIter : public Frame
 };
 
 /* A filtering of the ScriptFrameIter to only stop at non-self-hosted scripts. */
 class NonBuiltinScriptFrameIter : public ScriptFrameIter
 {
     void settle();
 
   public:
-    explicit NonBuiltinScriptFrameIter(JSContext* cx)
-      : ScriptFrameIter(cx)
+    explicit NonBuiltinScriptFrameIter(JSContext* cx,
+                                       ScriptFrameIter::DebuggerEvalOption debuggerEvalOption =
+                                       ScriptFrameIter::FOLLOW_DEBUGGER_EVAL_PREV_LINK)
+      : ScriptFrameIter(cx, debuggerEvalOption)
     {
         settle();
     }
 
     NonBuiltinScriptFrameIter(JSContext* cx,
-                              ScriptFrameIter::ContextOption contextOption,
-                              ScriptFrameIter::DebuggerEvalOption debuggerEvalOption =
-                              ScriptFrameIter::FOLLOW_DEBUGGER_EVAL_PREV_LINK)
-      : ScriptFrameIter(cx, contextOption, debuggerEvalOption)
-    {
-        settle();
-    }
-
-    NonBuiltinScriptFrameIter(JSContext* cx,
-                              ScriptFrameIter::ContextOption contextOption,
                               ScriptFrameIter::DebuggerEvalOption debuggerEvalOption,
                               JSPrincipals* principals)
-      : ScriptFrameIter(cx, contextOption, debuggerEvalOption, principals)
+      : ScriptFrameIter(cx, debuggerEvalOption, principals)
     {
         settle();
     }
 
     explicit NonBuiltinScriptFrameIter(const ScriptFrameIter::Data& data)
       : ScriptFrameIter(data)
     {}
 
@@ -2029,18 +1998,17 @@ class NonBuiltinScriptFrameIter : public
 /*
  * Blindly iterate over all frames in the current thread's stack. These frames
  * can be from different contexts and compartments, so beware.
  */
 class AllFramesIter : public ScriptFrameIter
 {
   public:
     explicit AllFramesIter(JSContext* cx)
-      : ScriptFrameIter(cx, ScriptFrameIter::ALL_CONTEXTS,
-                        ScriptFrameIter::IGNORE_DEBUGGER_EVAL_PREV_LINK)
+      : ScriptFrameIter(cx, ScriptFrameIter::IGNORE_DEBUGGER_EVAL_PREV_LINK)
     {}
 };
 
 /* Popular inline definitions. */
 
 inline JSScript*
 FrameIter::script() const
 {