DebuggerCallFrame::scope() should return a DebuggerScope.
authormark.lam@apple.com <mark.lam@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 29 Aug 2014 00:48:59 +0000 (00:48 +0000)
committermark.lam@apple.com <mark.lam@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 29 Aug 2014 00:48:59 +0000 (00:48 +0000)
<https://webkit.org/b/134420>

Reviewed by Geoffrey Garen.

Source/JavaScriptCore:

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:

Source/WebCore:

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.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@173100 268f45cc-cd09-0410-ab3c-d52691b4dbfc

21 files changed:
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/debugger/Debugger.h
Source/JavaScriptCore/debugger/DebuggerCallFrame.cpp
Source/JavaScriptCore/debugger/DebuggerCallFrame.h
Source/JavaScriptCore/debugger/DebuggerScope.cpp
Source/JavaScriptCore/debugger/DebuggerScope.h
Source/JavaScriptCore/inspector/JSJavaScriptCallFrame.cpp
Source/JavaScriptCore/inspector/JavaScriptCallFrame.h
Source/JavaScriptCore/inspector/ScriptDebugServer.cpp
Source/JavaScriptCore/runtime/JSActivation.h
Source/JavaScriptCore/runtime/JSGlobalObject.cpp
Source/JavaScriptCore/runtime/JSGlobalObject.h
Source/JavaScriptCore/runtime/JSObject.cpp
Source/JavaScriptCore/runtime/JSObject.h
Source/JavaScriptCore/runtime/JSScope.h
Source/JavaScriptCore/runtime/PropertySlot.h
Source/JavaScriptCore/runtime/PutPropertySlot.h
Source/JavaScriptCore/runtime/VM.cpp
Source/JavaScriptCore/runtime/VM.h
Source/WebCore/ChangeLog
Source/WebCore/bindings/js/ScriptController.cpp

index e92c15e..7117644 100644 (file)
@@ -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 Geoffrey Garen.
+
+        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 2e8524a..ae8ad46 100644 (file)
@@ -33,6 +33,7 @@
 
 namespace JSC {
 
+class CodeBlock;
 class ExecState;
 class JSGlobalObject;
 class SourceProvider;
index 58ba4e1..7412a73 100644 (file)
 #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() const
     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(const String& script, JSValue& exception)
     }
 
     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(const String& script, JSValue& exception)
 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 ca22569..5759728 100644 (file)
 #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 7ee53a3..0142452 100644 (file)
@@ -28,6 +28,7 @@
 
 #include "JSActivation.h"
 #include "JSCInlines.h"
+#include "JSWithScope.h"
 
 namespace JSC {
 
@@ -35,17 +36,16 @@ STATIC_ASSERT_IS_TRIVIALLY_DESTRUCTIBLE(DebuggerScope);
 
 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* cell, SlotVisitor& visitor)
     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 2ba8340..4c32116 100644 (file)
 
 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;
+    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; }
 
-    JS_EXPORT_PRIVATE void finishCreation(VM&, JSObject* activation);
+    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 49e1b72..036920d 100644 (file)
@@ -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(ExecState* exec)
         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::scopeChain(ExecState* exec) const
     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 b584067..6bd643b 100644 (file)
@@ -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 d279893..d118231 100644 (file)
@@ -34,6 +34,7 @@
 #if ENABLE(INSPECTOR)
 
 #include "DebuggerCallFrame.h"
+#include "DebuggerScope.h"
 #include "JSJavaScriptCallFrame.h"
 #include "JSLock.h"
 #include "JavaScriptCallFrame.h"
index 409961f..4e76758 100644 (file)
@@ -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 5e7047d..f89187e 100644 (file)
@@ -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 prototype)
     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(JSCell* cell, SlotVisitor& visitor)
     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 837ebe3..864cb1e 100644 (file)
@@ -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 2412ab6..d4d820f 100644 (file)
@@ -2782,5 +2782,4 @@ void JSObject::getGenericPropertyNames(JSObject* object, ExecState* exec, Proper
     }
 }
 
-
 } // namespace JSC
index 0b768cc..6b7496d 100644 (file)
@@ -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() const
     return type() == ErrorInstanceType;
 }
 
+inline bool JSObject::isWithScope() const
+{
+    return type() == WithScopeType;
+}
+
 inline void JSObject::setStructureAndButterfly(VM& vm, Structure* structure, Butterfly* butterfly)
 {
     ASSERT(structure);
index 5ea4ee7..57a1fbb 100644 (file)
@@ -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 f460488..eed9fec 100644 (file)
@@ -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 503411b..0095a30 100644 (file)
@@ -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 f5811ee..8af1d99 100644 (file)
@@ -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 947e583..fca639b 100644 (file)
@@ -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 3fe12f9..c57a6ed 100644 (file)
@@ -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 Geoffrey Garen.
+
+        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  Enrica Casucci  <enrica@apple.com>
 
         Can't hit tab key more than 3 times continuously.
index 4bb56f7..de35af0 100644 (file)
@@ -307,6 +307,7 @@ void ScriptController::attachDebugger(JSDOMWindowShell* shell, JSC::Debugger* de
         return;
 
     JSDOMWindow* globalObject = shell->window();
+    JSLockHolder lock(globalObject->vm());
     if (debugger)
         debugger->attach(globalObject);
     else if (JSC::Debugger* currentDebugger = globalObject->debugger())