Changeset 34109 in webkit

Timestamp:

May 24, 2008 6:01:35 PM (16 years ago)

Author:

timothy@apple.com

Message:

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.

Location:

trunk/WebCore

Files:

• ChangeLog (modified) (1 diff)
• bindings/js/JSInspectedObjectWrapper.cpp (modified) (3 diffs)
• bindings/js/JSQuarantinedObjectWrapper.h (modified) (1 diff)

trunk/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/WebCore/bindings/js/JSInspectedObjectWrapper.cpp

@@ -34 +34 @@
 
 typedef HashMap<JSObject*, JSInspectedObjectWrapper*> WrapperMap;
+typedef HashMap<JSGlobalObject*, WrapperMap*> GlobalObjectWrapperMap;
 
-static WrapperMap& wrappers()
+static GlobalObjectWrapperMap& wrappers()
 {
-    static WrapperMap map;
+    static GlobalObjectWrapperMap map;
     return map;
 }
@@ -53 +54 @@
         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();
@@ -65 +67 @@
     : 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;
+    }
 }
 

trunk/WebCore/bindings/js/JSQuarantinedObjectWrapper.h

@@ -38 +38 @@
 
         KJS::JSObject* unwrappedObject() const { return m_unwrappedObject; }
+        KJS::JSGlobalObject* unwrappedGlobalObject() const { return m_unwrappedGlobalObject; };
         KJS::ExecState* unwrappedExecState() const;