Changeset 73414 in webkit

Timestamp:

Dec 6, 2010 4:57:32 PM (13 years ago)

Author:

andersca@apple.com

Message:

WebProcess crash in NPRemoteObjectMap::invalidate when closing tab
​https://bugs.webkit.org/show_bug.cgi?id=50597
<rdar://problem/8655584>

Reviewed by Sam Weinig.

When invalidating the NPRemoteObjectMap, we don't want NPObjectMessageReceiver to
release all objects NPObjects blindly because NPJSObjects have already been deallocated by the plug-in view.

This is not an ideal solution; an ideal solution would involve NPJSObjects notifying any NPObjectMessageReceiver objects
that the NPJSObject is being destroyed. The NPObjectMessageReceiver could then simply null out the NPObject pointer.

• Shared/Plugins/NPObjectMessageReceiver.cpp:

(WebKit::NPObjectMessageReceiver::NPObjectMessageReceiver):
(WebKit::NPObjectMessageReceiver::~NPObjectMessageReceiver):

• Shared/Plugins/NPObjectMessageReceiver.h:
• Shared/Plugins/NPRemoteObjectMap.cpp:

(WebKit::NPRemoteObjectMap::NPRemoteObjectMap):
(WebKit::NPRemoteObjectMap::invalidate):

• Shared/Plugins/NPRemoteObjectMap.h:

(WebKit::NPRemoteObjectMap::isInvalidating):

Location:

trunk/WebKit2

Files:

• ChangeLog (modified) (1 diff)
• Shared/Plugins/NPObjectMessageReceiver.cpp (modified) (3 diffs)
• Shared/Plugins/NPObjectMessageReceiver.h (modified) (1 diff)
• Shared/Plugins/NPRemoteObjectMap.cpp (modified) (3 diffs)
• Shared/Plugins/NPRemoteObjectMap.h (modified) (2 diffs)

trunk/WebKit2/ChangeLog

@@ +1 @@
+2010-12-06  Anders Carlsson  <andersca@apple.com>
+
+        Reviewed by Sam Weinig.
+
+        WebProcess crash in NPRemoteObjectMap::invalidate when closing tab
+        https://bugs.webkit.org/show_bug.cgi?id=50597
+        <rdar://problem/8655584>
+
+        When invalidating the NPRemoteObjectMap, we don't want NPObjectMessageReceiver to
+        release all objects NPObjects blindly because NPJSObjects have already been deallocated by the plug-in view.
+
+        This is not an ideal solution; an ideal solution would involve NPJSObjects notifying any NPObjectMessageReceiver objects
+        that the NPJSObject is being destroyed. The NPObjectMessageReceiver could then simply null out the NPObject pointer.
+
+        * Shared/Plugins/NPObjectMessageReceiver.cpp:
+        (WebKit::NPObjectMessageReceiver::NPObjectMessageReceiver):
+        (WebKit::NPObjectMessageReceiver::~NPObjectMessageReceiver):
+        * Shared/Plugins/NPObjectMessageReceiver.h:
+        * Shared/Plugins/NPRemoteObjectMap.cpp:
+        (WebKit::NPRemoteObjectMap::NPRemoteObjectMap):
+        (WebKit::NPRemoteObjectMap::invalidate):
+        * Shared/Plugins/NPRemoteObjectMap.h:
+        (WebKit::NPRemoteObjectMap::isInvalidating):
+
 2010-12-06  Sam Weinig  <sam@webkit.org>
 

trunk/WebKit2/Shared/Plugins/NPObjectMessageReceiver.cpp

@@ -33 +33 @@
 #include "NPVariantData.h"
 
+// FIXME: This code shouldn't know about NPJSObject.
+#include "NPJSObject.h"
+
 namespace WebKit {
 
@@ -44 +47 @@
     , m_npObjectID(npObjectID)
     , m_npObject(npObject)
+    , m_shouldReleaseObjectWhenInvalidating(!NPJSObject::isNPJSObject(npObject))
 {
     retainNPObject(m_npObject);
@@ -51 +55 @@
 {
     m_npRemoteObjectMap->unregisterNPObject(m_npObjectID);
+
+    // If we're invalidating the remote object map, we don't always want to release the underlying NPObject.
+    // One example of this is NPJSObjects in the Web process, which have already been deallocated by the plug-in view.
+    // FIXME: This is not the ideal way to handle this. Maybe NPObjectMessageReceiver should be notified somehow when the underlying
+    // NPObject is deallocated.
+    if (m_npRemoteObjectMap->isInvalidating() && !m_shouldReleaseObjectWhenInvalidating)
+        return;
 
     releaseNPObject(m_npObject);

trunk/WebKit2/Shared/Plugins/NPObjectMessageReceiver.h

@@ -69 +69 @@
     uint64_t m_npObjectID;
     NPObject* m_npObject;
+    bool m_shouldReleaseObjectWhenInvalidating;
 };
     

trunk/WebKit2/Shared/Plugins/NPRemoteObjectMap.cpp

@@ -50 +50 @@
 NPRemoteObjectMap::NPRemoteObjectMap(CoreIPC::Connection* connection)
     : m_connection(connection)
+    , m_isInvalidating(false)
 {
 }
@@ -188 +189 @@
 void NPRemoteObjectMap::invalidate()
 {
+    ASSERT(!m_isInvalidating);
+
+    m_isInvalidating = true;
+
     Vector<NPObjectMessageReceiver*> messageReceivers;
     copyValuesToVector(m_registeredNPObjects, messageReceivers);
@@ -199 +204 @@
         NPObjectProxy::toNPObjectProxy(*it)->invalidate();
     m_npObjectProxies.clear();
+
+    m_isInvalidating = false;
 }
 

trunk/WebKit2/Shared/Plugins/NPRemoteObjectMap.h

@@ -61 +61 @@
 
     CoreIPC::Connection* connection() const { return m_connection; }
+    bool isInvalidating() const { return m_isInvalidating; }
 
     void invalidate();
@@ -69 +70 @@
     explicit NPRemoteObjectMap(CoreIPC::Connection*);
     CoreIPC::Connection* m_connection;
+
+    bool m_isInvalidating;
 
     // A map of NPObjectMessageReceiver classes, wrapping objects that we export to the