Diff
Modified: branches/safari-534-branch/LayoutTests/ChangeLog (87244 => 87245)
--- branches/safari-534-branch/LayoutTests/ChangeLog 2011-05-25 00:48:00 UTC (rev 87244)
+++ branches/safari-534-branch/LayoutTests/ChangeLog 2011-05-25 00:56:44 UTC (rev 87245)
@@ -1,5 +1,30 @@
2011-05-24 Lucas Forschler <[email protected]>
+ Merged r87179.
+
+ 2011-05-24 Adam Roben <[email protected]>
+
+ Test that we don't crash when a JS wrapper for an NPObject is destroyed after its plugin is unloaded
+
+ Test for <http://webkit.org/b/61316> <rdar://problem/9489824> Crash in deallocateNPObject
+ when reloading yahoo.com webarchive in WebKit2
+
+ and
+
+ <http://webkit.org/b/61317> <rdar://problem/9489829> Crash in _NPN_DeallocateObject when
+ reloading yahoo.com webarchive in WebKit1
+
+ Reviewed by Oliver Hunt.
+
+ * plugins/npobject-js-wrapper-destroyed-after-plugin-unload-expected.txt: Added.
+ * plugins/npobject-js-wrapper-destroyed-after-plugin-unload.html: Added.
+ (startTest): Gets a JS wrapper for an NPObject from the plugin, allocate a bunch of memory
+ so the JS wrapper will be finalized, then destroy the plugin and wait for a little bit
+ before calling finishTest.
+ (finishTest): Force a GC so the JS wrapper will be destroyed. If we didn't crash, we passed!
+
+2011-05-24 Lucas Forschler <[email protected]>
+
Merged r87083.
2011-05-23 Abhishek Arya <[email protected]>
Copied: branches/safari-534-branch/LayoutTests/plugins/npobject-js-wrapper-destroyed-after-plugin-unload-expected.txt (from rev 87179, trunk/LayoutTests/plugins/npobject-js-wrapper-destroyed-after-plugin-unload-expected.txt) (0 => 87245)
--- branches/safari-534-branch/LayoutTests/plugins/npobject-js-wrapper-destroyed-after-plugin-unload-expected.txt (rev 0)
+++ branches/safari-534-branch/LayoutTests/plugins/npobject-js-wrapper-destroyed-after-plugin-unload-expected.txt 2011-05-25 00:56:44 UTC (rev 87245)
@@ -0,0 +1,3 @@
+This test will only work in DumpRenderTree/WebKitTestRunner.
+
+PASSED
Copied: branches/safari-534-branch/LayoutTests/plugins/npobject-js-wrapper-destroyed-after-plugin-unload.html (from rev 87179, trunk/LayoutTests/plugins/npobject-js-wrapper-destroyed-after-plugin-unload.html) (0 => 87245)
--- branches/safari-534-branch/LayoutTests/plugins/npobject-js-wrapper-destroyed-after-plugin-unload.html (rev 0)
+++ branches/safari-534-branch/LayoutTests/plugins/npobject-js-wrapper-destroyed-after-plugin-unload.html 2011-05-25 00:56:44 UTC (rev 87245)
@@ -0,0 +1,51 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <script>
+ function startTest() {
+ if (window.layoutTestController) {
+ layoutTestController.dumpAsText();
+ layoutTestController.waitUntilDone();
+ }
+
+ // Access all objects/properties that we're going to use later in the test so that JS
+ // allocations only happen when we expect.
+ var body = document.body;
+ body.removeChild;
+ var plugin = body.getElementsByTagName('embed')[0];
+ var testObject = plugin.testObject;
+ setTimeout;
+
+ testObject = null;
+
+ // Allocate a bunch of JS memory. This should cause testObject to be finalized, but it's
+ // destructor shouldn't run until the GCController.collect call we make later.
+ var array = new Array(10000);
+ for (var i = 0; i < 10000; ++i)
+ array[i] = new Object();
+
+ // Remove the plugin and wait for a little bit to ensure it has been unloaded (WebKit1
+ // on Windows unloads plugins after a delay).
+ body.removeChild(plugin);
+ setTimeout(finishTest, 250);
+ }
+
+ function finishTest() {
+ // Force a GC. If we don't crash here, we've passed the test.
+ if (window.GCController)
+ GCController.collect();
+
+ document.body.appendChild(document.createTextNode('PASSED'));
+
+ if (window.layoutTestController)
+ layoutTestController.notifyDone();
+ }
+
+ addEventListener('load', startTest, false);
+ </script>
+</head>
+<body>
+ <p>This test will only work in DumpRenderTree/WebKitTestRunner.</p>
+ <embed type="application/x-webkit-test-netscape">
+</body>
+</html>
Modified: branches/safari-534-branch/Source/WebCore/ChangeLog (87244 => 87245)
--- branches/safari-534-branch/Source/WebCore/ChangeLog 2011-05-25 00:48:00 UTC (rev 87244)
+++ branches/safari-534-branch/Source/WebCore/ChangeLog 2011-05-25 00:56:44 UTC (rev 87245)
@@ -1,3 +1,43 @@
+2011-05-24 Adam Roben <[email protected]>
+
+ Leopard build fix
+
+ * bridge/runtime_root.cpp: Added a missing #include.
+
+2011-05-24 Jian Li <[email protected]>
+
+ Merged r87179.
+
+ 2011-05-24 Adam Roben <[email protected]>
+
+ Invalidate RuntimeObjects when they are finalized
+
+ This will cause the underlying NPObject to be released at finalization time, rather than at
+ destruction time (which is unpredictable and could occur after the plugin has been
+ unloaded).
+
+ Test: plugins/npobject-js-wrapper-destroyed-after-plugin-unload.html
+
+ Fixes <http://webkit.org/b/61317> <rdar://problem/9489829> Crash in _NPN_DeallocateObject
+ when reloading yahoo.com webarchive in WebKit1
+
+ Reviewed by Oliver Hunt.
+
+ * bridge/runtime_object.cpp:
+ (JSC::Bindings::RuntimeObject::~RuntimeObject): Assert that we've already been invalidated.
+
+ * bridge/runtime_root.cpp:
+ (JSC::Bindings::RootObject::invalidate):
+ (JSC::Bindings::RootObject::addRuntimeObject):
+ Updated for m_runtimeObjects type change.
+
+ (JSC::Bindings::RootObject::finalize): Added. Invalidates the RuntimeObject and removes it
+ from the map.
+
+ * bridge/runtime_root.h: Now inherits from WeakHandleOwner.
+ Changed m_runtimeObjects from a WeakGCMap to a HashMap of JSC::Weak objects so that we will
+ be notified when the RuntimeObjects are finalized.
+
2011-05-24 Lucas Forschler <[email protected]>
Merged r87102.
Modified: branches/safari-534-branch/Source/WebCore/bridge/runtime_object.cpp (87244 => 87245)
--- branches/safari-534-branch/Source/WebCore/bridge/runtime_object.cpp 2011-05-25 00:48:00 UTC (rev 87244)
+++ branches/safari-534-branch/Source/WebCore/bridge/runtime_object.cpp 2011-05-25 00:56:44 UTC (rev 87245)
@@ -47,6 +47,7 @@
RuntimeObject::~RuntimeObject()
{
+ ASSERT(!m_instance);
}
void RuntimeObject::invalidate()
Modified: branches/safari-534-branch/Source/WebCore/bridge/runtime_root.cpp (87244 => 87245)
--- branches/safari-534-branch/Source/WebCore/bridge/runtime_root.cpp 2011-05-25 00:48:00 UTC (rev 87244)
+++ branches/safari-534-branch/Source/WebCore/bridge/runtime_root.cpp 2011-05-25 00:56:44 UTC (rev 87245)
@@ -101,9 +101,9 @@
return;
{
- WeakGCMap<RuntimeObject*, RuntimeObject>::iterator end = m_runtimeObjects.end();
- for (WeakGCMap<RuntimeObject*, RuntimeObject>::iterator it = m_runtimeObjects.begin(); it != end; ++it) {
- it.get().second->invalidate();
+ HashMap<RuntimeObject*, JSC::Weak<RuntimeObject> >::iterator end = m_runtimeObjects.end();
+ for (HashMap<RuntimeObject*, JSC::Weak<RuntimeObject> >::iterator it = m_runtimeObjects.begin(); it != end; ++it) {
+ it->second.get()->invalidate();
}
m_runtimeObjects.clear();
@@ -179,7 +179,7 @@
ASSERT(m_isValid);
ASSERT(!m_runtimeObjects.get(object));
- m_runtimeObjects.set(globalData, object, object);
+ m_runtimeObjects.set(object, JSC::Weak<RuntimeObject>(globalData, object, this));
}
void RootObject::removeRuntimeObject(RuntimeObject* object)
@@ -192,4 +192,13 @@
m_runtimeObjects.take(object);
}
+void RootObject::finalize(JSC::Handle<JSC::Unknown> handle, void*)
+{
+ RuntimeObject* object = static_cast<RuntimeObject*>(asObject(handle.get()));
+ ASSERT(m_runtimeObjects.contains(object));
+
+ object->invalidate();
+ m_runtimeObjects.remove(object);
+}
+
} } // namespace JSC::Bindings
Modified: branches/safari-534-branch/Source/WebCore/bridge/runtime_root.h (87244 => 87245)
--- branches/safari-534-branch/Source/WebCore/bridge/runtime_root.h 2011-05-25 00:48:00 UTC (rev 87244)
+++ branches/safari-534-branch/Source/WebCore/bridge/runtime_root.h 2011-05-25 00:56:44 UTC (rev 87245)
@@ -31,8 +31,10 @@
#endif
#include <heap/Strong.h>
-#include <runtime/WeakGCMap.h>
+#include <heap/Weak.h>
#include <wtf/Forward.h>
+#include <wtf/HashCountedSet.h>
+#include <wtf/HashSet.h>
#include <wtf/Noncopyable.h>
#include <wtf/PassRefPtr.h>
#include <wtf/RefCounted.h>
@@ -52,7 +54,7 @@
extern RootObject* findProtectingRootObject(JSObject*);
extern RootObject* findRootObject(JSGlobalObject*);
-class RootObject : public RefCounted<RootObject> {
+class RootObject : public RefCounted<RootObject>, private JSC::WeakHandleOwner {
friend class JavaJSObject;
public:
@@ -82,14 +84,17 @@
private:
RootObject(const void* nativeHandle, JSGlobalObject*);
-
+
+ // WeakHandleOwner
+ virtual void finalize(JSC::Handle<JSC::Unknown>, void* context);
+
bool m_isValid;
const void* m_nativeHandle;
Strong<JSGlobalObject> m_globalObject;
ProtectCountSet m_protectCountSet;
- WeakGCMap<RuntimeObject*, RuntimeObject> m_runtimeObjects; // Really need a WeakGCSet, but this will do.
+ HashMap<RuntimeObject*, JSC::Weak<RuntimeObject> > m_runtimeObjects; // Really need a WeakGCSet, but this will do.
HashSet<InvalidationCallback*> m_invalidationCallbacks;
};
Modified: branches/safari-534-branch/Source/WebKit2/ChangeLog (87244 => 87245)
--- branches/safari-534-branch/Source/WebKit2/ChangeLog 2011-05-25 00:48:00 UTC (rev 87244)
+++ branches/safari-534-branch/Source/WebKit2/ChangeLog 2011-05-25 00:56:44 UTC (rev 87245)
@@ -1,5 +1,40 @@
2011-05-24 Lucas Forschler <[email protected]>
+ Merged r87179.
+
+ 2011-05-24 Adam Roben <[email protected]>
+
+ Invalidate JSNPObjects when they are finalized
+
+ This will cause the underlying NPObject to be released at finalization time, rather than at
+ destruction time (which is unpredictable and could occur after the plugin has been
+ unloaded).
+
+ Test: plugins/npobject-js-wrapper-destroyed-after-plugin-unload.html
+
+ Fixes <http://webkit.org/b/61316> <rdar://problem/9489824> Crash in deallocateNPObject when
+ reloading yahoo.com webarchive in WebKit2
+
+ Reviewed by Oliver Hunt.
+
+ * WebProcess/Plugins/Netscape/JSNPObject.cpp:
+ (WebKit::JSNPObject::~JSNPObject): Assert that we've already been invalidated, rather than
+ trying to perform invalidation now (when the plugin might already be unloaded).
+
+ * WebProcess/Plugins/Netscape/NPRuntimeObjectMap.cpp:
+ (WebKit::NPRuntimeObjectMap::getOrCreateJSObject):
+ (WebKit::NPRuntimeObjectMap::invalidate):
+ Updated for m_jsNPObjects type change.
+
+ (WebKit::NPRuntimeObjectMap::finalize): Added. Invalidates the JSNPObject and removes it
+ from the map.
+
+ * WebProcess/Plugins/Netscape/NPRuntimeObjectMap.h: Now inherits from WeakHandleOwner.
+ Changed m_jsNPObjects from a WeakGCMap to a HashMap of JSC::Weak objects so that we will be
+ notified when the JSNPObjects are finalized.
+
+2011-05-24 Lucas Forschler <[email protected]>
+
Merged r87170.
2011-05-24 Sam Weinig <[email protected]>
Modified: branches/safari-534-branch/Source/WebKit2/WebProcess/Plugins/Netscape/JSNPObject.cpp (87244 => 87245)
--- branches/safari-534-branch/Source/WebKit2/WebProcess/Plugins/Netscape/JSNPObject.cpp 2011-05-25 00:48:00 UTC (rev 87244)
+++ branches/safari-534-branch/Source/WebKit2/WebProcess/Plugins/Netscape/JSNPObject.cpp 2011-05-25 00:56:44 UTC (rev 87245)
@@ -64,9 +64,7 @@
JSNPObject::~JSNPObject()
{
- if (!m_npObject)
- return;
- releaseNPObject(m_npObject);
+ ASSERT(!m_npObject);
}
void JSNPObject::invalidate()
Modified: branches/safari-534-branch/Source/WebKit2/WebProcess/Plugins/Netscape/NPRuntimeObjectMap.cpp (87244 => 87245)
--- branches/safari-534-branch/Source/WebKit2/WebProcess/Plugins/Netscape/NPRuntimeObjectMap.cpp 2011-05-25 00:48:00 UTC (rev 87244)
+++ branches/safari-534-branch/Source/WebKit2/WebProcess/Plugins/Netscape/NPRuntimeObjectMap.cpp 2011-05-25 00:56:44 UTC (rev 87245)
@@ -95,11 +95,11 @@
if (NPJSObject::isNPJSObject(npObject))
return NPJSObject::toNPJSObject(npObject)->jsObject();
- if (JSNPObject* jsNPObject = m_jsNPObjects.get(npObject))
- return jsNPObject;
+ if (JSC::Weak<JSNPObject> jsNPObject = m_jsNPObjects.get(npObject))
+ return jsNPObject.get();
JSNPObject* jsNPObject = new (&globalObject->globalData()) JSNPObject(globalObject, this, npObject);
- m_jsNPObjects.set(globalObject->globalData(), npObject, jsNPObject);
+ m_jsNPObjects.set(npObject, JSC::Weak<JSNPObject>(globalObject->globalData(), jsNPObject, this, npObject));
return jsNPObject;
}
@@ -217,9 +217,9 @@
// We shouldn't have any NPJSObjects left now.
ASSERT(m_npJSObjects.isEmpty());
- WeakGCMap<NPObject*, JSNPObject>::iterator end = m_jsNPObjects.end();
- for (WeakGCMap<NPObject*, JSNPObject>::iterator ptr = m_jsNPObjects.begin(); ptr != end; ++ptr)
- ptr.get().second->invalidate();
+ HashMap<NPObject*, JSC::Weak<JSNPObject> >::iterator end = m_jsNPObjects.end();
+ for (HashMap<NPObject*, JSC::Weak<JSNPObject> >::iterator ptr = m_jsNPObjects.begin(); ptr != end; ++ptr)
+ ptr->second.get()->invalidate();
m_jsNPObjects.clear();
}
@@ -265,4 +265,14 @@
globalExceptionString() = String();
}
+void NPRuntimeObjectMap::finalize(JSC::Handle<JSC::Unknown> handle, void* context)
+{
+ HashMap<NPObject*, JSC::Weak<JSNPObject> >::iterator found = m_jsNPObjects.find(static_cast<NPObject*>(context));
+ ASSERT(found != m_jsNPObjects.end());
+ ASSERT_UNUSED(handle, asObject(handle.get()) == found->second);
+
+ found->second.get()->invalidate();
+ m_jsNPObjects.remove(found);
+}
+
} // namespace WebKit
Modified: branches/safari-534-branch/Source/WebKit2/WebProcess/Plugins/Netscape/NPRuntimeObjectMap.h (87244 => 87245)
--- branches/safari-534-branch/Source/WebKit2/WebProcess/Plugins/Netscape/NPRuntimeObjectMap.h 2011-05-25 00:48:00 UTC (rev 87244)
+++ branches/safari-534-branch/Source/WebKit2/WebProcess/Plugins/Netscape/NPRuntimeObjectMap.h 2011-05-25 00:56:44 UTC (rev 87245)
@@ -26,7 +26,7 @@
#ifndef NPJSObjectWrapperMap_h
#define NPJSObjectWrapperMap_h
-#include <_javascript_Core/WeakGCMap.h>
+#include <heap/Weak.h>
#include <wtf/Forward.h>
#include <wtf/HashMap.h>
@@ -48,7 +48,7 @@
class PluginView;
// A per plug-in map of NPObjects that wrap _javascript_ objects.
-class NPRuntimeObjectMap {
+class NPRuntimeObjectMap : private JSC::WeakHandleOwner {
public:
explicit NPRuntimeObjectMap(PluginView*);
@@ -85,10 +85,13 @@
static void moveGlobalExceptionToExecState(JSC::ExecState*);
private:
+ // WeakHandleOwner
+ virtual void finalize(JSC::Handle<JSC::Unknown>, void* context);
+
PluginView* m_pluginView;
HashMap<JSC::JSObject*, NPJSObject*> m_npJSObjects;
- JSC::WeakGCMap<NPObject*, JSNPObject> m_jsNPObjects;
+ HashMap<NPObject*, JSC::Weak<JSNPObject> > m_jsNPObjects;
};
} // namespace WebKit
