Unreviewed, rolling out r144924.
authorrafaelw@chromium.org <rafaelw@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 6 Mar 2013 19:05:03 +0000 (19:05 +0000)
committerrafaelw@chromium.org <rafaelw@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 6 Mar 2013 19:05:03 +0000 (19:05 +0000)
http://trac.webkit.org/changeset/144924
https://bugs.webkit.org/show_bug.cgi?id=109908

caused multiple crashes in inspector/debugger tests

Source/WebCore:

* bindings/js/JSInjectedScriptHostCustom.cpp:
* bindings/js/ScriptObject.h:
* bindings/v8/ScriptObject.h:
* bindings/v8/custom/V8InjectedScriptHostCustom.cpp:
* inspector/InjectedScriptHost.cpp:
(WebCore::InjectedScriptHost::create):
(WebCore::InjectedScriptHost::InjectedScriptHost):
(WebCore::InjectedScriptHost::disconnect):
* inspector/InjectedScriptHost.h:
(WebCore):
(InjectedScriptHost):
* inspector/InjectedScriptHost.idl:
* inspector/InjectedScriptManager.cpp:
(WebCore::InjectedScriptManager::InjectedScriptManager):
(WebCore::InjectedScriptManager::discardInjectedScripts):
(WebCore::InjectedScriptManager::discardInjectedScriptsFor):
* inspector/InjectedScriptManager.h:
(InjectedScriptManager):
* inspector/InjectedScriptSource.js:
(.):

LayoutTests:

* inspector-protocol/persistent-id-expected.txt: Removed.
* inspector-protocol/persistent-id.html: Removed.
* inspector/console/command-line-api-expected.txt:

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

15 files changed:
LayoutTests/ChangeLog
LayoutTests/inspector-protocol/persistent-id-expected.txt [deleted file]
LayoutTests/inspector-protocol/persistent-id.html [deleted file]
LayoutTests/inspector/console/command-line-api-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/bindings/js/JSInjectedScriptHostCustom.cpp
Source/WebCore/bindings/js/ScriptObject.h
Source/WebCore/bindings/v8/ScriptObject.h
Source/WebCore/bindings/v8/custom/V8InjectedScriptHostCustom.cpp
Source/WebCore/inspector/InjectedScriptHost.cpp
Source/WebCore/inspector/InjectedScriptHost.h
Source/WebCore/inspector/InjectedScriptHost.idl
Source/WebCore/inspector/InjectedScriptManager.cpp
Source/WebCore/inspector/InjectedScriptManager.h
Source/WebCore/inspector/InjectedScriptSource.js

index 61b9341..d88cfd6 100644 (file)
@@ -1,3 +1,15 @@
+2013-03-06  Rafael Weinstein  <rafaelw@chromium.org>
+
+        Unreviewed, rolling out r144924.
+        http://trac.webkit.org/changeset/144924
+        https://bugs.webkit.org/show_bug.cgi?id=109908
+
+        caused multiple crashes in inspector/debugger tests
+
+        * inspector-protocol/persistent-id-expected.txt: Removed.
+        * inspector-protocol/persistent-id.html: Removed.
+        * inspector/console/command-line-api-expected.txt:
+
 2013-03-06  Florin Malita  <fmalita@chromium.org>
 
         SVG pattern to pattern reference does not work if first pattern has a child node
