Reduce amount of platform specific code in MessagePortChannel
authorap@apple.com <ap@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 15 Mar 2013 23:40:32 +0000 (23:40 +0000)
committerap@apple.com <ap@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 15 Mar 2013 23:40:32 +0000 (23:40 +0000)
        https://bugs.webkit.org/show_bug.cgi?id=112469

        Reviewed by Sam Weinig.

        * dom/MessagePortChannel.h: Ifdefed out an unused channel() function, except for
        Chromium, where it is used. We certainly don't want to expose this implementation
        detail as a public function.

        * dom/default/PlatformMessagePortChannel.h: Removed functions that are no longer
        delegated. Made class contents all public, as it's now basically a data storage
        for MessagePortChannel.

        * dom/default/PlatformMessagePortChannel.cpp:
        (WebCore::MessagePortChannel::createChannel):
        (WebCore::MessagePortChannel::~MessagePortChannel):
        (WebCore::MessagePortChannel::entangleIfOpen):
        (WebCore::MessagePortChannel::disentangle):
        (WebCore::MessagePortChannel::postMessageToRemote):
        (WebCore::MessagePortChannel::tryGetMessageFromRemote):
        (WebCore::MessagePortChannel::close):
        (WebCore::MessagePortChannel::isConnectedTo):
        (WebCore::MessagePortChannel::hasPendingActivity):
        (WebCore::MessagePortChannel::locallyEntangledPort):
        (WebCore::PlatformMessagePortChannel::PlatformMessagePortChannel):
        (WebCore::PlatformMessagePortChannel::entangledChannel):
        Moved code from PlatformMessagePortChannel to MessagePortChannel. Added some
        FIXMEs.

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

Source/WebCore/ChangeLog
Source/WebCore/dom/MessagePortChannel.h
Source/WebCore/dom/default/PlatformMessagePortChannel.cpp
Source/WebCore/dom/default/PlatformMessagePortChannel.h

index 391cf1f..3141a00 100644 (file)
@@ -1,3 +1,34 @@
+2013-03-15  Alexey Proskuryakov  <ap@apple.com>
+
+        Reduce amount of platform specific code in MessagePortChannel
+        https://bugs.webkit.org/show_bug.cgi?id=112469
+
+        Reviewed by Sam Weinig.
+
+        * dom/MessagePortChannel.h: Ifdefed out an unused channel() function, except for
+        Chromium, where it is used. We certainly don't want to expose this implementation
+        detail as a public function.
+
+        * dom/default/PlatformMessagePortChannel.h: Removed functions that are no longer
+        delegated. Made class contents all public, as it's now basically a data storage
+        for MessagePortChannel.
+
+        * dom/default/PlatformMessagePortChannel.cpp:
+        (WebCore::MessagePortChannel::createChannel):
+        (WebCore::MessagePortChannel::~MessagePortChannel):
+        (WebCore::MessagePortChannel::entangleIfOpen):
+        (WebCore::MessagePortChannel::disentangle):
+        (WebCore::MessagePortChannel::postMessageToRemote):
+        (WebCore::MessagePortChannel::tryGetMessageFromRemote):
+        (WebCore::MessagePortChannel::close):
+        (WebCore::MessagePortChannel::isConnectedTo):
+        (WebCore::MessagePortChannel::hasPendingActivity):
+        (WebCore::MessagePortChannel::locallyEntangledPort):
+        (WebCore::PlatformMessagePortChannel::PlatformMessagePortChannel):
+        (WebCore::PlatformMessagePortChannel::entangledChannel):
+        Moved code from PlatformMessagePortChannel to MessagePortChannel. Added some
+        FIXMEs.
+
 2013-03-15  Arvid Nilsson  <anilsson@rim.com>
 
         [BlackBerry] Expose the compositing thread layer's draw rectangle to aid hit testing
