2009-11-25 Drew Wilson <atwilson@chromium.org>
authoreric@webkit.org <eric@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 25 Nov 2009 20:04:19 +0000 (20:04 +0000)
committereric@webkit.org <eric@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 25 Nov 2009 20:04:19 +0000 (20:04 +0000)
        Reviewed by David Levin.

        MessagePorts always look remotely entangled even when closed.
        https://bugs.webkit.org/show_bug.cgi?id=31698

        Tests: Existing tests suffice, Chromium soak test passes now.

        * bindings/v8/custom/V8CustomBinding.h:
        Removed kMessagePortEntangledPortIndex which is no longer used.
        * bindings/v8/V8GCController.cpp:
        (WebCore::GCPrologueVisitor::visitDOMWrapper):
        Simplified GC code to reflect the Chromium MessagePort implementation
        (locallyEntangledPort() always returns false).
        (WebCore::GCEpilogueVisitor::visitDOMWrapper):
        Cleaned up epilogue code to handle the case where the port gets closed
        in mid-GC (due to the parent context being freed).
        * dom/MessagePort.cpp:
        (WebCore::MessagePort::MessagePort):
        (WebCore::MessagePort::close):
        Now sets the closed flag.
        (WebCore::MessagePort::disentanglePorts):
        Updated to use new isCloned() API instead of relying on isEntangled(), which was incorrect.
        * dom/MessagePort.h:
        Added a m_closed flag and updated isEntangled() to check it.
        (WebCore::MessagePort::isEntangled):
        Now returns false if the port has been closed.
        (WebCore::MessagePort::isCloned):
        Added new API to differentiate between cloned and closed ports (closed ports can still be passed to postMessage).

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

WebCore/ChangeLog
WebCore/bindings/v8/V8GCController.cpp
WebCore/bindings/v8/custom/V8CustomBinding.h
WebCore/dom/MessagePort.cpp
WebCore/dom/MessagePort.h

index f8c2e7e702e29bd9467b1241391365729fcc81ad..46455887035e7b0c9fb9ba5e831954ffc6cb595b 100644 (file)
@@ -1,3 +1,34 @@
+2009-11-25  Drew Wilson  <atwilson@chromium.org>
+
+        Reviewed by David Levin.
+
+        MessagePorts always look remotely entangled even when closed.
+        https://bugs.webkit.org/show_bug.cgi?id=31698
+
+        Tests: Existing tests suffice, Chromium soak test passes now.
+
+        * bindings/v8/custom/V8CustomBinding.h:
+        Removed kMessagePortEntangledPortIndex which is no longer used.
+        * bindings/v8/V8GCController.cpp:
+        (WebCore::GCPrologueVisitor::visitDOMWrapper):
+        Simplified GC code to reflect the Chromium MessagePort implementation
+        (locallyEntangledPort() always returns false).
+        (WebCore::GCEpilogueVisitor::visitDOMWrapper):
+        Cleaned up epilogue code to handle the case where the port gets closed
+        in mid-GC (due to the parent context being freed).
+        * dom/MessagePort.cpp:
+        (WebCore::MessagePort::MessagePort):
+        (WebCore::MessagePort::close):
+        Now sets the closed flag.
+        (WebCore::MessagePort::disentanglePorts):
+        Updated to use new isCloned() API instead of relying on isEntangled(), which was incorrect.
+        * dom/MessagePort.h:
+        Added a m_closed flag and updated isEntangled() to check it.
+        (WebCore::MessagePort::isEntangled):
+        Now returns false if the port has been closed.
+        (WebCore::MessagePort::isCloned):
+        Added new API to differentiate between cloned and closed ports (closed ports can still be passed to postMessage).
+
 2009-11-25  Jocelyn Turcotte  <jocelyn.turcotte@nokia.com>
 
         Reviewed by Simon Hausmann.
index bd545bbf7638861e17cceabf6c6efdf9356abbb8..b71b11d05eeac3efaae4c5044ca1fa3a0f790974 100644 (file)
@@ -204,35 +204,11 @@ public:
     // has the drawback that the wrappers are "entangled/unentangled" for each
     // GC even though their entaglement most likely is still the same.
     if (type == V8ClassIndex::MESSAGEPORT) {
-        // Get the port and its entangled port.
+        // Mark each port as in-use if it's entangled. For simplicity's sake, we assume all ports are remotely entangled,
+        // since the Chromium port implementation can't tell the difference.
         MessagePort* port1 = static_cast<MessagePort*>(object);
-        MessagePort* port2 = port1->locallyEntangledPort();
-
-        // If we are remotely entangled, then mark this object as reachable
-        // (we can't determine reachability directly as the remote object is
-        // out-of-proc).
-        if (port1->isEntangled() && !port2)
+        if (port1->isEntangled())
             wrapper.ClearWeak();
-
-        if (port2) {
-            // As ports are always entangled in pairs only perform the entanglement
-            // once for each pair (see ASSERT in MessagePort::unentangle()).
-            if (port1 < port2) {
-                v8::Handle<v8::Value> port1Wrapper = V8DOMWrapper::jsWrapperForActiveDOMObject(port1);
-                v8::Handle<v8::Value> port2Wrapper = V8DOMWrapper::jsWrapperForActiveDOMObject(port2);
-                ASSERT(port1Wrapper->IsObject());
-                v8::Handle<v8::Object>::Cast(port1Wrapper)->SetInternalField(V8Custom::kMessagePortEntangledPortIndex, port2Wrapper);
-                ASSERT(port2Wrapper->IsObject());
-                v8::Handle<v8::Object>::Cast(port2Wrapper)->SetInternalField(V8Custom::kMessagePortEntangledPortIndex, port1Wrapper);
-            }
-        } else {
-            // Remove the wrapper entanglement when a port is not entangled.
-            if (V8DOMWrapper::domObjectHasJSWrapper(port1)) {
-                v8::Handle<v8::Value> wrapper = V8DOMWrapper::jsWrapperForActiveDOMObject(port1);
-                ASSERT(wrapper->IsObject());
-                v8::Handle<v8::Object>::Cast(wrapper)->SetInternalField(V8Custom::kMessagePortEntangledPortIndex, v8::Undefined());
-            }
-        }
     }
 }
 };
