[SCM] WebKit Debian packaging branch, webkit-1.2, updated. upstream/1.1.90-6072-g9a69373
ap at apple.com
ap at apple.com
Thu Apr 8 01:47:49 UTC 2010
The following commit has been merged in the webkit-1.2 branch:
commit 1289d37127110d94206d5f840d96846a2e8475ac
Author: ap at apple.com <ap at apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Mon Feb 15 19:16:21 2010 +0000
Reviewed by Kevin Decker.
<rdar://problem/7130641> Browser objects identity is not preserved by Safari
Test: plugins/netscape-browser-object-identity.html
* bridge/runtime_root.h: (JSC::Bindings::RootObject::addInvalidationCallback):
RootObject can now call out during invalidation, making it possible for other code to know
when this happens.
* bridge/runtime_root.cpp:
(JSC::Bindings::RootObject::InvalidationCallback::~InvalidationCallback): Empty destructor,
in cpp file since it's virtual.
(JSC::Bindings::RootObject::invalidate): Invoke invalidation callbacks.
* bridge/NP_jsobject.cpp:
(ObjectMap): Keep a JSObject->NPObject map for each RootObject. It somewhat cleaner to
keep it outside RootObject, because (1) it is agnostic of what kinds of objects can wrap
JSObject, and (2) out of process NPAPI implementation also keeps its corresponding map
separately, due to supporting per-instance granularity (as opposed to per-RootObject here).
(jsDeallocate): Remove the corresponding map entry.
(_NPN_CreateScriptObject): Try to fetch existing object from the map, incrementing refcount.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@54783 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 212eb45..b21ce22 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,13 @@
+2010-02-12 Alexey Proskuryakov <ap at apple.com>
+
+ Reviewed by Kevin Decker.
+
+ <rdar://problem/7130641> Browser objects identity is not preserved by Safari
+
+ * plugins/netscape-browser-object-identity-expected.txt: Added.
+ * plugins/netscape-browser-object-identity.html: Added.
+ * plugins/script-tests/netscape-browser-object-identity.js: Added.
+
2010-02-15 Pavel Feldman <pfeldman at chromium.org>
Reviewed by Timothy Hatcher.
diff --git a/LayoutTests/plugins/netscape-browser-object-identity-expected.txt b/LayoutTests/plugins/netscape-browser-object-identity-expected.txt
new file mode 100644
index 0000000..3c108dc
--- /dev/null
+++ b/LayoutTests/plugins/netscape-browser-object-identity-expected.txt
@@ -0,0 +1,18 @@
+Test that plug-in doesn't get a new browser object instance each time
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS embed.getAndForgetRememberedObject().foo is 'bar'
+PASS embed.refCount(obj) is 1
+PASS embed.refCount(obj) is 1
+PASS embed.refCount(obj) is 2
+PASS embed.getRememberedObject() is obj
+PASS embed.getRememberedObject() is obj
+PASS embed.refCount(obj) is 2
+PASS embed.getAndForgetRememberedObject() is obj
+PASS embed.refCount(obj) is 1
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/plugins/netscape-browser-object-identity.html b/LayoutTests/plugins/netscape-browser-object-identity.html
new file mode 100644
index 0000000..882b432
--- /dev/null
+++ b/LayoutTests/plugins/netscape-browser-object-identity.html
@@ -0,0 +1,13 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<link rel="stylesheet" href="../fast/js/resources/js-test-style.css">
+<script src="../fast/js/resources/js-test-pre.js"></script>
+</head>
+<body>
+<p id="description"></p>
+<div id="console"></div>
+<script src="script-tests/netscape-browser-object-identity.js"></script>
+<script src="../fast/js/resources/js-test-post.js"></script>
+</body>
+</html>
diff --git a/LayoutTests/plugins/script-tests/netscape-browser-object-identity.js b/LayoutTests/plugins/script-tests/netscape-browser-object-identity.js
new file mode 100644
index 0000000..e6c8fa2
--- /dev/null
+++ b/LayoutTests/plugins/script-tests/netscape-browser-object-identity.js
@@ -0,0 +1,38 @@
+description("Test that plug-in doesn't get a new browser object instance each time")
+
+function gc()
+{
+ if (window.GCController)
+ return GCController.collect();
+
+ for (var i = 0; i < 10000; i++) { // > force garbage collection (FF requires about 9K allocations before a collect)
+ var s = new String("abc");
+ }
+}
+
+embed = document.createElement("embed");
+embed.type = "application/x-webkit-test-netscape";
+document.body.appendChild(embed);
+
+var obj = new XMLHttpRequest;
+obj.foo = "bar";
+embed.remember(obj);
+obj = null;
+gc();
+shouldBe("embed.getAndForgetRememberedObject().foo", "'bar'");
+gc();
+
+obj = new XMLHttpRequest;
+shouldBe("embed.refCount(obj)", "1");
+shouldBe("embed.refCount(obj)", "1");
+embed.remember(obj);
+shouldBe("embed.refCount(obj)", "2");
+shouldBe("embed.getRememberedObject()", "obj");
+shouldBe("embed.getRememberedObject()", "obj");
+shouldBe("embed.refCount(obj)", "2");
+shouldBe("embed.getAndForgetRememberedObject()", "obj");
+shouldBe("embed.refCount(obj)", "1");
+obj = null;
+gc();
+
+var successfullyParsed = true;
diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index 30d5f0b..c408627 100644
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,28 @@
+2010-02-12 Alexey Proskuryakov <ap at apple.com>
+
+ Reviewed by Kevin Decker.
+
+ <rdar://problem/7130641> Browser objects identity is not preserved by Safari
+
+ Test: plugins/netscape-browser-object-identity.html
+
+ * bridge/runtime_root.h: (JSC::Bindings::RootObject::addInvalidationCallback):
+ RootObject can now call out during invalidation, making it possible for other code to know
+ when this happens.
+
+ * bridge/runtime_root.cpp:
+ (JSC::Bindings::RootObject::InvalidationCallback::~InvalidationCallback): Empty destructor,
+ in cpp file since it's virtual.
+ (JSC::Bindings::RootObject::invalidate): Invoke invalidation callbacks.
+
+ * bridge/NP_jsobject.cpp:
+ (ObjectMap): Keep a JSObject->NPObject map for each RootObject. It somewhat cleaner to
+ keep it outside RootObject, because (1) it is agnostic of what kinds of objects can wrap
+ JSObject, and (2) out of process NPAPI implementation also keeps its corresponding map
+ separately, due to supporting per-instance granularity (as opposed to per-RootObject here).
+ (jsDeallocate): Remove the corresponding map entry.
+ (_NPN_CreateScriptObject): Try to fetch existing object from the map, incrementing refcount.
+
2010-02-15 Philippe Normand <pnormand at igalia.com>
Rubber-stamped by Gustavo Noronha Silva.
diff --git a/WebCore/bridge/NP_jsobject.cpp b/WebCore/bridge/NP_jsobject.cpp
index 09851df..2b1d17f 100644
--- a/WebCore/bridge/NP_jsobject.cpp
+++ b/WebCore/bridge/NP_jsobject.cpp
@@ -51,6 +51,64 @@ using namespace JSC;
using namespace JSC::Bindings;
using namespace WebCore;
+class ObjectMap {
+public:
+ NPObject* get(RootObject* rootObject, JSObject* jsObject)
+ {
+ return m_map.get(rootObject).get(jsObject);
+ }
+
+ void add(RootObject* rootObject, JSObject* jsObject, NPObject* npObject)
+ {
+ HashMap<RootObject*, JSToNPObjectMap>::iterator iter = m_map.find(rootObject);
+ if (iter == m_map.end()) {
+ rootObject->addInvalidationCallback(&m_invalidationCallback);
+ iter = m_map.add(rootObject, JSToNPObjectMap()).first;
+ }
+
+ ASSERT(iter->second.find(jsObject) == iter->second.end());
+ iter->second.add(jsObject, npObject);
+ }
+
+ void remove(RootObject* rootObject)
+ {
+ HashMap<RootObject*, JSToNPObjectMap>::iterator iter = m_map.find(rootObject);
+ ASSERT(iter != m_map.end());
+ m_map.remove(iter);
+ }
+
+ void remove(RootObject* rootObject, JSObject* jsObject)
+ {
+ HashMap<RootObject*, JSToNPObjectMap>::iterator iter = m_map.find(rootObject);
+ ASSERT(iter != m_map.end());
+ ASSERT(iter->second.find(jsObject) != iter->second.end());
+
+ iter->second.remove(jsObject);
+ }
+
+private:
+ struct RootObjectInvalidationCallback : public RootObject::InvalidationCallback {
+ virtual void operator()(RootObject*);
+ };
+ RootObjectInvalidationCallback m_invalidationCallback;
+
+ // JSObjects are protected by RootObject.
+ typedef HashMap<JSObject*, NPObject*> JSToNPObjectMap;
+ HashMap<RootObject*, JSToNPObjectMap> m_map;
+};
+
+
+static ObjectMap& objectMap()
+{
+ DEFINE_STATIC_LOCAL(ObjectMap, map, ());
+ return map;
+}
+
+void ObjectMap::RootObjectInvalidationCallback::operator()(RootObject* rootObject)
+{
+ objectMap().remove(rootObject);
+}
+
static void getListFromVariantArgs(ExecState* exec, const NPVariant* args, unsigned argCount, RootObject* rootObject, MarkedArgumentBuffer& aList)
{
for (unsigned i = 0; i < argCount; ++i)
@@ -66,8 +124,10 @@ static void jsDeallocate(NPObject* npObj)
{
JavaScriptObject* obj = reinterpret_cast<JavaScriptObject*>(npObj);
- if (obj->rootObject && obj->rootObject->isValid())
+ if (obj->rootObject && obj->rootObject->isValid()) {
+ objectMap().remove(obj->rootObject, obj->imp);
obj->rootObject->gcUnprotect(obj->imp);
+ }
if (obj->rootObject)
obj->rootObject->deref();
@@ -83,12 +143,18 @@ static NPClass* NPNoScriptObjectClass = &noScriptClass;
NPObject* _NPN_CreateScriptObject(NPP npp, JSObject* imp, PassRefPtr<RootObject> rootObject)
{
+ if (NPObject* object = objectMap().get(rootObject.get(), imp))
+ return _NPN_RetainObject(object);
+
JavaScriptObject* obj = reinterpret_cast<JavaScriptObject*>(_NPN_CreateObject(npp, NPScriptObjectClass));
obj->rootObject = rootObject.releaseRef();
- if (obj->rootObject)
+ if (obj->rootObject) {
obj->rootObject->gcProtect(imp);
+ objectMap().add(obj->rootObject, imp, reinterpret_cast<NPObject*>(obj));
+ }
+
obj->imp = imp;
return reinterpret_cast<NPObject*>(obj);
diff --git a/WebCore/bridge/runtime_root.cpp b/WebCore/bridge/runtime_root.cpp
index 143c3ad..b179d56 100644
--- a/WebCore/bridge/runtime_root.cpp
+++ b/WebCore/bridge/runtime_root.cpp
@@ -71,6 +71,10 @@ RootObject* findRootObject(JSGlobalObject* globalObject)
return 0;
}
+RootObject::InvalidationCallback::~InvalidationCallback()
+{
+}
+
PassRefPtr<RootObject> RootObject::create(const void* nativeHandle, JSGlobalObject* globalObject)
{
return adoptRef(new RootObject(nativeHandle, globalObject));
@@ -109,6 +113,14 @@ void RootObject::invalidate()
m_nativeHandle = 0;
m_globalObject = 0;
+ {
+ HashSet<InvalidationCallback*>::iterator end = m_invalidationCallbacks.end();
+ for (HashSet<InvalidationCallback*>::iterator iter = m_invalidationCallbacks.begin(); iter != end; ++iter)
+ (**iter)(this);
+
+ m_invalidationCallbacks.clear();
+ }
+
ProtectCountSet::iterator end = m_protectCountSet.end();
for (ProtectCountSet::iterator it = m_protectCountSet.begin(); it != end; ++it)
JSC::gcUnprotect(it->first);
diff --git a/WebCore/bridge/runtime_root.h b/WebCore/bridge/runtime_root.h
index fdd73c4..a81afb8 100644
--- a/WebCore/bridge/runtime_root.h
+++ b/WebCore/bridge/runtime_root.h
@@ -72,6 +72,13 @@ public:
void addRuntimeObject(RuntimeObjectImp*);
void removeRuntimeObject(RuntimeObjectImp*);
+
+ struct InvalidationCallback {
+ virtual void operator()(RootObject*) = 0;
+ virtual ~InvalidationCallback();
+ };
+ void addInvalidationCallback(InvalidationCallback* callback) { m_invalidationCallbacks.add(callback); }
+
private:
RootObject(const void* nativeHandle, JSGlobalObject*);
@@ -79,9 +86,11 @@ private:
const void* m_nativeHandle;
ProtectedPtr<JSGlobalObject> m_globalObject;
- ProtectCountSet m_protectCountSet;
+ ProtectCountSet m_protectCountSet;
HashSet<RuntimeObjectImp*> m_runtimeObjects;
+
+ HashSet<InvalidationCallback*> m_invalidationCallbacks;
};
} // namespace Bindings
diff --git a/WebKit/mac/ChangeLog b/WebKit/mac/ChangeLog
index 84b153e..41fbcc5 100644
--- a/WebKit/mac/ChangeLog
+++ b/WebKit/mac/ChangeLog
@@ -1,3 +1,70 @@
+2010-02-12 Alexey Proskuryakov <ap at apple.com>
+
+ Reviewed by Kevin Decker.
+
+ <rdar://problem/7130641> Browser objects identity is not preserved by Safari
+
+ Out of process part.
+
+ To avoid excessive IPC, plugin process doesn't send each NPObject release/retain call to
+ Safari. It only sends one when the last one is removed, and it can destroy the proxy
+ NPObject.
+
+ However, the browser may be sending the same object out to plug-in as a function call
+ argument at the same time - neither side can know what the other one is up to. The solution
+ is to make the "destroying object" call return a boolean result, making it possible for
+ the browser to make plugin host keep the proxy with zero refcount for a little longer.
+
+ * Plugins/Hosted/NetscapePluginHostProxy.mm:
+ (WKPCForgetBrowserObject): This function (that used to be named ReleaseObject) is only
+ called when plug-in releases all of its references, so renamed it. Its boolean result
+ is returned as call success or failure.
+
+ * Plugins/Hosted/NetscapePluginInstanceProxy.h:
+ (WebKit::NetscapePluginInstanceProxy::LocalObjectMap): Made the numeric ID to JSObject map
+ two-way.
+
+ * Plugins/Hosted/NetscapePluginInstanceProxy.mm:
+ (WebKit::NetscapePluginInstanceProxy::LocalObjectMap::idForObject): This method is tricky
+ in that it creates objects with refcount of 1, but doesn't increase refcount when returning
+ found objects. This extra count accounts for the "reference" kept by plugin process.
+ (WebKit::NetscapePluginInstanceProxy::LocalObjectMap::retain): Only retaining for the
+ duration of calls out to plug-in, which means that refcount is almost always equal to 1.
+ Note that we can't use "++" here, due to how std::pair works!
+ (WebKit::NetscapePluginInstanceProxy::LocalObjectMap::release): Ditto.
+ (WebKit::NetscapePluginInstanceProxy::LocalObjectMap::clear): Clear all references when
+ stopping plug-in.
+ (WebKit::NetscapePluginInstanceProxy::LocalObjectMap::forget): Like release(), but only
+ functional when recount is 1 (meaning that the object is not being sent out to plug-in at
+ the moment).
+ (WebKit::NetscapePluginInstanceProxy::NetscapePluginInstanceProxy): Updated for other changes.
+ (WebKit::NetscapePluginInstanceProxy::cleanup): Ditto.
+ (WebKit::NetscapePluginInstanceProxy::getWindowNPObject): Ditto.
+ (WebKit::NetscapePluginInstanceProxy::getPluginElementNPObject): Ditto.
+ (WebKit::NetscapePluginInstanceProxy::forgetBrowserObjectID): Ditto.
+ (WebKit::NetscapePluginInstanceProxy::evaluate): Ditto.
+ (WebKit::NetscapePluginInstanceProxy::invoke): Ditto.
+ (WebKit::NetscapePluginInstanceProxy::invokeDefault): Ditto.
+ (WebKit::NetscapePluginInstanceProxy::construct): Ditto.
+ (WebKit::NetscapePluginInstanceProxy::getProperty): Ditto.
+ (WebKit::NetscapePluginInstanceProxy::setProperty): Ditto.
+ (WebKit::NetscapePluginInstanceProxy::removeProperty): Ditto.
+ (WebKit::NetscapePluginInstanceProxy::hasProperty): Ditto.
+ (WebKit::NetscapePluginInstanceProxy::hasMethod): Ditto.
+ (WebKit::NetscapePluginInstanceProxy::enumerate): Ditto.
+ (WebKit::NetscapePluginInstanceProxy::addValueToArray): Ditto.
+ (WebKit::NetscapePluginInstanceProxy::demarshalValueFromArray): Ditto.
+ (WebKit::NetscapePluginInstanceProxy::retainLocalObject): Helper for calling retain when
+ making calls out to plug-in. No-op for objects that aren't wrapped to be sent (i.e. for
+ objects wrapping ProxyInstance wrappers for plug-in objects being sent bak).
+ (WebKit::NetscapePluginInstanceProxy::releaseLocalObject): Ditto.
+
+ * Plugins/Hosted/ProxyInstance.mm:
+ (WebKit::ProxyInstance::invoke): Retain/release arguments during call.
+ (WebKit::ProxyInstance::setFieldValue): Ditto.
+
+ * Plugins/Hosted/WebKitPluginClient.defs: Renamed PCReleaseObject to PCForgetBrowserObject.
+
2010-02-12 Darin Adler <darin at apple.com>
Reviewed by Sam Weinig.
diff --git a/WebKit/mac/Plugins/Hosted/NetscapePluginHostProxy.mm b/WebKit/mac/Plugins/Hosted/NetscapePluginHostProxy.mm
index c5beb07..836277c 100644
--- a/WebKit/mac/Plugins/Hosted/NetscapePluginHostProxy.mm
+++ b/WebKit/mac/Plugins/Hosted/NetscapePluginHostProxy.mm
@@ -546,7 +546,7 @@ kern_return_t WKPCGetPluginElementNPObject(mach_port_t clientPort, uint32_t plug
return KERN_SUCCESS;
}
-kern_return_t WKPCReleaseObject(mach_port_t clientPort, uint32_t pluginID, uint32_t objectID)
+kern_return_t WKPCForgetBrowserObject(mach_port_t clientPort, uint32_t pluginID, uint32_t objectID)
{
NetscapePluginHostProxy* hostProxy = pluginProxyMap().get(clientPort);
if (!hostProxy)
@@ -556,8 +556,7 @@ kern_return_t WKPCReleaseObject(mach_port_t clientPort, uint32_t pluginID, uint3
if (!instanceProxy)
return KERN_FAILURE;
- instanceProxy->releaseObject(objectID);
- return KERN_SUCCESS;
+ return instanceProxy->forgetBrowserObjectID(objectID) ? KERN_SUCCESS : KERN_FAILURE;
}
kern_return_t WKPCEvaluate(mach_port_t clientPort, uint32_t pluginID, uint32_t requestID, uint32_t objectID, data_t scriptData, mach_msg_type_number_t scriptLength, boolean_t allowPopups)
diff --git a/WebKit/mac/Plugins/Hosted/NetscapePluginInstanceProxy.h b/WebKit/mac/Plugins/Hosted/NetscapePluginInstanceProxy.h
index 76981a4..b782d42 100644
--- a/WebKit/mac/Plugins/Hosted/NetscapePluginInstanceProxy.h
+++ b/WebKit/mac/Plugins/Hosted/NetscapePluginInstanceProxy.h
@@ -111,8 +111,8 @@ public:
// NPRuntime
bool getWindowNPObject(uint32_t& objectID);
bool getPluginElementNPObject(uint32_t& objectID);
- void releaseObject(uint32_t objectID);
-
+ bool forgetBrowserObjectID(uint32_t objectID); // Will fail if the ID is being sent to plug-in right now (i.e., retain/release calls aren't balanced).
+
bool evaluate(uint32_t objectID, const WebCore::String& script, data_t& resultData, mach_msg_type_number_t& resultLength, bool allowPopups);
bool invoke(uint32_t objectID, const JSC::Identifier& methodName, data_t argumentsData, mach_msg_type_number_t argumentsLength, data_t& resultData, mach_msg_type_number_t& resultLength);
bool invokeDefault(uint32_t objectID, data_t argumentsData, mach_msg_type_number_t argumentsLength, data_t& resultData, mach_msg_type_number_t& resultLength);
@@ -143,9 +143,13 @@ public:
PassRefPtr<JSC::Bindings::Instance> createBindingsInstance(PassRefPtr<JSC::Bindings::RootObject>);
RetainPtr<NSData *> marshalValues(JSC::ExecState*, const JSC::ArgList& args);
- void marshalValue(JSC::ExecState*, JSC::JSValue value, data_t& resultData, mach_msg_type_number_t& resultLength);
+ void marshalValue(JSC::ExecState*, JSC::JSValue, data_t& resultData, mach_msg_type_number_t& resultLength);
JSC::JSValue demarshalValue(JSC::ExecState*, const char* valueData, mach_msg_type_number_t valueLength);
+ // No-op if the value does not contain a local object.
+ void retainLocalObject(JSC::JSValue);
+ void releaseLocalObject(JSC::JSValue);
+
void addInstance(ProxyInstance*);
void removeInstance(ProxyInstance*);
@@ -300,17 +304,33 @@ private:
HashMap<uint32_t, Reply*> m_replies;
// NPRuntime
- uint32_t idForObject(JSC::JSObject*);
-
+
void addValueToArray(NSMutableArray *, JSC::ExecState* exec, JSC::JSValue value);
bool demarshalValueFromArray(JSC::ExecState*, NSArray *array, NSUInteger& index, JSC::JSValue& result);
void demarshalValues(JSC::ExecState*, data_t valuesData, mach_msg_type_number_t valuesLength, JSC::MarkedArgumentBuffer& result);
- uint32_t m_objectIDCounter;
- typedef HashMap<uint32_t, JSC::ProtectedPtr<JSC::JSObject> > ObjectMap;
- ObjectMap m_objects;
-
+ class LocalObjectMap {
+ public:
+ LocalObjectMap();
+ uint32_t idForObject(JSC::JSObject*);
+ void retain(JSC::JSObject*);
+ void release(JSC::JSObject*);
+ void clear();
+ bool forget(uint32_t);
+ bool contains(uint32_t objectID) const { return m_idToJSObjectMap.contains(objectID); }
+ JSC::JSObject* get(uint32_t objectID) const { return m_idToJSObjectMap.get(objectID); }
+
+ private:
+ HashMap<uint32_t, JSC::ProtectedPtr<JSC::JSObject> > m_idToJSObjectMap;
+ // The pair consists of object ID and a reference count. One reference belongs to remote plug-in,
+ // and the proxy will add transient references for arguments that are being sent out.
+ HashMap<JSC::JSObject*, pair<uint32_t, uint32_t> > m_jsObjectToIDMap;
+ uint32_t m_objectIDCounter;
+ };
+
+ LocalObjectMap m_localObjects;
+
typedef HashSet<ProxyInstance*> ProxyInstanceSet;
ProxyInstanceSet m_instances;
diff --git a/WebKit/mac/Plugins/Hosted/NetscapePluginInstanceProxy.mm b/WebKit/mac/Plugins/Hosted/NetscapePluginInstanceProxy.mm
index c09e3ea..4354cb6 100644
--- a/WebKit/mac/Plugins/Hosted/NetscapePluginInstanceProxy.mm
+++ b/WebKit/mac/Plugins/Hosted/NetscapePluginInstanceProxy.mm
@@ -100,6 +100,84 @@ private:
bool m_allowPopups;
};
+NetscapePluginInstanceProxy::LocalObjectMap::LocalObjectMap()
+ : m_objectIDCounter(0)
+{
+}
+
+uint32_t NetscapePluginInstanceProxy::LocalObjectMap::idForObject(JSObject* object)
+{
+ // This method creates objects with refcount of 1, but doesn't increase refcount when returning
+ // found objects. This extra count accounts for the main "reference" kept by plugin process.
+
+ // To avoid excessive IPC, plugin process doesn't send each NPObject release/retain call to
+ // Safari. It only sends one when the last reference is removed, and it can destroy the proxy
+ // NPObject.
+
+ // However, the browser may be sending the same object out to plug-in as a function call
+ // argument at the same time - neither side can know what the other one is doing. So,
+ // is to make PCForgetBrowserObject call return a boolean result, making it possible for
+ // the browser to make plugin host keep the proxy with zero refcount for a little longer.
+
+ uint32_t objectID = 0;
+
+ HashMap<JSC::JSObject*, pair<uint32_t, uint32_t> >::iterator iter = m_jsObjectToIDMap.find(object);
+ if (iter != m_jsObjectToIDMap.end())
+ return iter->second.first;
+
+ do {
+ objectID = ++m_objectIDCounter;
+ } while (!m_objectIDCounter || m_objectIDCounter == static_cast<uint32_t>(-1) || m_idToJSObjectMap.contains(objectID));
+
+ m_idToJSObjectMap.set(objectID, object);
+ m_jsObjectToIDMap.set(object, make_pair<uint32_t, uint32_t>(objectID, 1));
+
+ return objectID;
+}
+
+void NetscapePluginInstanceProxy::LocalObjectMap::retain(JSC::JSObject* object)
+{
+ HashMap<JSC::JSObject*, pair<uint32_t, uint32_t> >::iterator iter = m_jsObjectToIDMap.find(object);
+ ASSERT(iter != m_jsObjectToIDMap.end());
+
+ iter->second.second = iter->second.second + 1;
+}
+
+void NetscapePluginInstanceProxy::LocalObjectMap::release(JSC::JSObject* object)
+{
+ HashMap<JSC::JSObject*, pair<uint32_t, uint32_t> >::iterator iter = m_jsObjectToIDMap.find(object);
+ ASSERT(iter != m_jsObjectToIDMap.end());
+
+ ASSERT(iter->second.second > 0);
+ iter->second.second = iter->second.second - 1;
+ if (!iter->second.second) {
+ m_idToJSObjectMap.remove(iter->second.first);
+ m_jsObjectToIDMap.remove(iter);
+ }
+}
+
+void NetscapePluginInstanceProxy::LocalObjectMap::clear()
+{
+ m_idToJSObjectMap.clear();
+ m_jsObjectToIDMap.clear();
+}
+
+bool NetscapePluginInstanceProxy::LocalObjectMap::forget(uint32_t objectID)
+{
+ HashMap<uint32_t, JSC::ProtectedPtr<JSC::JSObject> >::iterator iter = m_idToJSObjectMap.find(objectID);
+ ASSERT(iter != m_idToJSObjectMap.end());
+
+ HashMap<JSC::JSObject*, pair<uint32_t, uint32_t> >::iterator rIter = m_jsObjectToIDMap.find(iter->second.get());
+
+ // If the object is being sent to plug-in right now, then it's not the time to forget.
+ if (rIter->second.second != 1)
+ return false;
+
+ m_jsObjectToIDMap.remove(rIter);
+ m_idToJSObjectMap.remove(iter);
+ return true;
+}
+
static uint32_t pluginIDCounter;
#ifndef NDEBUG
@@ -114,7 +192,6 @@ NetscapePluginInstanceProxy::NetscapePluginInstanceProxy(NetscapePluginHostProxy
, m_renderContextID(0)
, m_useSoftwareRenderer(false)
, m_waitingForReply(false)
- , m_objectIDCounter(0)
, m_urlCheckCounter(0)
, m_pluginFunctionCallDepth(0)
, m_shouldStopSoon(false)
@@ -185,7 +262,7 @@ void NetscapePluginInstanceProxy::cleanup()
// Clear the object map, this will cause any outstanding JS objects that the plug-in had a reference to
// to go away when the next garbage collection takes place.
- m_objects.clear();
+ m_localObjects.clear();
if (Frame* frame = core([m_pluginView webFrame]))
frame->script()->cleanupScriptObjectsForPlugin(m_pluginView);
@@ -683,20 +760,6 @@ NetscapePluginInstanceProxy::Reply* NetscapePluginInstanceProxy::processRequests
return reply;
}
-uint32_t NetscapePluginInstanceProxy::idForObject(JSObject* object)
-{
- uint32_t objectID = 0;
-
- // Assign an object ID.
- do {
- objectID = ++m_objectIDCounter;
- } while (!m_objectIDCounter || m_objectIDCounter == static_cast<uint32_t>(-1) || m_objects.contains(objectID));
-
- m_objects.set(objectID, object);
-
- return objectID;
-}
-
// NPRuntime support
bool NetscapePluginInstanceProxy::getWindowNPObject(uint32_t& objectID)
{
@@ -707,7 +770,7 @@ bool NetscapePluginInstanceProxy::getWindowNPObject(uint32_t& objectID)
if (!frame->script()->canExecuteScripts())
objectID = 0;
else
- objectID = idForObject(frame->script()->windowShell(pluginWorld())->window());
+ objectID = m_localObjects.idForObject(frame->script()->windowShell(pluginWorld())->window());
return true;
}
@@ -719,16 +782,16 @@ bool NetscapePluginInstanceProxy::getPluginElementNPObject(uint32_t& objectID)
return false;
if (JSObject* object = frame->script()->jsObjectForPluginElement([m_pluginView element]))
- objectID = idForObject(object);
+ objectID = m_localObjects.idForObject(object);
else
objectID = 0;
return true;
}
-void NetscapePluginInstanceProxy::releaseObject(uint32_t objectID)
+bool NetscapePluginInstanceProxy::forgetBrowserObjectID(uint32_t objectID)
{
- m_objects.remove(objectID);
+ return m_localObjects.forget(objectID);
}
bool NetscapePluginInstanceProxy::evaluate(uint32_t objectID, const String& script, data_t& resultData, mach_msg_type_number_t& resultLength, bool allowPopups)
@@ -736,7 +799,7 @@ bool NetscapePluginInstanceProxy::evaluate(uint32_t objectID, const String& scri
resultData = 0;
resultLength = 0;
- if (!m_objects.contains(objectID))
+ if (!m_localObjects.contains(objectID))
return false;
Frame* frame = core([m_pluginView webFrame]);
@@ -778,7 +841,7 @@ bool NetscapePluginInstanceProxy::invoke(uint32_t objectID, const Identifier& me
if (m_inDestroy)
return false;
- JSObject* object = m_objects.get(objectID);
+ JSObject* object = m_localObjects.get(objectID);
if (!object)
return false;
@@ -812,7 +875,7 @@ bool NetscapePluginInstanceProxy::invokeDefault(uint32_t objectID, data_t argume
if (m_inDestroy)
return false;
- JSObject* object = m_objects.get(objectID);
+ JSObject* object = m_localObjects.get(objectID);
if (!object)
return false;
@@ -845,7 +908,7 @@ bool NetscapePluginInstanceProxy::construct(uint32_t objectID, data_t argumentsD
if (m_inDestroy)
return false;
- JSObject* object = m_objects.get(objectID);
+ JSObject* object = m_localObjects.get(objectID);
if (!object)
return false;
@@ -879,7 +942,7 @@ bool NetscapePluginInstanceProxy::getProperty(uint32_t objectID, const Identifie
if (m_inDestroy)
return false;
- JSObject* object = m_objects.get(objectID);
+ JSObject* object = m_localObjects.get(objectID);
if (!object)
return false;
@@ -898,7 +961,7 @@ bool NetscapePluginInstanceProxy::getProperty(uint32_t objectID, const Identifie
bool NetscapePluginInstanceProxy::getProperty(uint32_t objectID, unsigned propertyName, data_t& resultData, mach_msg_type_number_t& resultLength)
{
- JSObject* object = m_objects.get(objectID);
+ JSObject* object = m_localObjects.get(objectID);
if (!object)
return false;
@@ -920,7 +983,7 @@ bool NetscapePluginInstanceProxy::setProperty(uint32_t objectID, const Identifie
if (m_inDestroy)
return false;
- JSObject* object = m_objects.get(objectID);
+ JSObject* object = m_localObjects.get(objectID);
if (!object)
return false;
@@ -944,7 +1007,7 @@ bool NetscapePluginInstanceProxy::setProperty(uint32_t objectID, unsigned proper
if (m_inDestroy)
return false;
- JSObject* object = m_objects.get(objectID);
+ JSObject* object = m_localObjects.get(objectID);
if (!object)
return false;
@@ -967,7 +1030,7 @@ bool NetscapePluginInstanceProxy::removeProperty(uint32_t objectID, const Identi
if (m_inDestroy)
return false;
- JSObject* object = m_objects.get(objectID);
+ JSObject* object = m_localObjects.get(objectID);
if (!object)
return false;
@@ -992,7 +1055,7 @@ bool NetscapePluginInstanceProxy::removeProperty(uint32_t objectID, unsigned pro
if (m_inDestroy)
return false;
- JSObject* object = m_objects.get(objectID);
+ JSObject* object = m_localObjects.get(objectID);
if (!object)
return false;
@@ -1017,7 +1080,7 @@ bool NetscapePluginInstanceProxy::hasProperty(uint32_t objectID, const Identifie
if (m_inDestroy)
return false;
- JSObject* object = m_objects.get(objectID);
+ JSObject* object = m_localObjects.get(objectID);
if (!object)
return false;
@@ -1037,7 +1100,7 @@ bool NetscapePluginInstanceProxy::hasProperty(uint32_t objectID, unsigned proper
if (m_inDestroy)
return false;
- JSObject* object = m_objects.get(objectID);
+ JSObject* object = m_localObjects.get(objectID);
if (!object)
return false;
@@ -1057,7 +1120,7 @@ bool NetscapePluginInstanceProxy::hasMethod(uint32_t objectID, const Identifier&
if (m_inDestroy)
return false;
- JSObject* object = m_objects.get(objectID);
+ JSObject* object = m_localObjects.get(objectID);
if (!object)
return false;
@@ -1077,7 +1140,7 @@ bool NetscapePluginInstanceProxy::enumerate(uint32_t objectID, data_t& resultDat
if (m_inDestroy)
return false;
- JSObject* object = m_objects.get(objectID);
+ JSObject* object = m_localObjects.get(objectID);
if (!object)
return false;
@@ -1136,7 +1199,7 @@ void NetscapePluginInstanceProxy::addValueToArray(NSMutableArray *array, ExecSta
}
} else {
[array addObject:[NSNumber numberWithInt:JSObjectValueType]];
- [array addObject:[NSNumber numberWithInt:idForObject(object)]];
+ [array addObject:[NSNumber numberWithInt:m_localObjects.idForObject(object)]];
}
} else
[array addObject:[NSNumber numberWithInt:VoidValueType]];
@@ -1198,7 +1261,7 @@ bool NetscapePluginInstanceProxy::demarshalValueFromArray(ExecState* exec, NSArr
case JSObjectValueType: {
uint32_t objectID = [[array objectAtIndex:index++] intValue];
- result = m_objects.get(objectID);
+ result = m_localObjects.get(objectID);
ASSERT(result);
return true;
}
@@ -1255,6 +1318,30 @@ void NetscapePluginInstanceProxy::demarshalValues(ExecState* exec, data_t values
result.append(value);
}
+void NetscapePluginInstanceProxy::retainLocalObject(JSC::JSValue value)
+{
+ if (!value.isObject())
+ return;
+
+ JSObject* object = asObject(value);
+ if (object->classInfo() == &RuntimeObjectImp::s_info)
+ return;
+
+ m_localObjects.retain(object);
+}
+
+void NetscapePluginInstanceProxy::releaseLocalObject(JSC::JSValue value)
+{
+ if (!value.isObject())
+ return;
+
+ JSObject* object = asObject(value);
+ if (object->classInfo() == &RuntimeObjectImp::s_info)
+ return;
+
+ m_localObjects.release(object);
+}
+
PassRefPtr<Instance> NetscapePluginInstanceProxy::createBindingsInstance(PassRefPtr<RootObject> rootObject)
{
uint32_t requestID = nextRequestID();
diff --git a/WebKit/mac/Plugins/Hosted/ProxyInstance.mm b/WebKit/mac/Plugins/Hosted/ProxyInstance.mm
index 1587ad0..c7a0ebe 100644
--- a/WebKit/mac/Plugins/Hosted/ProxyInstance.mm
+++ b/WebKit/mac/Plugins/Hosted/ProxyInstance.mm
@@ -137,17 +137,27 @@ JSValue ProxyInstance::invoke(JSC::ExecState* exec, InvokeType type, uint64_t id
{
if (!m_instanceProxy)
return jsUndefined();
-
+
RetainPtr<NSData*> arguments(m_instanceProxy->marshalValues(exec, args));
uint32_t requestID = m_instanceProxy->nextRequestID();
+ for (unsigned i = 0; i < args.size(); i++)
+ m_instanceProxy->retainLocalObject(args.at(i));
+
if (_WKPHNPObjectInvoke(m_instanceProxy->hostProxy()->port(), m_instanceProxy->pluginID(), requestID, m_objectID,
- type, identifier, (char*)[arguments.get() bytes], [arguments.get() length]) != KERN_SUCCESS)
+ type, identifier, (char*)[arguments.get() bytes], [arguments.get() length]) != KERN_SUCCESS) {
+ for (unsigned i = 0; i < args.size(); i++)
+ m_instanceProxy->releaseLocalObject(args.at(i));
return jsUndefined();
+ }
auto_ptr<NetscapePluginInstanceProxy::BooleanAndDataReply> reply = waitForReply<NetscapePluginInstanceProxy::BooleanAndDataReply>(requestID);
NetscapePluginInstanceProxy::moveGlobalExceptionToExecState(exec);
+
+ for (unsigned i = 0; i < args.size(); i++)
+ m_instanceProxy->releaseLocalObject(args.at(i));
+
if (!reply.get() || !reply->m_returnValue)
return jsUndefined();
@@ -381,10 +391,12 @@ void ProxyInstance::setFieldValue(ExecState* exec, const Field* field, JSValue v
mach_msg_type_number_t valueLength;
m_instanceProxy->marshalValue(exec, value, valueData, valueLength);
+ m_instanceProxy->retainLocalObject(value);
kern_return_t kr = _WKPHNPObjectSetProperty(m_instanceProxy->hostProxy()->port(),
m_instanceProxy->pluginID(), requestID,
m_objectID, serverIdentifier, valueData, valueLength);
mig_deallocate(reinterpret_cast<vm_address_t>(valueData), valueLength);
+ m_instanceProxy->releaseLocalObject(value);
if (kr != KERN_SUCCESS)
return;
diff --git a/WebKit/mac/Plugins/Hosted/WebKitPluginClient.defs b/WebKit/mac/Plugins/Hosted/WebKitPluginClient.defs
index 58a7996..6522bf7 100644
--- a/WebKit/mac/Plugins/Hosted/WebKitPluginClient.defs
+++ b/WebKit/mac/Plugins/Hosted/WebKitPluginClient.defs
@@ -112,7 +112,7 @@ routine PCGetPluginElementNPObject(clientPort :mach_port_t;
pluginID :uint32_t;
out objectID :uint32_t);
-routine PCReleaseObject(clientPort :mach_port_t;
+routine PCForgetBrowserObject(clientPort :mach_port_t;
pluginID :uint32_t;
objectID :uint32_t);
diff --git a/WebKitTools/ChangeLog b/WebKitTools/ChangeLog
index 2d88397..6ab1d62 100644
--- a/WebKitTools/ChangeLog
+++ b/WebKitTools/ChangeLog
@@ -1,3 +1,12 @@
+2010-02-12 Alexey Proskuryakov <ap at apple.com>
+
+ Reviewed by Kevin Decker.
+
+ <rdar://problem/7130641> Browser objects identity is not preserved by Safari
+
+ * DumpRenderTree/TestNetscapePlugIn.subproj/PluginObject.cpp:
+ (pluginInvoke): Added methods for checking object identity (via refcount).
+
2010-02-15 Robert Hogan <robert at roberthogan.net>
Reviewed by Simon Hausmann.
diff --git a/WebKitTools/DumpRenderTree/TestNetscapePlugIn.subproj/PluginObject.cpp b/WebKitTools/DumpRenderTree/TestNetscapePlugIn.subproj/PluginObject.cpp
index e69da73..1616d3e 100644
--- a/WebKitTools/DumpRenderTree/TestNetscapePlugIn.subproj/PluginObject.cpp
+++ b/WebKitTools/DumpRenderTree/TestNetscapePlugIn.subproj/PluginObject.cpp
@@ -176,6 +176,10 @@ enum {
ID_TEST_RELOAD_PLUGINS_AND_PAGES,
ID_TEST_GET_BROWSER_PROPERTY,
ID_TEST_SET_BROWSER_PROPERTY,
+ ID_REMEMBER,
+ ID_GET_REMEMBERED_OBJECT,
+ ID_GET_AND_FORGET_REMEMBERED_OBJECT,
+ ID_REF_COUNT,
NUM_METHOD_IDENTIFIERS
};
@@ -205,7 +209,11 @@ static const NPUTF8 *pluginMethodIdentifierNames[NUM_METHOD_IDENTIFIERS] = {
"reloadPluginsNoPages",
"reloadPluginsAndPages",
"testGetBrowserProperty",
- "testSetBrowserProperty"
+ "testSetBrowserProperty",
+ "remember",
+ "getRememberedObject",
+ "getAndForgetRememberedObject",
+ "refCount"
};
static NPUTF8* createCStringFromNPVariant(const NPVariant* variant)
@@ -747,6 +755,8 @@ bool testWindowOpen(NPP npp)
return false;
}
+static NPObject* rememberedObject;
+
static bool pluginInvoke(NPObject* header, NPIdentifier name, const NPVariant* args, uint32_t argCount, NPVariant* result)
{
PluginObject* plugin = reinterpret_cast<PluginObject*>(header);
@@ -807,6 +817,27 @@ static bool pluginInvoke(NPObject* header, NPIdentifier name, const NPVariant* a
} else if (name == pluginMethodIdentifiers[ID_TEST_SET_BROWSER_PROPERTY]) {
browser->setproperty(plugin->npp, NPVARIANT_TO_OBJECT(args[0]), stringVariantToIdentifier(args[1]), &args[2]);
return true;
+ } else if (name == pluginMethodIdentifiers[ID_REMEMBER]) {
+ if (rememberedObject)
+ browser->releaseobject(rememberedObject);
+ rememberedObject = NPVARIANT_TO_OBJECT(args[0]);
+ browser->retainobject(rememberedObject);
+ VOID_TO_NPVARIANT(*result);
+ return true;
+ } else if (name == pluginMethodIdentifiers[ID_GET_REMEMBERED_OBJECT]) {
+ assert(rememberedObject);
+ browser->retainobject(rememberedObject);
+ OBJECT_TO_NPVARIANT(rememberedObject, *result);
+ return true;
+ } else if (name == pluginMethodIdentifiers[ID_GET_AND_FORGET_REMEMBERED_OBJECT]) {
+ assert(rememberedObject);
+ OBJECT_TO_NPVARIANT(rememberedObject, *result);
+ rememberedObject = 0;
+ return true;
+ } else if (name == pluginMethodIdentifiers[ID_REF_COUNT]) {
+ uint32_t refCount = NPVARIANT_TO_OBJECT(args[0])->referenceCount;
+ INT32_TO_NPVARIANT(refCount, *result);
+ return true;
}
return false;
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list