Attachment 237328 Details for Bug 134420 – patch 5: with getPropertySlot() fix for With and Global scopes.

[patch] patch 5: with getPropertySlot() fix for With and Global scopes.

bug-134420.patch (text/plain), 36.40 KB, created by Mark Lam on 2014-08-28 16:09:10 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Mark Lam

Created: 2014-08-28 16:09:10 PDT

Size: 36.40 KB

patch

obsolete

>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 173091)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,141 @@
>+2014-08-28  Mark Lam  <mark.lam@apple.com>
>+
>+        DebuggerCallFrame::scope() should return a DebuggerScope.
>+        <https://webkit.org/b/134420>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Rolling back in r170680 with the fix for <https://webkit.org/b/135656>.
>+
>+        Previously, DebuggerCallFrame::scope() returns a JSActivation (and relevant
>+        peers) which the WebInspector will use to introspect CallFrame variables.
>+        Instead, we should be returning a DebuggerScope as an abstraction layer that
>+        provides the introspection functionality that the WebInspector needs.  This
>+        is the first step towards not forcing every frame to have a JSActivation
>+        object just because the debugger is enabled.
>+
>+        1. Instantiate the debuggerScopeStructure as a member of the JSGlobalObject
>+           instead of the VM.  This allows JSObject::globalObject() to be able to
>+           return the global object for the DebuggerScope.
>+
>+        2. On the DebuggerScope's life-cycle management:
>+
>+           The DebuggerCallFrame is designed to be "valid" only during a debugging session
>+           (while the debugger is broken) through the use of a DebuggerCallFrameScope in
>+           Debugger::pauseIfNeeded().  Once the debugger resumes from the break, the
>+           DebuggerCallFrameScope destructs, and the DebuggerCallFrame will be invalidated.
>+           We can't guarantee (from this code alone) that the Inspector code isn't still
>+           holding a ref to the DebuggerCallFrame (though they shouldn't), but by contract,
>+           the frame will be invalidated, and any attempt to query it will return null values.
>+           This is pre-existing behavior.
>+
>+           Now, we're adding the DebuggerScope into the picture.  While a single debugger
>+           pause session is in progress, the Inspector may request the scope from the
>+           DebuggerCallFrame.  While the DebuggerCallFrame is still valid, we want
>+           DebuggerCallFrame::scope() to always return the same DebuggerScope object.
>+           This is why we hold on to the DebuggerScope with a strong ref.
>+
>+           If we use a weak ref instead, the following cooky behavior can manifest:
>+           1. The Inspector calls Debugger::scope() to get the top scope.
>+           2. The Inspector iterates down the scope chain and is now only holding a
>+              reference to a parent scope.  It is no longer referencing the top scope.
>+           3. A GC occurs, and the DebuggerCallFrame's weak m_scope ref to the top scope
>+              gets cleared.
>+           4. The Inspector calls DebuggerCallFrame::scope() to get the top scope again but gets
>+              a different DebuggerScope instance.
>+           5. The Inspector iterates down the scope chain but never sees the parent scope
>+              instance that retained a ref to in step 2 above.  This is because when iterating
>+              this new DebuggerScope instance (which has no knowledge of the previous parent
>+              DebuggerScope instance), a new DebuggerScope instance will get created for the
>+              same parent scope. 
>+
>+           Since the DebuggerScope is a JSObject, its liveness is determined by its reachability.
>+           However, its "validity" is determined by the life-cycle of its owner DebuggerCallFrame.
>+           When the owner DebuggerCallFrame gets invalidated, its debugger scope chain (if
>+           instantiated) will also get invalidated.  This is why we need the
>+           DebuggerScope::invalidateChain() method.  The Inspector should not be using the
>+           DebuggerScope instance after its owner DebuggerCallFrame is invalidated.  If it does,
>+           those methods will do nothing or returned a failed status.
>+
>+        Fix for <https://webkit.org/b/135656>:
>+        3. DebuggerScope::getOwnPropertySlot() and DebuggerScope::put() need to set
>+           m_thisValue in the returned slot to the wrapped scope object.  Previously,
>+           it was pointing to the DebuggerScope though the rest of the fields in the
>+           returned slot will be set to data pertaining the wrapped scope object.
>+
>+        4. DebuggerScope::getOwnPropertySlot() will invoke getPropertySlot() on its
>+           wrapped scope.  This is because JSObject::getPropertySlot() cannot be
>+           overridden, and when called on a DebuggerScope, will not know to look in
>+           the ptototype chain of the DebuggerScope's wrapped scope.  Hence, we'll
>+           treat all properties in the wrapped scope as own properties in the
>+           DebuggerScope.  This is fine because the WebInspector does not presently
>+           care about where in the prototype chain the scope property comes from.
>+
>+           Note that the DebuggerScope and the JSActivation objects that it wraps do
>+           not have prototypes.  They are always jsNull().  This works perfectly with
>+           the above change to use getPropertySlot() instead of getOwnPropertySlot().
>+           To make this an explicit invariant, I also changed DebuggerScope::createStructure()
>+           and JSActivation::createStructure() to not take a prototype argument, and
>+           to always use jsNull() for their prototype value.
>+
>+        * debugger/Debugger.h:
>+        * debugger/DebuggerCallFrame.cpp:
>+        (JSC::DebuggerCallFrame::scope):
>+        (JSC::DebuggerCallFrame::evaluate):
>+        (JSC::DebuggerCallFrame::invalidate):
>+        * debugger/DebuggerCallFrame.h:
>+        * debugger/DebuggerScope.cpp:
>+        (JSC::DebuggerScope::DebuggerScope):
>+        (JSC::DebuggerScope::finishCreation):
>+        (JSC::DebuggerScope::visitChildren):
>+        (JSC::DebuggerScope::className):
>+        (JSC::DebuggerScope::getOwnPropertySlot):
>+        (JSC::DebuggerScope::put):
>+        (JSC::DebuggerScope::deleteProperty):
>+        (JSC::DebuggerScope::getOwnPropertyNames):
>+        (JSC::DebuggerScope::defineOwnProperty):
>+        (JSC::DebuggerScope::next):
>+        (JSC::DebuggerScope::invalidateChain):
>+        (JSC::DebuggerScope::isWithScope):
>+        (JSC::DebuggerScope::isGlobalScope):
>+        (JSC::DebuggerScope::isFunctionOrEvalScope):
>+        * debugger/DebuggerScope.h:
>+        (JSC::DebuggerScope::create):
>+        (JSC::DebuggerScope::createStructure):
>+        (JSC::DebuggerScope::iterator::iterator):
>+        (JSC::DebuggerScope::iterator::get):
>+        (JSC::DebuggerScope::iterator::operator++):
>+        (JSC::DebuggerScope::iterator::operator==):
>+        (JSC::DebuggerScope::iterator::operator!=):
>+        (JSC::DebuggerScope::isValid):
>+        (JSC::DebuggerScope::jsScope):
>+        (JSC::DebuggerScope::begin):
>+        (JSC::DebuggerScope::end):
>+        * inspector/JSJavaScriptCallFrame.cpp:
>+        (Inspector::JSJavaScriptCallFrame::scopeType):
>+        (Inspector::JSJavaScriptCallFrame::scopeChain):
>+        * inspector/JavaScriptCallFrame.h:
>+        (Inspector::JavaScriptCallFrame::scopeChain):
>+        * inspector/ScriptDebugServer.cpp:
>+        * runtime/JSActivation.h:
>+        (JSC::JSActivation::createStructure):
>+        * runtime/JSGlobalObject.cpp:
>+        (JSC::JSGlobalObject::reset):
>+        (JSC::JSGlobalObject::visitChildren):
>+        * runtime/JSGlobalObject.h:
>+        (JSC::JSGlobalObject::debuggerScopeStructure):
>+        * runtime/JSObject.cpp:
>+        * runtime/JSObject.h:
>+        (JSC::JSObject::isWithScope):
>+        * runtime/JSScope.h:
>+        * runtime/PropertySlot.h:
>+        (JSC::PropertySlot::setThisValue):
>+        * runtime/PutPropertySlot.h:
>+        (JSC::PutPropertySlot::setThisValue):
>+        * runtime/VM.cpp:
>+        (JSC::VM::VM):
>+        * runtime/VM.h:
>+
> 2014-08-28  Andreas Kling  <akling@apple.com>
> 
>         Use JSString::toIdentifier() in more places.
>Index: Source/JavaScriptCore/debugger/Debugger.h
>===================================================================
>--- Source/JavaScriptCore/debugger/Debugger.h	(revision 173091)
>+++ Source/JavaScriptCore/debugger/Debugger.h	(working copy)
>@@ -33,6 +33,7 @@
> 
> namespace JSC {
> 
>+class CodeBlock;
> class ExecState;
> class JSGlobalObject;
> class SourceProvider;
>Index: Source/JavaScriptCore/debugger/DebuggerCallFrame.cpp
>===================================================================
>--- Source/JavaScriptCore/debugger/DebuggerCallFrame.cpp	(revision 173091)
>+++ Source/JavaScriptCore/debugger/DebuggerCallFrame.cpp	(working copy)
>@@ -30,12 +30,14 @@
> #include "DebuggerCallFrame.h"
> 
> #include "CodeBlock.h"
>+#include "DebuggerScope.h"
> #include "Interpreter.h"
> #include "JSActivation.h"
> #include "JSFunction.h"
> #include "JSCInlines.h"
> #include "Parser.h"
> #include "StackVisitor.h"
>+#include "StrongInlines.h"
> 
> namespace JSC {
> 
>@@ -132,20 +134,25 @@ String DebuggerCallFrame::functionName()
>     return getCalculatedDisplayName(m_callFrame, function);
> }
> 
>-JSScope* DebuggerCallFrame::scope() const
>+DebuggerScope* DebuggerCallFrame::scope()
> {
>     ASSERT(isValid());
>     if (!isValid())
>         return 0;
> 
>-    CodeBlock* codeBlock = m_callFrame->codeBlock();
>-    if (codeBlock && codeBlock->needsActivation() && !m_callFrame->hasActivation()) {
>-        JSActivation* activation = JSActivation::create(*codeBlock->vm(), m_callFrame, codeBlock);
>-        m_callFrame->setActivation(activation);
>-        m_callFrame->setScope(activation);
>-    }
>+    if (!m_scope) {
>+        VM& vm = m_callFrame->vm();
>+        CodeBlock* codeBlock = m_callFrame->codeBlock();
>+        if (codeBlock && codeBlock->needsActivation() && !m_callFrame->hasActivation()) {
>+            ASSERT(!m_callFrame->scope()->isWithScope());
>+            JSActivation* activation = JSActivation::create(vm, m_callFrame, codeBlock);
>+            m_callFrame->setActivation(activation);
>+            m_callFrame->setScope(activation);
>+        }
> 
>-    return m_callFrame->scope();
>+        m_scope.set(vm, DebuggerScope::create(vm, m_callFrame->scope()));
>+    }
>+    return m_scope.get();
> }
> 
> DebuggerCallFrame::Type DebuggerCallFrame::type() const
>@@ -188,7 +195,7 @@ JSValue DebuggerCallFrame::evaluate(cons
>     }
> 
>     JSValue thisValue = thisValueForCallFrame(callFrame);
>-    JSValue result = vm.interpreter->execute(eval, callFrame, thisValue, scope());
>+    JSValue result = vm.interpreter->execute(eval, callFrame, thisValue, scope()->jsScope());
>     if (vm.exception()) {
>         exception = vm.exception();
>         vm.clearException();
>@@ -200,6 +207,10 @@ JSValue DebuggerCallFrame::evaluate(cons
> void DebuggerCallFrame::invalidate()
> {
>     m_callFrame = nullptr;
>+    if (m_scope) {
>+        m_scope->invalidateChain();
>+        m_scope.clear();
>+    }
>     RefPtr<DebuggerCallFrame> frame = m_caller.release();
>     while (frame) {
>         frame->m_callFrame = nullptr;
>Index: Source/JavaScriptCore/debugger/DebuggerCallFrame.h
>===================================================================
>--- Source/JavaScriptCore/debugger/DebuggerCallFrame.h	(revision 173091)
>+++ Source/JavaScriptCore/debugger/DebuggerCallFrame.h	(working copy)
>@@ -29,14 +29,18 @@
> #ifndef DebuggerCallFrame_h
> #define DebuggerCallFrame_h
> 
>-#include "CallFrame.h"
> #include "DebuggerPrimitives.h"
>+#include "Strong.h"
> #include <wtf/PassRefPtr.h>
> #include <wtf/RefCounted.h>
> #include <wtf/text/TextPosition.h>
> 
> namespace JSC {
> 
>+class DebuggerScope;
>+class ExecState;
>+typedef ExecState CallFrame;
>+
> class DebuggerCallFrame : public RefCounted<DebuggerCallFrame> {
> public:
>     enum Type { ProgramType, FunctionType };
>@@ -58,7 +62,7 @@ public:
>     JS_EXPORT_PRIVATE const TextPosition& position() const { return m_position; }
> 
>     JS_EXPORT_PRIVATE JSGlobalObject* vmEntryGlobalObject() const;
>-    JS_EXPORT_PRIVATE JSScope* scope() const;
>+    JS_EXPORT_PRIVATE DebuggerScope* scope();
>     JS_EXPORT_PRIVATE String functionName() const;
>     JS_EXPORT_PRIVATE Type type() const;
>     JS_EXPORT_PRIVATE JSValue thisValue() const;
>@@ -78,6 +82,9 @@ private:
>     CallFrame* m_callFrame;
>     RefPtr<DebuggerCallFrame> m_caller;
>     TextPosition m_position;
>+    // The DebuggerCallFrameScope is responsible for calling invalidate() which,
>+    // in turn, will clear this strong ref.
>+    Strong<DebuggerScope> m_scope;
> };
> 
> } // namespace JSC
>Index: Source/JavaScriptCore/debugger/DebuggerScope.cpp
>===================================================================
>--- Source/JavaScriptCore/debugger/DebuggerScope.cpp	(revision 173091)
>+++ Source/JavaScriptCore/debugger/DebuggerScope.cpp	(working copy)
>@@ -28,6 +28,7 @@
> 
> #include "JSActivation.h"
> #include "JSCInlines.h"
>+#include "JSWithScope.h"
> 
> namespace JSC {
> 
>@@ -35,17 +36,16 @@ STATIC_ASSERT_IS_TRIVIALLY_DESTRUCTIBLE(
> 
> const ClassInfo DebuggerScope::s_info = { "DebuggerScope", &Base::s_info, 0, CREATE_METHOD_TABLE(DebuggerScope) };
> 
>-DebuggerScope::DebuggerScope(VM& vm)
>-    : JSNonFinalObject(vm, vm.debuggerScopeStructure.get())
>+DebuggerScope::DebuggerScope(VM& vm, JSScope* scope)
>+    : JSNonFinalObject(vm, scope->globalObject()->debuggerScopeStructure())
> {
>+    ASSERT(scope);
>+    m_scope.set(vm, this, scope);
> }
> 
>-void DebuggerScope::finishCreation(VM& vm, JSObject* activation)
>+void DebuggerScope::finishCreation(VM& vm)
> {
>     Base::finishCreation(vm);
>-    ASSERT(activation);
>-    ASSERT(activation->isActivationObject());
>-    m_activation.set(vm, this, jsCast<JSActivation*>(activation));
> }
> 
> void DebuggerScope::visitChildren(JSCell* cell, SlotVisitor& visitor)
>@@ -53,43 +53,120 @@ void DebuggerScope::visitChildren(JSCell
>     DebuggerScope* thisObject = jsCast<DebuggerScope*>(cell);
>     ASSERT_GC_OBJECT_INHERITS(thisObject, info());
>     JSObject::visitChildren(thisObject, visitor);
>-    visitor.append(&thisObject->m_activation);
>+    visitor.append(&thisObject->m_scope);
>+    visitor.append(&thisObject->m_next);
> }
> 
> String DebuggerScope::className(const JSObject* object)
> {
>-    const DebuggerScope* thisObject = jsCast<const DebuggerScope*>(object);
>-    return thisObject->m_activation->methodTable()->className(thisObject->m_activation.get());
>+    const DebuggerScope* scope = jsCast<const DebuggerScope*>(object);
>+    ASSERT(scope->isValid());
>+    if (!scope->isValid())
>+        return String();
>+    JSObject* thisObject = JSScope::objectAtScope(scope->jsScope());
>+    return thisObject->methodTable()->className(thisObject);
> }
> 
> bool DebuggerScope::getOwnPropertySlot(JSObject* object, ExecState* exec, PropertyName propertyName, PropertySlot& slot)
> {
>-    DebuggerScope* thisObject = jsCast<DebuggerScope*>(object);
>-    return thisObject->m_activation->methodTable()->getOwnPropertySlot(thisObject->m_activation.get(), exec, propertyName, slot);
>+    DebuggerScope* scope = jsCast<DebuggerScope*>(object);
>+    ASSERT(scope->isValid());
>+    if (!scope->isValid())
>+        return false;
>+    JSObject* thisObject = JSScope::objectAtScope(scope->jsScope());
>+    slot.setThisValue(JSValue(thisObject));
>+
>+    // By default, JSObject::getPropertySlot() will look in the DebuggerScope's prototype
>+    // chain and not the wrapped scope, and JSObject::getPropertySlot() cannot be overridden
>+    // to behave differently for the DebuggerScope.
>+    //
>+    // Instead, we'll treat all properties in the wrapped scope and its prototype chain as
>+    // the own properties of the DebuggerScope. This is fine because the WebInspector
>+    // does not presently need to distinguish between what's owned at each level in the
>+    // prototype chain. Hence, we'll invoke getPropertySlot() on the wrapped scope here
>+    // instead of getOwnPropertySlot().
>+    return thisObject->getPropertySlot(exec, propertyName, slot);
> }
> 
> void DebuggerScope::put(JSCell* cell, ExecState* exec, PropertyName propertyName, JSValue value, PutPropertySlot& slot)
> {
>-    DebuggerScope* thisObject = jsCast<DebuggerScope*>(cell);
>-    thisObject->m_activation->methodTable()->put(thisObject->m_activation.get(), exec, propertyName, value, slot);
>+    DebuggerScope* scope = jsCast<DebuggerScope*>(cell);
>+    ASSERT(scope->isValid());
>+    if (!scope->isValid())
>+        return;
>+    JSObject* thisObject = JSScope::objectAtScope(scope->jsScope());
>+    slot.setThisValue(JSValue(thisObject));
>+    thisObject->methodTable()->put(thisObject, exec, propertyName, value, slot);
> }
> 
> bool DebuggerScope::deleteProperty(JSCell* cell, ExecState* exec, PropertyName propertyName)
> {
>-    DebuggerScope* thisObject = jsCast<DebuggerScope*>(cell);
>-    return thisObject->m_activation->methodTable()->deleteProperty(thisObject->m_activation.get(), exec, propertyName);
>+    DebuggerScope* scope = jsCast<DebuggerScope*>(cell);
>+    ASSERT(scope->isValid());
>+    if (!scope->isValid())
>+        return false;
>+    JSObject* thisObject = JSScope::objectAtScope(scope->jsScope());
>+    return thisObject->methodTable()->deleteProperty(thisObject, exec, propertyName);
> }
> 
> void DebuggerScope::getOwnPropertyNames(JSObject* object, ExecState* exec, PropertyNameArray& propertyNames, EnumerationMode mode)
> {
>-    DebuggerScope* thisObject = jsCast<DebuggerScope*>(object);
>-    thisObject->m_activation->methodTable()->getPropertyNames(thisObject->m_activation.get(), exec, propertyNames, mode);
>+    DebuggerScope* scope = jsCast<DebuggerScope*>(object);
>+    ASSERT(scope->isValid());
>+    if (!scope->isValid())
>+        return;
>+    JSObject* thisObject = JSScope::objectAtScope(scope->jsScope());
>+    thisObject->methodTable()->getPropertyNames(thisObject, exec, propertyNames, mode);
> }
> 
> bool DebuggerScope::defineOwnProperty(JSObject* object, ExecState* exec, PropertyName propertyName, const PropertyDescriptor& descriptor, bool shouldThrow)
> {
>-    DebuggerScope* thisObject = jsCast<DebuggerScope*>(object);
>-    return thisObject->m_activation->methodTable()->defineOwnProperty(thisObject->m_activation.get(), exec, propertyName, descriptor, shouldThrow);
>+    DebuggerScope* scope = jsCast<DebuggerScope*>(object);
>+    ASSERT(scope->isValid());
>+    if (!scope->isValid())
>+        return false;
>+    JSObject* thisObject = JSScope::objectAtScope(scope->jsScope());
>+    return thisObject->methodTable()->defineOwnProperty(thisObject, exec, propertyName, descriptor, shouldThrow);
>+}
>+
>+DebuggerScope* DebuggerScope::next()
>+{
>+    ASSERT(isValid());
>+    if (!m_next && m_scope->next()) {
>+        VM& vm = *m_scope->vm();
>+        DebuggerScope* nextScope = create(vm, m_scope->next());
>+        m_next.set(vm, this, nextScope);
>+    }
>+    return m_next.get();
>+}
>+
>+void DebuggerScope::invalidateChain()
>+{
>+    DebuggerScope* scope = this;
>+    while (scope) {
>+        ASSERT(scope->isValid());
>+        DebuggerScope* nextScope = scope->m_next.get();
>+        scope->m_next.clear();
>+        scope->m_scope.clear();
>+        scope = nextScope;
>+    }
>+}
>+
>+bool DebuggerScope::isWithScope() const
>+{
>+    return m_scope->isWithScope();
>+}
>+
>+bool DebuggerScope::isGlobalScope() const
>+{
>+    return m_scope->isGlobalObject();
>+}
>+
>+bool DebuggerScope::isFunctionOrEvalScope() const
>+{
>+    // In the current debugger implementation, every function or eval will create an
>+    // activation object. Hence, an activation object implies a function or eval scope.
>+    return m_scope->isActivationObject();
> }
> 
> } // namespace JSC
>Index: Source/JavaScriptCore/debugger/DebuggerScope.h
>===================================================================
>--- Source/JavaScriptCore/debugger/DebuggerScope.h	(revision 173091)
>+++ Source/JavaScriptCore/debugger/DebuggerScope.h	(working copy)
>@@ -30,15 +30,18 @@
> 
> namespace JSC {
> 
>+class DebuggerCallFrame;
>+class JSScope;
>+
> class DebuggerScope : public JSNonFinalObject {
> public:
>     typedef JSNonFinalObject Base;
> 
>-    static DebuggerScope* create(VM& vm, JSObject* object)
>+    static DebuggerScope* create(VM& vm, JSScope* scope)
>     {
>-        DebuggerScope* activation = new (NotNull, allocateCell<DebuggerScope>(vm.heap)) DebuggerScope(vm);
>-        activation->finishCreation(vm, object);
>-        return activation;
>+        DebuggerScope* debuggerScope = new (NotNull, allocateCell<DebuggerScope>(vm.heap)) DebuggerScope(vm, scope);
>+        debuggerScope->finishCreation(vm);
>+        return debuggerScope;
>     }
> 
>     static void visitChildren(JSCell*, SlotVisitor&);
>@@ -51,21 +54,64 @@ public:
> 
>     DECLARE_EXPORT_INFO;
> 
>-    static Structure* createStructure(VM& vm, JSGlobalObject* globalObject, JSValue prototype) 
>+    static Structure* createStructure(VM& vm, JSGlobalObject* globalObject) 
>     {
>-        return Structure::create(vm, globalObject, prototype, TypeInfo(ObjectType, StructureFlags), info()); 
>+        return Structure::create(vm, globalObject, jsNull(), TypeInfo(ObjectType, StructureFlags), info()); 
>     }
> 
>-protected:
>-    static const unsigned StructureFlags = OverridesGetOwnPropertySlot | JSObject::StructureFlags;
>-
>-    JS_EXPORT_PRIVATE void finishCreation(VM&, JSObject* activation);
>+    class iterator {
>+    public:
>+        iterator(DebuggerScope* node)
>+            : m_node(node)
>+        {
>+        }
>+
>+        DebuggerScope* get() { return m_node; }
>+        iterator& operator++() { m_node = m_node->next(); return *this; }
>+        // postfix ++ intentionally omitted
>+
>+        bool operator==(const iterator& other) const { return m_node == other.m_node; }
>+        bool operator!=(const iterator& other) const { return m_node != other.m_node; }
>+
>+    private:
>+        DebuggerScope* m_node;
>+    };
>+
>+    iterator begin();
>+    iterator end();
>+    DebuggerScope* next();
>+
>+    void invalidateChain();
>+    bool isValid() const { return !!m_scope; }
>+
>+    bool isWithScope() const;
>+    bool isGlobalScope() const;
>+    bool isFunctionOrEvalScope() const;
> 
> private:
>-    JS_EXPORT_PRIVATE DebuggerScope(VM&);
>-    WriteBarrier<JSActivation> m_activation;
>+    JS_EXPORT_PRIVATE DebuggerScope(VM&, JSScope*);
>+    JS_EXPORT_PRIVATE void finishCreation(VM&);
>+
>+    JSScope* jsScope() const { return m_scope.get(); }
>+
>+    static const unsigned StructureFlags = OverridesGetOwnPropertySlot | OverridesGetPropertyNames | JSObject::StructureFlags;
>+
>+    WriteBarrier<JSScope> m_scope;
>+    WriteBarrier<DebuggerScope> m_next;
>+
>+    friend class DebuggerCallFrame;
> };
> 
>+inline DebuggerScope::iterator DebuggerScope::begin()
>+{
>+    return iterator(this); 
>+}
>+
>+inline DebuggerScope::iterator DebuggerScope::end()
>+{ 
>+    return iterator(0); 
>+}
>+
> } // namespace JSC
> 
> #endif // DebuggerScope_h
>Index: Source/JavaScriptCore/inspector/JSJavaScriptCallFrame.cpp
>===================================================================
>--- Source/JavaScriptCore/inspector/JSJavaScriptCallFrame.cpp	(revision 173091)
>+++ Source/JavaScriptCore/inspector/JSJavaScriptCallFrame.cpp	(working copy)
>@@ -28,6 +28,7 @@
> 
> #if ENABLE(INSPECTOR)
> 
>+#include "DebuggerScope.h"
> #include "Error.h"
> #include "JSCJSValue.h"
> #include "JSCellInlines.h"
>@@ -95,29 +96,30 @@ JSValue JSJavaScriptCallFrame::scopeType
>         return jsUndefined();
>     int index = exec->argument(0).asInt32();
> 
>-    JSScope* scopeChain = impl().scopeChain();
>-    ScopeChainIterator end = scopeChain->end();
>-
>-    // FIXME: We should be identifying and returning CATCH_SCOPE appropriately.
>+    DebuggerScope* scopeChain = impl().scopeChain();
>+    DebuggerScope::iterator end = scopeChain->end();
> 
>     bool foundLocalScope = false;
>-    for (ScopeChainIterator iter = scopeChain->begin(); iter != end; ++iter) {
>-        JSObject* scope = iter.get();
>-        if (scope->isActivationObject()) {
>-            if (!foundLocalScope) {
>-                // First activation object is local scope, each successive activation object is closure.
>-                if (!index)
>-                    return jsNumber(JSJavaScriptCallFrame::LOCAL_SCOPE);
>-                foundLocalScope = true;
>-            } else if (!index)
>-                return jsNumber(JSJavaScriptCallFrame::CLOSURE_SCOPE);
>+    for (DebuggerScope::iterator iter = scopeChain->begin(); iter != end; ++iter) {
>+        DebuggerScope* scope = iter.get();
>+
>+        if (!foundLocalScope && scope->isFunctionOrEvalScope()) {
>+            // First function scope is the local scope, each successive one is a closure.
>+            if (!index)
>+                return jsNumber(JSJavaScriptCallFrame::LOCAL_SCOPE);
>+            foundLocalScope = true;
>         }
> 
>         if (!index) {
>-            // Last in the chain is global scope.
>-            if (++iter == end)
>+            if (scope->isWithScope())
>+                return jsNumber(JSJavaScriptCallFrame::WITH_SCOPE);
>+            if (scope->isGlobalScope()) {
>+                ASSERT(++iter == end);
>                 return jsNumber(JSJavaScriptCallFrame::GLOBAL_SCOPE);
>-            return jsNumber(JSJavaScriptCallFrame::WITH_SCOPE);
>+            }
>+            // FIXME: We should be identifying and returning CATCH_SCOPE appropriately.
>+            ASSERT(scope->isFunctionOrEvalScope());
>+            return jsNumber(JSJavaScriptCallFrame::CLOSURE_SCOPE);
>         }
> 
>         --index;
>@@ -157,9 +159,9 @@ JSValue JSJavaScriptCallFrame::scopeChai
>     if (!impl().scopeChain())
>         return jsNull();
> 
>-    JSScope* scopeChain = impl().scopeChain();
>-    ScopeChainIterator iter = scopeChain->begin();
>-    ScopeChainIterator end = scopeChain->end();
>+    DebuggerScope* scopeChain = impl().scopeChain();
>+    DebuggerScope::iterator iter = scopeChain->begin();
>+    DebuggerScope::iterator end = scopeChain->end();
> 
>     // We must always have something in the scope chain.
>     ASSERT(iter != end);
>Index: Source/JavaScriptCore/inspector/JavaScriptCallFrame.h
>===================================================================
>--- Source/JavaScriptCore/inspector/JavaScriptCallFrame.h	(revision 173091)
>+++ Source/JavaScriptCore/inspector/JavaScriptCallFrame.h	(working copy)
>@@ -1,5 +1,5 @@
> /*
>- * Copyright (C) 2008, 2013 Apple Inc. All Rights Reserved.
>+ * Copyright (C) 2008, 2013-2014 Apple Inc. All Rights Reserved.
>  *
>  * Redistribution and use in source and binary forms, with or without
>  * modification, are permitted provided that the following conditions
>@@ -53,7 +53,7 @@ public:
> 
>     String functionName() const { return m_debuggerCallFrame->functionName(); }
>     JSC::DebuggerCallFrame::Type type() const { return m_debuggerCallFrame->type(); }
>-    JSC::JSScope* scopeChain() const { return m_debuggerCallFrame->scope(); }
>+    JSC::DebuggerScope* scopeChain() const { return m_debuggerCallFrame->scope(); }
>     JSC::JSGlobalObject* vmEntryGlobalObject() const { return m_debuggerCallFrame->vmEntryGlobalObject(); }
> 
>     JSC::JSValue thisValue() const { return m_debuggerCallFrame->thisValue(); }
>Index: Source/JavaScriptCore/inspector/ScriptDebugServer.cpp
>===================================================================
>--- Source/JavaScriptCore/inspector/ScriptDebugServer.cpp	(revision 173091)
>+++ Source/JavaScriptCore/inspector/ScriptDebugServer.cpp	(working copy)
>@@ -34,6 +34,7 @@
> #if ENABLE(INSPECTOR)
> 
> #include "DebuggerCallFrame.h"
>+#include "DebuggerScope.h"
> #include "JSJavaScriptCallFrame.h"
> #include "JSLock.h"
> #include "JavaScriptCallFrame.h"
>Index: Source/JavaScriptCore/runtime/JSActivation.h
>===================================================================
>--- Source/JavaScriptCore/runtime/JSActivation.h	(revision 173091)
>+++ Source/JavaScriptCore/runtime/JSActivation.h	(working copy)
>@@ -81,7 +81,7 @@ public:
>         
>     DECLARE_INFO;
> 
>-    static Structure* createStructure(VM& vm, JSGlobalObject* globalObject, JSValue proto) { return Structure::create(vm, globalObject, proto, TypeInfo(ActivationObjectType, StructureFlags), info()); }
>+    static Structure* createStructure(VM& vm, JSGlobalObject* globalObject) { return Structure::create(vm, globalObject, jsNull(), TypeInfo(ActivationObjectType, StructureFlags), info()); }
> 
>     WriteBarrierBase<Unknown>& registerAt(int) const;
>     bool isValidIndex(int) const;
>Index: Source/JavaScriptCore/runtime/JSGlobalObject.cpp
>===================================================================
>--- Source/JavaScriptCore/runtime/JSGlobalObject.cpp	(revision 173091)
>+++ Source/JavaScriptCore/runtime/JSGlobalObject.cpp	(working copy)
>@@ -45,6 +45,7 @@
> #include "DateConstructor.h"
> #include "DatePrototype.h"
> #include "Debugger.h"
>+#include "DebuggerScope.h"
> #include "Error.h"
> #include "ErrorConstructor.h"
> #include "ErrorPrototype.h"
>@@ -318,8 +319,9 @@ void JSGlobalObject::reset(JSValue proto
>     m_typedArrays[toIndex(TypeDataView)].structure.set(vm, this, JSDataView::createStructure(vm, this, m_typedArrays[toIndex(TypeDataView)].prototype.get()));
> 
>     m_nameScopeStructure.set(vm, this, JSNameScope::createStructure(vm, this, jsNull()));
>-    m_activationStructure.set(vm, this, JSActivation::createStructure(vm, this, jsNull()));
>+    m_activationStructure.set(vm, this, JSActivation::createStructure(vm, this));
>     m_strictEvalActivationStructure.set(vm, this, StrictEvalActivation::createStructure(vm, this, jsNull()));
>+    m_debuggerScopeStructure.set(m_vm, this, DebuggerScope::createStructure(m_vm, this));
>     m_withScopeStructure.set(vm, this, JSWithScope::createStructure(vm, this, jsNull()));
> 
>     m_nullPrototypeObjectStructure.set(vm, this, JSFinalObject::createStructure(vm, this, jsNull(), JSFinalObject::defaultInlineCapacity()));
>@@ -662,6 +664,7 @@ void JSGlobalObject::visitChildren(JSCel
>     visitor.append(&thisObject->m_promisePrototype);
> #endif
> 
>+    visitor.append(&thisObject->m_debuggerScopeStructure);
>     visitor.append(&thisObject->m_withScopeStructure);
>     visitor.append(&thisObject->m_strictEvalActivationStructure);
>     visitor.append(&thisObject->m_activationStructure);
>Index: Source/JavaScriptCore/runtime/JSGlobalObject.h
>===================================================================
>--- Source/JavaScriptCore/runtime/JSGlobalObject.h	(revision 173091)
>+++ Source/JavaScriptCore/runtime/JSGlobalObject.h	(working copy)
>@@ -186,6 +186,7 @@ protected:
>     WriteBarrier<JSPromisePrototype> m_promisePrototype;
> #endif
> 
>+    WriteBarrier<Structure> m_debuggerScopeStructure;
>     WriteBarrier<Structure> m_withScopeStructure;
>     WriteBarrier<Structure> m_strictEvalActivationStructure;
>     WriteBarrier<Structure> m_activationStructure;
>@@ -391,6 +392,7 @@ public:
>     JSPromisePrototype* promisePrototype() const { return m_promisePrototype.get(); }
> #endif
> 
>+    Structure* debuggerScopeStructure() const { return m_debuggerScopeStructure.get(); }
>     Structure* withScopeStructure() const { return m_withScopeStructure.get(); }
>     Structure* strictEvalActivationStructure() const { return m_strictEvalActivationStructure.get(); }
>     Structure* activationStructure() const { return m_activationStructure.get(); }
>Index: Source/JavaScriptCore/runtime/JSObject.cpp
>===================================================================
>--- Source/JavaScriptCore/runtime/JSObject.cpp	(revision 173091)
>+++ Source/JavaScriptCore/runtime/JSObject.cpp	(working copy)
>@@ -2782,5 +2782,4 @@ void JSObject::getGenericPropertyNames(J
>     }
> }
> 
>-
> } // namespace JSC
>Index: Source/JavaScriptCore/runtime/JSObject.h
>===================================================================
>--- Source/JavaScriptCore/runtime/JSObject.h	(revision 173091)
>+++ Source/JavaScriptCore/runtime/JSObject.h	(working copy)
>@@ -594,6 +594,7 @@ public:
>     bool isNameScopeObject() const;
>     bool isActivationObject() const;
>     bool isErrorInstance() const;
>+    bool isWithScope() const;
> 
>     JS_EXPORT_PRIVATE void seal(VM&);
>     JS_EXPORT_PRIVATE void freeze(VM&);
>@@ -1150,6 +1151,11 @@ inline bool JSObject::isErrorInstance() 
>     return type() == ErrorInstanceType;
> }
> 
>+inline bool JSObject::isWithScope() const
>+{
>+    return type() == WithScopeType;
>+}
>+
> inline void JSObject::setStructureAndButterfly(VM& vm, Structure* structure, Butterfly* butterfly)
> {
>     ASSERT(structure);
>Index: Source/JavaScriptCore/runtime/JSScope.h
>===================================================================
>--- Source/JavaScriptCore/runtime/JSScope.h	(revision 173091)
>+++ Source/JavaScriptCore/runtime/JSScope.h	(working copy)
>@@ -150,7 +150,7 @@ public:
>     friend class LLIntOffsetsExtractor;
>     static size_t offsetOfNext();
> 
>-    JS_EXPORT_PRIVATE static JSObject* objectAtScope(JSScope*);
>+    static JSObject* objectAtScope(JSScope*);
> 
>     static JSValue resolve(ExecState*, JSScope*, const Identifier&);
>     static ResolveOp abstractResolve(ExecState*, bool hasTopActivation, JSScope*, const Identifier&, GetOrPut, ResolveType);
>Index: Source/JavaScriptCore/runtime/PropertySlot.h
>===================================================================
>--- Source/JavaScriptCore/runtime/PropertySlot.h	(revision 173091)
>+++ Source/JavaScriptCore/runtime/PropertySlot.h	(working copy)
>@@ -204,6 +204,11 @@ public:
>         m_offset = offset;
>     }
> 
>+    void setThisValue(JSValue thisValue)
>+    {
>+        m_thisValue = thisValue;
>+    }
>+
>     void setUndefined()
>     {
>         m_data.value = JSValue::encode(jsUndefined());
>@@ -235,7 +240,7 @@ private:
> 
>     PropertyType m_propertyType;
>     PropertyOffset m_offset;
>-    const JSValue m_thisValue;
>+    JSValue m_thisValue;
>     JSObject* m_slotBase;
>     WatchpointSet* m_watchpointSet;
>     CacheabilityType m_cacheability;
>Index: Source/JavaScriptCore/runtime/PutPropertySlot.h
>===================================================================
>--- Source/JavaScriptCore/runtime/PutPropertySlot.h	(revision 173091)
>+++ Source/JavaScriptCore/runtime/PutPropertySlot.h	(working copy)
>@@ -80,6 +80,11 @@ namespace JSC {
>             m_offset = offset;
>         }
> 
>+        void setThisValue(JSValue thisValue)
>+        {
>+            m_thisValue = thisValue;
>+        }
>+
>         PutValueFunc customSetter() const { return m_putFunction; }
> 
>         Context context() const { return static_cast<Context>(m_context); }
>Index: Source/JavaScriptCore/runtime/VM.cpp
>===================================================================
>--- Source/JavaScriptCore/runtime/VM.cpp	(revision 173091)
>+++ Source/JavaScriptCore/runtime/VM.cpp	(working copy)
>@@ -40,7 +40,6 @@
> #include "CustomGetterSetter.h"
> #include "DFGLongLivedState.h"
> #include "DFGWorklist.h"
>-#include "DebuggerScope.h"
> #include "ErrorInstance.h"
> #include "FTLThunks.h"
> #include "FunctionConstructor.h"
>@@ -209,7 +208,6 @@ VM::VM(VMType vmType, HeapType heapType)
>     propertyNames = new CommonIdentifiers(this);
>     structureStructure.set(*this, Structure::createStructure(*this));
>     structureRareDataStructure.set(*this, StructureRareData::createStructure(*this, 0, jsNull()));
>-    debuggerScopeStructure.set(*this, DebuggerScope::createStructure(*this, 0, jsNull()));
>     terminatedExecutionErrorStructure.set(*this, TerminatedExecutionError::createStructure(*this, 0, jsNull()));
>     stringStructure.set(*this, JSString::createStructure(*this, 0, jsNull()));
>     notAnObjectStructure.set(*this, JSNotAnObject::createStructure(*this, 0, jsNull()));
>Index: Source/JavaScriptCore/runtime/VM.h
>===================================================================
>--- Source/JavaScriptCore/runtime/VM.h	(revision 173091)
>+++ Source/JavaScriptCore/runtime/VM.h	(working copy)
>@@ -246,7 +246,6 @@ namespace JSC {
> 
>         Strong<Structure> structureStructure;
>         Strong<Structure> structureRareDataStructure;
>-        Strong<Structure> debuggerScopeStructure;
>         Strong<Structure> terminatedExecutionErrorStructure;
>         Strong<Structure> stringStructure;
>         Strong<Structure> notAnObjectStructure;
>Index: Source/WebCore/ChangeLog
>===================================================================
>--- Source/WebCore/ChangeLog	(revision 173091)
>+++ Source/WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,18 @@
>+2014-08-28  Mark Lam  <mark.lam@apple.com>
>+
>+        DebuggerCallFrame::scope() should return a DebuggerScope.
>+        <https://webkit.org/b/134420>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        No new tests.
>+
>+        Rolling back in r170680 with the fix for <https://webkit.org/b/135656>.
>+
>+        * bindings/js/ScriptController.cpp:
>+        (WebCore::ScriptController::attachDebugger):
>+        - We should acquire the JSLock before modifying a JS global object.
>+
> 2014-08-28  Daniel Bates  <dabates@apple.com>
> 
>         [iOS] Clients that include WebCoreThread.h fail to build after <http://trac.webkit.org/changeset/172814/>
>Index: Source/WebCore/bindings/js/ScriptController.cpp
>===================================================================
>--- Source/WebCore/bindings/js/ScriptController.cpp	(revision 173091)
>+++ Source/WebCore/bindings/js/ScriptController.cpp	(working copy)
>@@ -307,6 +307,7 @@ void ScriptController::attachDebugger(JS
>         return;
> 
>     JSDOMWindow* globalObject = shell->window();
>+    JSLockHolder lock(globalObject->vm());
>     if (debugger)
>         debugger->attach(globalObject);
>     else if (JSC::Debugger* currentDebugger = globalObject->debugger())

Flags:

ggaren: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 134420: 234038 | 234198 | 234219 | 237195 | 237196 | 237328