Fixes a crash where a new Inspector would get an old
authortimothy@apple.com <timothy@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 25 May 2008 01:01:35 +0000 (01:01 +0000)
committertimothy@apple.com <timothy@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 25 May 2008 01:01:35 +0000 (01:01 +0000)
JSInspectedObjectWrapper for a user agent CSSStyleDeclaration.
Since these style objects shared between pages, the wrapper cache
would have a wrapper for the object still. But the wrapper was
for a previous global object and with a disconnected frame. This
fixes the wrapper cache so wrappers are remembered per global object
and the object they are wrapping.

<rdar://problem/5958567> repro crash in WebCore::Frame::keepAlive()
opening inspector window after closing it

Reviewed by Darin Adler.

* bindings/js/JSInspectedObjectWrapper.cpp:
(WebCore::wrappers): Return a GlobalObjectWrapperMap reference.
(WebCore::JSInspectedObjectWrapper::wrap): Find the WrapperMap
by the dynamicGlobalObject then find the wrapper for unwrappedObject.
(WebCore::JSInspectedObjectWrapper::JSInspectedObjectWrapper): Changes
how the wrapper is added to the wrapper cache.
(WebCore::JSInspectedObjectWrapper::~JSInspectedObjectWrapper): Changes
how the wrapper is removed from the wrapper cache.
* bindings/js/JSQuarantinedObjectWrapper.h:
(WebCore::JSQuarantinedObjectWrapper:unwrappedGlobalObject): Added.

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

WebCore/ChangeLog
WebCore/bindings/js/JSInspectedObjectWrapper.cpp
WebCore/bindings/js/JSQuarantinedObjectWrapper.h

index c71068d..fe9b7f4 100644 (file)
@@ -1,3 +1,29 @@
+2008-05-24  Timothy Hatcher  <timothy@apple.com>
+
+        Fixes a crash where a new Inspector would get an old
+        JSInspectedObjectWrapper for a user agent CSSStyleDeclaration.
+        Since these style objects shared between pages, the wrapper cache
+        would have a wrapper for the object still. But the wrapper was
+        for a previous global object and with a disconnected frame. This
+        fixes the wrapper cache so wrappers are remembered per global object
+        and the object they are wrapping.
+
+        <rdar://problem/5958567> repro crash in WebCore::Frame::keepAlive()
+        opening inspector window after closing it
+
+        Reviewed by Darin Adler.
+
+        * bindings/js/JSInspectedObjectWrapper.cpp:
+        (WebCore::wrappers): Return a GlobalObjectWrapperMap reference.
+        (WebCore::JSInspectedObjectWrapper::wrap): Find the WrapperMap
+        by the dynamicGlobalObject then find the wrapper for unwrappedObject.
+        (WebCore::JSInspectedObjectWrapper::JSInspectedObjectWrapper): Changes
+        how the wrapper is added to the wrapper cache.
+        (WebCore::JSInspectedObjectWrapper::~JSInspectedObjectWrapper): Changes
+        how the wrapper is removed from the wrapper cache.
+        * bindings/js/JSQuarantinedObjectWrapper.h:
+        (WebCore::JSQuarantinedObjectWrapper:unwrappedGlobalObject): Added.
+
 2008-05-24  Alexey Proskuryakov  <ap@webkit.org>
 
         Reviewed by Maciej.
index 4b534cb..2a06fd4 100644 (file)
@@ -33,10 +33,11 @@ using namespace KJS;
 namespace WebCore {
 
 typedef HashMap<JSObject*, JSInspectedObjectWrapper*> WrapperMap;
+typedef HashMap<JSGlobalObject*, WrapperMap*> GlobalObjectWrapperMap;
 
-static WrapperMap& wrappers()
+static GlobalObjectWrapperMap& wrappers()
 {
-    static WrapperMap map;
+    static GlobalObjectWrapperMap map;
     return map;
 }
 
@@ -52,8 +53,9 @@ JSValue* JSInspectedObjectWrapper::wrap(ExecState* unwrappedExec, JSValue* unwra
     if (unwrappedObject->inherits(&JSInspectedObjectWrapper::s_info))
         return unwrappedObject;
 
-    if (JSInspectedObjectWrapper* wrapper = wrappers().get(unwrappedObject))
-        return wrapper;
+    if (WrapperMap* wrapperMap = wrappers().get(unwrappedExec->dynamicGlobalObject()))
+        if (JSInspectedObjectWrapper* wrapper = wrapperMap->get(unwrappedObject))
+            return wrapper;
 
     JSValue* prototype = unwrappedObject->prototype();
     JSValue* wrappedPrototype = prototype ? wrap(unwrappedExec, prototype) : 0;
@@ -64,13 +66,28 @@ JSValue* JSInspectedObjectWrapper::wrap(ExecState* unwrappedExec, JSValue* unwra
 JSInspectedObjectWrapper::JSInspectedObjectWrapper(ExecState* unwrappedExec, JSObject* unwrappedObject, JSValue* wrappedPrototype)
     : JSQuarantinedObjectWrapper(unwrappedExec, unwrappedObject, wrappedPrototype)
 {
-    ASSERT(!wrappers().contains(unwrappedObject));
-    wrappers().set(unwrappedObject, this);
+    WrapperMap* wrapperMap = wrappers().get(unwrappedGlobalObject());
+    if (!wrapperMap) {
+        wrapperMap = new WrapperMap;
+        wrappers().set(unwrappedGlobalObject(), wrapperMap);
+    }
+
+    ASSERT(!wrapperMap->contains(unwrappedObject));
+    wrapperMap->set(unwrappedObject, this);
 }
 
 JSInspectedObjectWrapper::~JSInspectedObjectWrapper()
 {
-    wrappers().remove(unwrappedObject());
+    ASSERT(wrappers().contains(unwrappedGlobalObject()));
+    WrapperMap* wrapperMap = wrappers().get(unwrappedGlobalObject());
+
+    ASSERT(wrapperMap->contains(unwrappedObject()));
+    wrapperMap->remove(unwrappedObject());
+
+    if (wrapperMap->isEmpty()) {
+        wrappers().remove(unwrappedGlobalObject());
+        delete wrapperMap;
+    }
 }
 
 JSValue* JSInspectedObjectWrapper::prepareIncomingValue(ExecState*, JSValue* value) const
index a4b7d3a..c183763 100644 (file)
@@ -37,6 +37,7 @@ namespace WebCore {
         virtual ~JSQuarantinedObjectWrapper();
 
         KJS::JSObject* unwrappedObject() const { return m_unwrappedObject; }
+        KJS::JSGlobalObject* unwrappedGlobalObject() const { return m_unwrappedGlobalObject; };
         KJS::ExecState* unwrappedExecState() const;
 
         bool allowsUnwrappedAccessFrom(const KJS::ExecState*) const;