From 77f4e69c967644d33d6dc6588817bb620be05764 Mon Sep 17 00:00:00 2001 From: Jason Orendorff Date: Tue, 13 Mar 2018 12:40:36 -0500 Subject: [PATCH] Bug 1443583 - Part 2.5: Change DebuggerFrame objects so that the private data is a FrameIter::Data*, not a raw form of an AbstractFramePtr. r=jimb. --HG-- extra : rebase_source : e97f899d3b4c3c68bc3deea852a0c63cfc16ed5e --- js/src/vm/Debugger.cpp | 78 ++++++++++++++---------------------------- js/src/vm/Debugger.h | 3 ++ 2 files changed, 28 insertions(+), 53 deletions(-) diff --git a/js/src/vm/Debugger.cpp b/js/src/vm/Debugger.cpp index 5680eae8c3ad..93d3a37c784f 100644 --- a/js/src/vm/Debugger.cpp +++ b/js/src/vm/Debugger.cpp @@ -7525,10 +7525,10 @@ DebuggerFrame::create(JSContext* cx, HandleObject proto, const FrameIter& iter, DebuggerFrame& frame = obj->as(); - AbstractFramePtr data = iter.copyDataAsAbstractFramePtr(); + FrameIter::Data* data = iter.copyData(); if (!data) return nullptr; - frame.setPrivate(data.raw()); + frame.setPrivate(data); frame.setReservedSlot(JSSLOT_DEBUGFRAME_OWNER, ObjectValue(*debugger)); @@ -8051,34 +8051,24 @@ DebuggerFrame_requireLive(JSContext* cx, HandleDebuggerFrame frame) return true; } +FrameIter::Data* +DebuggerFrame::frameIterData() const +{ + return static_cast(getPrivate()); +} + /* static */ AbstractFramePtr DebuggerFrame::getReferent(HandleDebuggerFrame frame) { - AbstractFramePtr referent = AbstractFramePtr::FromRaw(frame->getPrivate()); - if (referent.isScriptFrameIterData()) { - FrameIter iter(*(FrameIter::Data*)(referent.raw())); - referent = iter.abstractFramePtr(); - } - return referent; + FrameIter iter(*frame->frameIterData()); + return iter.abstractFramePtr(); } /* static */ bool DebuggerFrame::getFrameIter(JSContext* cx, HandleDebuggerFrame frame, Maybe& result) { - AbstractFramePtr referent = AbstractFramePtr::FromRaw(frame->getPrivate()); - if (referent.isScriptFrameIterData()) { - result.emplace(*reinterpret_cast(referent.raw())); - } else { - result.emplace(cx, FrameIter::IGNORE_DEBUGGER_EVAL_PREV_LINK); - FrameIter& iter = *result; - while (!iter.hasUsableAbstractFramePtr() || iter.abstractFramePtr() != referent) - ++iter; - AbstractFramePtr data = iter.copyDataAsAbstractFramePtr(); - if (!data) - return false; - frame->setPrivate(data.raw()); - } + result.emplace(*frame->frameIterData()); return true; } @@ -8099,10 +8089,11 @@ DebuggerFrame::requireScriptReferent(JSContext* cx, HandleDebuggerFrame frame) static void DebuggerFrame_freeScriptFrameIterData(FreeOp* fop, JSObject* obj) { - AbstractFramePtr frame = AbstractFramePtr::FromRaw(obj->as().getPrivate()); - if (frame.isScriptFrameIterData()) - fop->delete_((FrameIter::Data*) frame.raw()); - obj->as().setPrivate(nullptr); + DebuggerFrame& frame = obj->as(); + if (FrameIter::Data* data = frame.frameIterData()) { + fop->delete_(data); + frame.setPrivate(nullptr); + } } static void @@ -8181,40 +8172,21 @@ DebuggerFrame_checkThis(JSContext* cx, const CallArgs& args, const char* fnname, } /* - * To make frequently fired hooks like onEnterFrame more performant, - * Debugger.Frame methods should not create a FrameIter unless it - * absolutely needs to. That is, unless the method has to call a method on - * FrameIter that's otherwise not available on AbstractFramePtr. + * Methods can use THIS_DEBUGGER_FRAME to check that `this` is a Debugger.Frame object + * and get it in a local Rooted. * - * When a Debugger.Frame is first created, its private slot is set to the - * AbstractFramePtr itself. The first time the users asks for a - * FrameIter, we construct one, have it settle on the frame pointed to - * by the AbstractFramePtr and cache its internal Data in the Debugger.Frame - * object's private slot. Subsequent uses of the Debugger.Frame object will - * always create a FrameIter from the cached Data. - * - * Methods that only need the AbstractFramePtr should use THIS_FRAME. + * Methods that need the AbstractFramePtr should use THIS_FRAME. */ - #define THIS_DEBUGGER_FRAME(cx, argc, vp, fnname, args, frame) \ CallArgs args = CallArgsFromVp(argc, vp); \ RootedDebuggerFrame frame(cx, DebuggerFrame_checkThis(cx, args, fnname, true)); \ if (!frame) \ return false; -#define THIS_FRAME_THISOBJ(cx, argc, vp, fnname, args, thisobj) \ - CallArgs args = CallArgsFromVp(argc, vp); \ - RootedNativeObject thisobj(cx, DebuggerFrame_checkThis(cx, args, fnname, true)); \ - if (!thisobj) \ - return false - -#define THIS_FRAME(cx, argc, vp, fnname, args, thisobj, frame) \ - THIS_FRAME_THISOBJ(cx, argc, vp, fnname, args, thisobj); \ - AbstractFramePtr frame = AbstractFramePtr::FromRaw(thisobj->getPrivate()); \ - if (frame.isScriptFrameIterData()) { \ - FrameIter iter(*(FrameIter::Data*)(frame.raw())); \ - frame = iter.abstractFramePtr(); \ - } +#define THIS_FRAME(cx, argc, vp, fnname, args, thisobj, iter, frame) \ + THIS_DEBUGGER_FRAME(cx, argc, vp, fnname, args, thisobj); \ + FrameIter iter(*thisobj->frameIterData()); \ + AbstractFramePtr frame = iter.abstractFramePtr() /* static */ bool DebuggerFrame::typeGetter(JSContext* cx, unsigned argc, Value* vp) @@ -8372,7 +8344,7 @@ DebuggerArguments_getArg(JSContext* cx, unsigned argc, Value* vp) * to check that it is still live and get the fp. */ args.setThis(argsobj->as().getReservedSlot(JSSLOT_DEBUGARGUMENTS_FRAME)); - THIS_FRAME(cx, argc, vp, "get argument", ca2, thisobj, frame); + THIS_FRAME(cx, argc, vp, "get argument", ca2, thisobj, frameIter, frame); // TODO handle wasm frame arguments -- they are not yet reflectable. MOZ_ASSERT(!frame.isWasmDebugFrame(), "a wasm frame args"); @@ -8475,7 +8447,7 @@ DebuggerFrame::argumentsGetter(JSContext* cx, unsigned argc, Value* vp) static bool DebuggerFrame_getScript(JSContext* cx, unsigned argc, Value* vp) { - THIS_FRAME(cx, argc, vp, "get script", args, thisobj, frame); + THIS_FRAME(cx, argc, vp, "get script", args, thisobj, frameIter, frame); Debugger* debug = Debugger::fromChildJSObject(thisobj); RootedObject scriptObject(cx); diff --git a/js/src/vm/Debugger.h b/js/src/vm/Debugger.h index a442c68c61f2..aae838776d94 100644 --- a/js/src/vm/Debugger.h +++ b/js/src/vm/Debugger.h @@ -26,6 +26,7 @@ #include "vm/JSCompartment.h" #include "vm/JSContext.h" #include "vm/SavedStacks.h" +#include "vm/Stack.h" #include "wasm/WasmJS.h" namespace js { @@ -1394,6 +1395,8 @@ class DebuggerFrame : public NativeObject static MOZ_MUST_USE bool evalWithBindingsMethod(JSContext* cx, unsigned argc, Value* vp); Debugger* owner() const; + public: + FrameIter::Data* frameIterData() const; }; class DebuggerObject : public NativeObject