diff --git a/LayoutTests/inspector-protocol/persistent-id-expected.txt b/LayoutTests/inspector-protocol/persistent-id-expected.txt
deleted file mode 100644 (file)
index f45cf54..0000000
+++ /dev/null
@@ -1,5 +0,0 @@
-Test that evaluating same object will result in same remote object id.Bug 109908.
-
- SUCCESS: evaluated to an object with same id
-SUCCESS: evaluated to an object with id different from released one
-
diff --git a/LayoutTests/inspector-protocol/persistent-id.html b/LayoutTests/inspector-protocol/persistent-id.html
deleted file mode 100644 (file)
index c7b0141..0000000
+++ /dev/null
@@ -1,51 +0,0 @@
-<html>
-<head>
-<script type="text/javascript" src="../http/tests/inspector-protocol/resources/protocol-test.js"></script>
-<script>
-var aGlobalObject = {};
-
-function test()
-{
-
-    InspectorTest.sendCommand("Runtime.evaluate", { "expression": "aGlobalObject" }, didEvaluate1.bind(this));
-
-    function didEvaluate1(messageObject)
-    {
-        var objectId = messageObject.result.result.objectId;
-        InspectorTest.sendCommand("Runtime.evaluate", { "expression": "aGlobalObject" }, didEvaluate2.bind(this, objectId));
-    }
-
-    function didEvaluate2(expectedObjectId, messageObject)
-    {
-        var objectId = messageObject.result.result.objectId;
-        if (expectedObjectId === objectId)
-            InspectorTest.log("SUCCESS: evaluated to an object with same id");
-        else {
-            InspectorTest.log("FAIL: object ids are different, expected " + expectedObjectId + " found " + objectId);
-            return InspectorTest.completeTest();
-        }
-        InspectorTest.sendCommand("Runtime.releaseObject", { "objectId": objectId }, didReleaseObjectId.bind(this, objectId));
-    }
-
-    function didReleaseObjectId(releasedObjectId, messageObject)
-    {
-        InspectorTest.sendCommand("Runtime.evaluate", { "expression": "aGlobalObject" }, didEvaluate3.bind(this, releasedObjectId));
-    }
-    function didEvaluate3(releasedObjectId, messageObject)
-    {
-        var objectId = messageObject.result.result.objectId;
-        if (releasedObjectId === objectId) {
-            InspectorTest.log("FAIL: object id is the same as was released");
-            return InspectorTest.completeTest();
-        } else
-            InspectorTest.log("SUCCESS: evaluated to an object with id different from released one");
-        InspectorTest.completeTest();
-    }
-
-}
-</script>
-</head>
-<body onload="runTest()">
-<p>Test that evaluating same object will result in same remote object id.<a href="https://bugs.webkit.org/show_bug.cgi?id=109908">Bug 109908.</p>
-</body>
-</html>
index 0964e76..b0ec879 100644 (file)
@@ -1,4 +1,4 @@
-CONSOLE MESSAGE: line 1184: The console function $() has changed from $=getElementById(id) to $=querySelector(selector). You might try $("#%s")
+CONSOLE MESSAGE: line 1182: The console function $() has changed from $=getElementById(id) to $=querySelector(selector). You might try $("#%s")
 Tests that command line api works.
 
 
index 250c7b8..4d6c003 100644 (file)
@@ -1,3 +1,32 @@
+2013-03-06  Rafael Weinstein  <rafaelw@chromium.org>
+
+        Unreviewed, rolling out r144924.
+        http://trac.webkit.org/changeset/144924
+        https://bugs.webkit.org/show_bug.cgi?id=109908
+
+        caused multiple crashes in inspector/debugger tests
+
+        * bindings/js/JSInjectedScriptHostCustom.cpp:
+        * bindings/js/ScriptObject.h:
+        * bindings/v8/ScriptObject.h:
+        * bindings/v8/custom/V8InjectedScriptHostCustom.cpp:
+        * inspector/InjectedScriptHost.cpp:
+        (WebCore::InjectedScriptHost::create):
+        (WebCore::InjectedScriptHost::InjectedScriptHost):
+        (WebCore::InjectedScriptHost::disconnect):
+        * inspector/InjectedScriptHost.h:
+        (WebCore):
+        (InjectedScriptHost):
+        * inspector/InjectedScriptHost.idl:
+        * inspector/InjectedScriptManager.cpp:
+        (WebCore::InjectedScriptManager::InjectedScriptManager):
+        (WebCore::InjectedScriptManager::discardInjectedScripts):
+        (WebCore::InjectedScriptManager::discardInjectedScriptsFor):
+        * inspector/InjectedScriptManager.h:
+        (InjectedScriptManager):
+        * inspector/InjectedScriptSource.js:
+        (.):
+
 2013-03-06  Dmitry Zvorygin  <zvorygin@chromium.org>
 
         Introduce new message sources for logging.