index f6d97be..44f577e 100644 (file)
@@ -102,7 +102,10 @@ namespace WebCore {
 
         ~MessagePortChannel();
 
+#if PLATFORM(CHROMIUM)
+        // FIXME: PlatformMessagePortChannel is an implementation detail, and should not be exposed via a public function.
         PlatformMessagePortChannel* channel() const { return m_channel.get(); }
+#endif
 
     private:
         explicit MessagePortChannel(PassRefPtr<PlatformMessagePortChannel>);
index 20ffec4..fd065fc 100644 (file)
@@ -1,5 +1,6 @@
 /*
  * Copyright (C) 2009 Google Inc. All rights reserved.
+ * Copyright (C) 2013 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
 
 namespace WebCore {
 
-// MessagePortChannel implementations - just delegate to the PlatformMessagePortChannel.
 void MessagePortChannel::createChannel(PassRefPtr<MessagePort> port1, PassRefPtr<MessagePort> port2)
 {
-    PlatformMessagePortChannel::createChannel(port1, port2);
+    RefPtr<PlatformMessagePortChannel::MessagePortQueue> queue1 = PlatformMessagePortChannel::MessagePortQueue::create();
+    RefPtr<PlatformMessagePortChannel::MessagePortQueue> queue2 = PlatformMessagePortChannel::MessagePortQueue::create();
+
+    // Create proxies for each endpoint.
+    RefPtr<PlatformMessagePortChannel> channel1 = PlatformMessagePortChannel::create(queue1, queue2);
+    RefPtr<PlatformMessagePortChannel> channel2 = PlatformMessagePortChannel::create(queue2, queue1);
+
+    // Entangle the two endpoints.
+    channel1->m_entangledChannel = channel2;
+    channel2->m_entangledChannel = channel1;
+
+    // Now entangle the proxies with the appropriate local ports.
+    port1->entangle(MessagePortChannel::create(channel2));
+    port2->entangle(MessagePortChannel::create(channel1));
 }
 
 PassOwnPtr<MessagePortChannel> MessagePortChannel::create(PassRefPtr<PlatformMessagePortChannel> channel)
@@ -55,47 +68,79 @@ MessagePortChannel::MessagePortChannel(PassRefPtr<PlatformMessagePortChannel> ch
 MessagePortChannel::~MessagePortChannel()
 {
     // Make sure we close our platform channel when the base is freed, to keep the channel objects from leaking.
-    m_channel->close();
+    close();
 }
 
 bool MessagePortChannel::entangleIfOpen(MessagePort* port)
 {
-    return m_channel->entangleIfOpen(port);
+    // We can't call member functions on our remote pair while holding our mutex or we'll deadlock,
+    // but we need to guard against the remote port getting closed/freed, so create a standalone reference.
+    RefPtr<PlatformMessagePortChannel> remote = m_channel->entangledChannel();
+    if (!remote)
+        return false;
+    remote->setRemotePort(port);
+    return true;
 }
 
 void MessagePortChannel::disentangle()
 {
-    m_channel->disentangle();
+    RefPtr<PlatformMessagePortChannel> remote = m_channel->entangledChannel();
+    if (remote)
+        remote->setRemotePort(0);
 }
 
 void MessagePortChannel::postMessageToRemote(PassOwnPtr<MessagePortChannel::EventData> message)
 {
-    m_channel->postMessageToRemote(message);
+    MutexLocker lock(m_channel->m_mutex);
+    if (!m_channel->m_outgoingQueue)
+        return;
+    bool wasEmpty = m_channel->m_outgoingQueue->appendAndCheckEmpty(message);
+    if (wasEmpty && m_channel->m_remotePort)
+        m_channel->m_remotePort->messageAvailable();
 }
 
 bool MessagePortChannel::tryGetMessageFromRemote(OwnPtr<MessagePortChannel::EventData>& result)
 {
-    return m_channel->tryGetMessageFromRemote(result);
+    MutexLocker lock(m_channel->m_mutex);
+    result = m_channel->m_incomingQueue->tryGetMessage();
+    return result;
 }
 
 void MessagePortChannel::close()
 {
-    m_channel->close();
+    RefPtr<PlatformMessagePortChannel> remote = m_channel->entangledChannel();
+    if (!remote)
+        return;
+    m_channel->closeInternal();
+    remote->closeInternal();
 }
 
 bool MessagePortChannel::isConnectedTo(MessagePort* port)
 {
-    return m_channel->isConnectedTo(port);
+    // FIXME: What guarantees that the result remains the same after we release the lock?
+    MutexLocker lock(m_channel->m_mutex);
+    return m_channel->m_remotePort == port;
 }
 
 bool MessagePortChannel::hasPendingActivity()
 {
-    return m_channel->hasPendingActivity();
+    // FIXME: What guarantees that the result remains the same after we release the lock?
+    MutexLocker lock(m_channel->m_mutex);
+    return !m_channel->m_incomingQueue->isEmpty();
 }
 
 MessagePort* MessagePortChannel::locallyEntangledPort(const ScriptExecutionContext* context)
 {
-    return m_channel->locallyEntangledPort(context);
+    MutexLocker lock(m_channel->m_mutex);
+    // See if both contexts are run by the same thread (are the same context, or are both documents).
+    if (m_channel->m_remotePort) {
+        // The remote port's ScriptExecutionContext is guaranteed not to change here - MessagePort::contextDestroyed()
+        // will close the port before the context goes away, and close() will block because we are holding the mutex.
+        ScriptExecutionContext* remoteContext = m_channel->m_remotePort->scriptExecutionContext();
+        if (remoteContext == context || (remoteContext && remoteContext->isDocument() && context->isDocument()))
+            return m_channel->m_remotePort;
+    }
+    return 0;
 }
 
 PassRefPtr<PlatformMessagePortChannel> PlatformMessagePortChannel::create(PassRefPtr<MessagePortQueue> incoming, PassRefPtr<MessagePortQueue> outgoing)
@@ -104,8 +149,7 @@ PassRefPtr<PlatformMessagePortChannel> PlatformMessagePortChannel::create(PassRe
 }
 
 PlatformMessagePortChannel::PlatformMessagePortChannel(PassRefPtr<MessagePortQueue> incoming, PassRefPtr<MessagePortQueue> outgoing)
-    : m_entangledChannel(0)
-    , m_incomingQueue(incoming)
+    : m_incomingQueue(incoming)
     , m_outgoingQueue(outgoing)
     , m_remotePort(0)
 {
@@ -115,42 +159,6 @@ PlatformMessagePortChannel::~PlatformMessagePortChannel()
 {
 }
 
-void PlatformMessagePortChannel::createChannel(PassRefPtr<MessagePort> port1, PassRefPtr<MessagePort> port2)
-{
-    // Create incoming/outgoing queues.
-    RefPtr<PlatformMessagePortChannel::MessagePortQueue> queue1 = PlatformMessagePortChannel::MessagePortQueue::create();
-    RefPtr<PlatformMessagePortChannel::MessagePortQueue> queue2 = PlatformMessagePortChannel::MessagePortQueue::create();
-
-    // Create proxies for each endpoint.
-    RefPtr<PlatformMessagePortChannel> channel1 = PlatformMessagePortChannel::create(queue1, queue2);
-    RefPtr<PlatformMessagePortChannel> channel2 = PlatformMessagePortChannel::create(queue2, queue1);
-
-    // Entangle the two endpoints.
-    channel1->setEntangledChannel(channel2);
-    channel2->setEntangledChannel(channel1);
-
-    // Now entangle the proxies with the appropriate local ports.
-    port1->entangle(MessagePortChannel::create(channel2));
-    port2->entangle(MessagePortChannel::create(channel1));
-}
-
-bool PlatformMessagePortChannel::entangleIfOpen(MessagePort* port)
-{
-    // We can't call member functions on our remote pair while holding our mutex or we'll deadlock, but we need to guard against the remote port getting closed/freed, so create a standalone reference.
-    RefPtr<PlatformMessagePortChannel> remote = entangledChannel();
-    if (!remote)
-        return false;
-    remote->setRemotePort(port);
-    return true;
-}
-
-void PlatformMessagePortChannel::disentangle()
-{
-    RefPtr<PlatformMessagePortChannel> remote = entangledChannel();
-    if (remote)
-        remote->setRemotePort(0);
-}
-
 void PlatformMessagePortChannel::setRemotePort(MessagePort* port)
 {
     MutexLocker lock(m_mutex);
@@ -159,60 +167,15 @@ void PlatformMessagePortChannel::setRemotePort(MessagePort* port)
     m_remotePort = port;
 }
 
-MessagePort* PlatformMessagePortChannel::remotePort()
-{
-    MutexLocker lock(m_mutex);
-    return m_remotePort;
-}
-
 PassRefPtr<PlatformMessagePortChannel> PlatformMessagePortChannel::entangledChannel()
 {
+    // FIXME: What guarantees that the result remains the same after we release the lock?
+    // This lock only guarantees that the returned pointer will not be pointing to released memory,
+    // but not that it will still be pointing to this object's entangled port channel.
     MutexLocker lock(m_mutex);
     return m_entangledChannel;
 }
 
-void PlatformMessagePortChannel::setEntangledChannel(PassRefPtr<PlatformMessagePortChannel> remote)
-{
-    MutexLocker lock(m_mutex);
-    // Should only be set as part of initial creation/entanglement.
-    if (remote)
-        ASSERT(!m_entangledChannel.get());
-    m_entangledChannel = remote;
-}
-
-void PlatformMessagePortChannel::postMessageToRemote(PassOwnPtr<MessagePortChannel::EventData> message)
-{
-    MutexLocker lock(m_mutex);
-    if (!m_outgoingQueue)
-        return;
-    bool wasEmpty = m_outgoingQueue->appendAndCheckEmpty(message);
-    if (wasEmpty && m_remotePort)
-        m_remotePort->messageAvailable();
-}
-
-bool PlatformMessagePortChannel::tryGetMessageFromRemote(OwnPtr<MessagePortChannel::EventData>& result)
-{
-    MutexLocker lock(m_mutex);
-    result = m_incomingQueue->tryGetMessage();
-    return result;
-}
-
-bool PlatformMessagePortChannel::isConnectedTo(MessagePort* port)
-{
-    MutexLocker lock(m_mutex);
-    return m_remotePort == port;
-}
-
-// Closes the port so no further messages can be sent from either end.
-void PlatformMessagePortChannel::close()
-{
-    RefPtr<PlatformMessagePortChannel> remote = entangledChannel();
-    if (!remote)
-        return;
-    closeInternal();
-    remote->closeInternal();
-}
-
 void PlatformMessagePortChannel::closeInternal()
 {
     MutexLocker lock(m_mutex);
@@ -222,23 +185,4 @@ void PlatformMessagePortChannel::closeInternal()
     m_outgoingQueue = 0;
 }
 
-bool PlatformMessagePortChannel::hasPendingActivity()
-{
-    MutexLocker lock(m_mutex);
-    return !m_incomingQueue->isEmpty();
-}
-
-MessagePort* PlatformMessagePortChannel::locallyEntangledPort(const ScriptExecutionContext* context)
-{
-    MutexLocker lock(m_mutex);
-    // See if both contexts are run by the same thread (are the same context, or are both documents).
-    if (m_remotePort) {
-        // The remote port's ScriptExecutionContext is guaranteed not to change here - MessagePort::contextDestroyed() will close the port before the context goes away, and close() will block because we are holding the mutex.
-        ScriptExecutionContext* remoteContext = m_remotePort->scriptExecutionContext();
-        if (remoteContext == context || (remoteContext && remoteContext->isDocument() && context->isDocument()))
-            return m_remotePort;
-    }
-    return 0;
-}
-
 } // namespace WebCore
index 651810b..40aedc0 100644 (file)
@@ -46,18 +46,6 @@ namespace WebCore {
     // The goal of this implementation is to eliminate contention except when cloning or closing the port, so each side of the channel has its own separate mutex.
     class PlatformMessagePortChannel : public ThreadSafeRefCounted<PlatformMessagePortChannel> {
     public:
-        static void createChannel(PassRefPtr<MessagePort>, PassRefPtr<MessagePort>);
-
-        // APIs delegated from MessagePortChannel.h
-        bool entangleIfOpen(MessagePort*);
-        void disentangle();
-        void postMessageToRemote(PassOwnPtr<MessagePortChannel::EventData>);
-        bool tryGetMessageFromRemote(OwnPtr<MessagePortChannel::EventData>&);
-        void close();
-        bool isConnectedTo(MessagePort*);
-        bool hasPendingActivity();
-        MessagePort* locallyEntangledPort(const ScriptExecutionContext*);
-
         // Wrapper for MessageQueue that allows us to do thread safe sharing by two proxies.
         class MessagePortQueue : public ThreadSafeRefCounted<MessagePortQueue> {
         public:
@@ -86,15 +74,12 @@ namespace WebCore {
 
         ~PlatformMessagePortChannel();
 
-    private:
         static PassRefPtr<PlatformMessagePortChannel> create(PassRefPtr<MessagePortQueue> incoming, PassRefPtr<MessagePortQueue> outgoing);
         PlatformMessagePortChannel(PassRefPtr<MessagePortQueue> incoming, PassRefPtr<MessagePortQueue> outgoing);
 
         PassRefPtr<PlatformMessagePortChannel> entangledChannel();
-        void setEntangledChannel(PassRefPtr<PlatformMessagePortChannel>);
 
         void setRemotePort(MessagePort*);
-        MessagePort* remotePort();
         void closeInternal();
 
         // Mutex used to ensure exclusive access to the object internals.