@@ -405,13 +381,11 @@ ACTIVE_DOM_OBJECT_TYPES(MAKE_CASE)
 
         if (type == V8ClassIndex::MESSAGEPORT) {
             MessagePort* port1 = static_cast<MessagePort*>(object);
-            MessagePort* port2 = port1->locallyEntangledPort();
-            if (port1->isEntangled() && !port2) {
-                // We marked this port as reachable in GCPrologueVisitor.  Undo this now since the
-                // port could be not reachable in the future if it gets disentangled (and also
-                // GCPrologueVisitor expects to see all handles marked as weak).
+            // We marked this port as reachable in GCPrologueVisitor.  Undo this now since the
+            // port could be not reachable in the future if it gets disentangled (and also
+            // GCPrologueVisitor expects to see all handles marked as weak).
+            if (!wrapper.IsWeak() && !wrapper.IsNearDeath())
                 wrapper.MakeWeak(port1, &DOMDataStore::weakActiveDOMObjectCallback);
-            }
         }
     }
 };
index a8fd96c6d8326f07105c30c281ca39b7829ca130..7a220eb86920bab99d4afc9c05891be93cbb18b1 100644 (file)
@@ -120,8 +120,7 @@ namespace WebCore {
         static const int kMessageChannelInternalFieldCount = kDefaultWrapperInternalFieldCount + 2;
 
         static const int kMessagePortRequestCacheIndex = kDefaultWrapperInternalFieldCount + 0;
-        static const int kMessagePortEntangledPortIndex = kDefaultWrapperInternalFieldCount + 1;
-        static const int kMessagePortInternalFieldCount = kDefaultWrapperInternalFieldCount + 2;
+        static const int kMessagePortInternalFieldCount = kDefaultWrapperInternalFieldCount + 1;
 
 #if ENABLE(WORKERS)
         static const int kAbstractWorkerRequestCacheIndex = kDefaultWrapperInternalFieldCount + 0;
index 9f6e6499ccff3f201454a0e5efa9864b07d4e0d7..10519205d3b94c361dcbf46d174ef8add5532294 100644 (file)
@@ -41,6 +41,7 @@ namespace WebCore {
 MessagePort::MessagePort(ScriptExecutionContext& scriptExecutionContext)
     : m_entangledChannel(0)
     , m_started(false)
+    , m_closed(false)
     , m_scriptExecutionContext(&scriptExecutionContext)
 {
     m_scriptExecutionContext->createdMessagePort(this);
@@ -131,6 +132,7 @@ void MessagePort::start()
 
 void MessagePort::close()
 {
+    m_closed = true;
     if (!m_entangledChannel)
         return;
     m_entangledChannel->close();
@@ -200,7 +202,7 @@ PassOwnPtr<MessagePortChannelArray> MessagePort::disentanglePorts(const MessageP
     // Walk the incoming array - if there are any duplicate ports, or null ports or cloned ports, throw an error (per section 8.3.3 of the HTML5 spec).
     for (unsigned int i = 0; i < ports->size(); ++i) {
         MessagePort* port = (*ports)[i].get();
-        if (!port || !port->isEntangled() || portSet.contains(port)) {
+        if (!port || port->isCloned() || portSet.contains(port)) {
             ec = INVALID_STATE_ERR;
             return 0;
         }
index 0ab0f505a4d6ede1df18ee48d9ad10cde16912ae..ae1eb22b125456ca79385238c66021f220a5a6da 100644 (file)
@@ -103,7 +103,10 @@ namespace WebCore {
         // Returns null otherwise.
         // NOTE: This is used solely to enable a GC optimization. Some platforms may not be able to determine ownership of the remote port (since it may live cross-process) - those platforms may always return null.
         MessagePort* locallyEntangledPort();
-        bool isEntangled() { return m_entangledChannel; }
+        // A port starts out its life entangled, and remains entangled until it is closed or is cloned.
+        bool isEntangled() { return !m_closed && !isCloned(); }
+        // A port is cloned if its entangled channel has been removed and sent to a new owner via postMessage().
+        bool isCloned() { return !m_entangledChannel; }
 
     private:
         MessagePort(ScriptExecutionContext&);
@@ -116,6 +119,7 @@ namespace WebCore {
         OwnPtr<MessagePortChannel> m_entangledChannel;
 
         bool m_started;
+        bool m_closed;
 
         ScriptExecutionContext* m_scriptExecutionContext;
         EventTargetData m_eventTargetData;