index f30a7c9..0f15e4b 100644 (file)
@@ -263,31 +263,6 @@ JSValue JSInjectedScriptHost::inspect(ExecState* exec)
     return jsUndefined();
 }
 
-JSValue JSInjectedScriptHost::objectId(ExecState* exec)
-{
-    if (exec->argumentCount() < 1)
-        return jsUndefined();
-    JSObject* jsObject = exec->argument(0).getObject();
-    if (!jsObject)
-        return jsUndefined();
-    ExecState* globalExec = exec->lexicalGlobalObject()->globalExec();
-    ScriptObject object(globalExec, jsObject);
-    return jsNumber(impl()->objectId(object));
-}
-
-JSValue JSInjectedScriptHost::releaseObjectId(ExecState* exec)
-{
-    if (exec->argumentCount() < 1)
-        return jsUndefined();
-    JSObject* jsObject = exec->argument(0).getObject();
-    if (!jsObject)
-        return jsUndefined();
-    ExecState* globalExec = exec->lexicalGlobalObject()->globalExec();
-    ScriptObject object(globalExec, jsObject);
-    impl()->releaseObjectId(object);
-    return jsUndefined();
-}
-
 JSValue JSInjectedScriptHost::databaseId(ExecState* exec)
 {
     if (exec->argumentCount() < 1)
index 24d46b1..09133b7 100644 (file)
@@ -68,30 +68,4 @@ namespace WebCore {
 
 }
 
-namespace WTF {
-
-struct ScriptObjectHash {
-    static unsigned hash(const WebCore::ScriptObject& key)
-    {
-        return DefaultHash<JSC::JSObject*>::Hash::hash(key.jsObject());
-    }
-    static bool equal(const WebCore::ScriptObject& a, const WebCore::ScriptObject& b)
-    {
-        return a == b;
-    }
-    static const bool safeToCompareToEmptyOrDeleted = true;
-};
-template<> struct DefaultHash<WebCore::ScriptObject> {
-    typedef ScriptObjectHash Hash;
-};
-
-template<> struct HashTraits<WebCore::ScriptObject> : GenericHashTraits<WebCore::ScriptObject> {
-    static const bool emptyValueIsZero = true;
-    static const bool needsDestruction = true;
-    static void constructDeletedValue(WebCore::ScriptObject& slot) { new (NotNull, &slot) WebCore::ScriptObject(); }
-    static bool isDeletedValue(WebCore::ScriptObject value) { return value.hasNoValue(); }
-};
-
-}
-
 #endif // ScriptObject_h
index ea1579c..1c47a60 100644 (file)
@@ -66,31 +66,4 @@ namespace WebCore {
 
 }
 
-namespace WTF {
-
-struct ScriptObjectHash {
-    static unsigned hash(const WebCore::ScriptObject& key)
-    {
-        return key.v8Object()->GetIdentityHash();
-    }
-    static bool equal(const WebCore::ScriptObject& a, const WebCore::ScriptObject& b)
-    {
-        return a == b;
-    }
-    static const bool safeToCompareToEmptyOrDeleted = true;
-};
-template<> struct DefaultHash<WebCore::ScriptObject> {
-    typedef ScriptObjectHash Hash;
-};
-
-template<> struct HashTraits<WebCore::ScriptObject> : GenericHashTraits<WebCore::ScriptObject> {
-    static const bool emptyValueIsZero = true;
-    static const bool needsDestruction = true;
-    static void constructDeletedValue(WebCore::ScriptObject& slot) { new (NotNull, &slot) WebCore::ScriptObject(); }
-    static bool isDeletedValue(WebCore::ScriptObject value) { return value.hasNoValue(); }
-};
-
-}
-
-
 #endif // ScriptObject_h
index 68cd818..e63698f 100644 (file)
@@ -282,30 +282,6 @@ v8::Handle<v8::Value> V8InjectedScriptHost::inspectMethodCustom(const v8::Argume
     return v8::Undefined();
 }
 
-v8::Handle<v8::Value> V8InjectedScriptHost::objectIdMethodCustom(const v8::Arguments& args)
-{
-    if (args.Length() < 1)
-        return v8::Undefined();
-    v8::Handle<v8::Object> object = args[0]->ToObject();
-    if (object.IsEmpty())
-        return v8::Undefined();
-    InjectedScriptHost* host = V8InjectedScriptHost::toNative(args.Holder());
-    unsigned id = host->objectId(ScriptObject(ScriptState::current(), object));
-    return v8::Number::New(id);
-}
-
-v8::Handle<v8::Value> V8InjectedScriptHost::releaseObjectIdMethodCustom(const v8::Arguments& args)
-{
-    if (args.Length() < 1)
-        return v8::Undefined();
-    v8::Handle<v8::Object> object = args[0]->ToObject();
-    if (object.IsEmpty())
-        return v8::Undefined();
-    InjectedScriptHost* host = V8InjectedScriptHost::toNative(args.Holder());
-    unsigned id = host->releaseObjectId(ScriptObject(ScriptState::current(), object));
-    return v8::Number::New(id);
-}
-
 v8::Handle<v8::Value> V8InjectedScriptHost::databaseIdMethodCustom(const v8::Arguments& args)
 {
     if (args.Length() < 1)
index 14425af..2e634d4 100644 (file)
@@ -64,14 +64,13 @@ using namespace std;
 
 namespace WebCore {
 
-PassRefPtr<InjectedScriptHost> InjectedScriptHost::create(InjectedScriptManager* manager)
+PassRefPtr<InjectedScriptHost> InjectedScriptHost::create()
 {
-    return adoptRef(new InjectedScriptHost(manager));
+    return adoptRef(new InjectedScriptHost());
 }
 
-InjectedScriptHost::InjectedScriptHost(InjectedScriptManager* manager)
-    : m_manager(manager)
-    , m_inspectorAgent(0)
+InjectedScriptHost::InjectedScriptHost()
+    : m_inspectorAgent(0)
     , m_consoleAgent(0)
 #if ENABLE(SQL_DATABASE)
     , m_databaseAgent(0)
@@ -88,7 +87,6 @@ InjectedScriptHost::~InjectedScriptHost()
 
 void InjectedScriptHost::disconnect()
 {
-    m_manager = 0;
     m_inspectorAgent = 0;
     m_consoleAgent = 0;
 #if ENABLE(SQL_DATABASE)
@@ -125,20 +123,6 @@ void InjectedScriptHost::copyText(const String& text)
     Pasteboard::generalPasteboard()->writePlainText(text, Pasteboard::CannotSmartReplace);
 }
 
-unsigned InjectedScriptHost::objectId(const ScriptObject& object)
-{
-    if (!m_manager)
-        return 0;
-    return m_manager->objectId(object);
-}
-
-unsigned InjectedScriptHost::releaseObjectId(const ScriptObject& object)
-{
-    if (!m_manager)
-        return 0;
-    return m_manager->releaseObjectId(object);
-}
-
 ScriptValue InjectedScriptHost::InspectableObject::get(ScriptState*)
 {
     return ScriptValue();
index f0aad32..af66b28 100644 (file)
@@ -40,7 +40,6 @@ namespace WebCore {
 
 class Database;
 class InjectedScript;
-class InjectedScriptManager;
 class InspectorAgent;
 class InspectorConsoleAgent;
 class InspectorDOMAgent;
@@ -60,7 +59,7 @@ struct EventListenerInfo;
 
 class InjectedScriptHost : public RefCounted<InjectedScriptHost> {
 public:
-    static PassRefPtr<InjectedScriptHost> create(InjectedScriptManager*);
+    static PassRefPtr<InjectedScriptHost> create();
     ~InjectedScriptHost();
 
     void init(InspectorAgent* inspectorAgent
@@ -107,8 +106,6 @@ public:
 
     void clearConsoleMessages();
     void copyText(const String& text);
-    unsigned objectId(const ScriptObject&);
-    unsigned releaseObjectId(const ScriptObject&);
 #if ENABLE(SQL_DATABASE)
     String databaseIdImpl(Database*);
 #endif
@@ -119,9 +116,8 @@ public:
 #endif
 
 private:
-    explicit InjectedScriptHost(InjectedScriptManager*);
+    InjectedScriptHost();
 
-    InjectedScriptManager* m_manager;
     InspectorAgent* m_inspectorAgent;
     InspectorConsoleAgent* m_consoleAgent;
 #if ENABLE(SQL_DATABASE)
index deaaaa1..bec3485 100644 (file)
@@ -46,8 +46,6 @@
     [Custom] Array getInternalProperties(in any object);
     [Custom] Array getEventListeners(in Node node);
 
-    [Custom] unsigned int objectId(in any object);
-    [Custom] void releaseObjectId(in any object);
     [Custom] DOMString databaseId(in any database);
     [Custom] DOMString storageId(in any storage);
     [Custom] any evaluate(in DOMString text);
index c6fef54..9756edf 100644 (file)
 
 namespace WebCore {
 
-class InjectedScriptManager::ObjectIdMap {
-    WTF_MAKE_NONCOPYABLE(ObjectIdMap);
-public:
-    ObjectIdMap() : m_nextId(1) { }
-
-    unsigned objectId(const ScriptObject& object)
-    {
-        Map::AddResult result = m_objectToId.add(object, m_nextId);
-        if (result.isNewEntry)
-            ++m_nextId;
-        return result.iterator->value;
-    }
-
-    unsigned releaseObjectId(const ScriptObject& object)
-    {
-        return m_objectToId.take(object);
-    }
-
-private:
-
-    typedef HashMap<ScriptObject, unsigned> Map;
-    Map m_objectToId;
-    unsigned m_nextId;
-};
-
 PassOwnPtr<InjectedScriptManager> InjectedScriptManager::createForPage()
 {
     return adoptPtr(new InjectedScriptManager(&InjectedScriptManager::canAccessInspectedWindow));
@@ -80,7 +55,7 @@ PassOwnPtr<InjectedScriptManager> InjectedScriptManager::createForWorker()
 
 InjectedScriptManager::InjectedScriptManager(InspectedStateAccessCheck accessCheck)
     : m_nextInjectedScriptId(1)
-    , m_injectedScriptHost(InjectedScriptHost::create(this))
+    , m_injectedScriptHost(InjectedScriptHost::create())
     , m_inspectedStateAccessCheck(accessCheck)
 {
 }
@@ -138,7 +113,6 @@ void InjectedScriptManager::discardInjectedScripts()
 {
     m_idToInjectedScript.clear();
     m_scriptStateToId.clear();
-    m_scriptStateToObjectIdMap.clear();
 }
 
 void InjectedScriptManager::discardInjectedScriptsFor(DOMWindow* window)
@@ -146,16 +120,6 @@ void InjectedScriptManager::discardInjectedScriptsFor(DOMWindow* window)
     if (m_scriptStateToId.isEmpty())
         return;
 
-    // Destroy object id maps.
-    Vector<ScriptState*> scriptStatesToRemove;
-    for (ScriptStateToObjectIdMap::iterator it = m_scriptStateToObjectIdMap.begin(); it != m_scriptStateToObjectIdMap.end(); ++it) {
-        ScriptState* scriptState = it->key;
-        if (window == domWindowFromScriptState(scriptState))
-            scriptStatesToRemove.append(scriptState);
-    }
-    for (size_t i = 0; i < scriptStatesToRemove.size(); i++)
-        m_scriptStateToObjectIdMap.remove(scriptStatesToRemove[i]);
-
     Vector<long> idsToRemove;
     IdToInjectedScriptMap::iterator end = m_idToInjectedScript.end();
     for (IdToInjectedScriptMap::iterator it = m_idToInjectedScript.begin(); it != end; ++it) {
@@ -170,7 +134,7 @@ void InjectedScriptManager::discardInjectedScriptsFor(DOMWindow* window)
         m_idToInjectedScript.remove(idsToRemove[i]);
 
     // Now remove script states that have id but no injected script.
-    scriptStatesToRemove.clear();
+    Vector<ScriptState*> scriptStatesToRemove;
     for (ScriptStateToId::iterator it = m_scriptStateToId.begin(); it != m_scriptStateToId.end(); ++it) {
         ScriptState* scriptState = it->key;
         if (window == domWindowFromScriptState(scriptState))
@@ -191,30 +155,6 @@ void InjectedScriptManager::releaseObjectGroup(const String& objectGroup)
         it->value.releaseObjectGroup(objectGroup);
 }
 
-unsigned InjectedScriptManager::objectId(const ScriptObject& object)
-{
-    if (object.hasNoValue())
-        return 0;
-    ScriptState* state = object.scriptState();
-    ObjectIdMap* map = m_scriptStateToObjectIdMap.get(state);
-    if (!map) {
-        ScriptStateToObjectIdMap::AddResult result = m_scriptStateToObjectIdMap.set(state, adoptPtr(new ObjectIdMap()));
-        map = result.iterator->value.get();
-    }
-    return map->objectId(object);
-}
-
-unsigned InjectedScriptManager::releaseObjectId(const ScriptObject& object)
-{
-    if (object.hasNoValue())
-        return 0;
-    ScriptState* state = object.scriptState();
-    ObjectIdMap* map = m_scriptStateToObjectIdMap.get(state);
-    if (!map)
-        return 0;
-    return map->releaseObjectId(object);
-}
-
 String InjectedScriptManager::injectedScriptSource()
 {
     return String(reinterpret_cast<const char*>(InjectedScriptSource_js), sizeof(InjectedScriptSource_js));
index 24f8e29..17f8f75 100644 (file)
@@ -62,9 +62,6 @@ public:
     void discardInjectedScriptsFor(DOMWindow*);
     void releaseObjectGroup(const String& objectGroup);
 
-    unsigned objectId(const ScriptObject&);
-    unsigned releaseObjectId(const ScriptObject&);
-
     typedef bool (*InspectedStateAccessCheck)(ScriptState*);
     InspectedStateAccessCheck inspectedStateAccessCheck() const { return m_inspectedStateAccessCheck; }
 
@@ -84,9 +81,6 @@ private:
     InspectedStateAccessCheck m_inspectedStateAccessCheck;
     typedef HashMap<ScriptState*, int> ScriptStateToId;
     ScriptStateToId m_scriptStateToId;
-    class ObjectIdMap;
-    typedef HashMap<ScriptState*, OwnPtr<ObjectIdMap> > ScriptStateToObjectIdMap;
-    ScriptStateToObjectIdMap m_scriptStateToObjectIdMap;
 };
 
 } // namespace WebCore
index fbf96aa..8613074 100644 (file)
@@ -77,6 +77,7 @@ function bind(func, thisObject, var_args)
  */
 var InjectedScript = function()
 {
+    this._lastBoundObjectId = 1;
     this._idToWrappedObject = {};
     this._idToObjectGroupName = {};
     this._objectGroups = {};
@@ -222,7 +223,7 @@ InjectedScript.prototype = {
      */
     _bind: function(object, objectGroupName)
     {
-        var id = InjectedScriptHost.objectId(object);
+        var id = this._lastBoundObjectId++;
         this._idToWrappedObject[id] = object;
         var objectId = "{\"injectedScriptId\":" + injectedScriptId + ",\"id\":" + id + "}";
         if (objectGroupName) {
@@ -370,9 +371,6 @@ InjectedScript.prototype = {
      */
     _releaseObject: function(id)
     {
-        var object = this._idToWrappedObject[id];
-        if (object)
-            InjectedScriptHost.releaseObjectId(object);
         delete this._idToWrappedObject[id];
         delete this._idToObjectGroupName[id];
     },