Leak in WebSocketChannel with workers/worker-reload.html (part 2)
authoryutak@chromium.org <yutak@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 16 Apr 2012 01:43:40 +0000 (01:43 +0000)
committeryutak@chromium.org <yutak@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 16 Apr 2012 01:43:40 +0000 (01:43 +0000)
https://bugs.webkit.org/show_bug.cgi?id=83749

Reviewed by David Levin.

Source/WebCore:

Second attempt to remove leaks around WorkerThreadableWebSocketChannel.

No new tests, as this patch does not impose any functional change.

* Modules/websockets/ThreadableWebSocketChannelClientWrapper.cpp:
(WebCore::ThreadableWebSocketChannelClientWrapper::ThreadableWebSocketChannelClientWrapper):
(WebCore::ThreadableWebSocketChannelClientWrapper::failedWebSocketChannelCreation):
(WebCore::ThreadableWebSocketChannelClientWrapper::setFailedWebSocketChannelCreation):
* Modules/websockets/ThreadableWebSocketChannelClientWrapper.h:
Add a boolean flag indicating whether Bridge::initialize() has exited without receiving
a pointer to the peer object.
* Modules/websockets/WorkerThreadableWebSocketChannel.cpp:
(WebCore::WorkerThreadableWebSocketChannel::WorkerContextDidInitializeTask::create):
(WebCore::WorkerThreadableWebSocketChannel::WorkerContextDidInitializeTask::WorkerContextDidInitializeTask):
(WebCore::WorkerThreadableWebSocketChannel::WorkerContextDidInitializeTask::performTask):
Kick mainThreadDestroy() to delete the peer if the bridge has failed to receive
a pointer to the peer (waitForMethodCompletion() exited due to message queue's
termination).
(WebCore::WorkerThreadableWebSocketChannel::Bridge::mainThreadInitialize):
(WebCore::WorkerThreadableWebSocketChannel::Bridge::initialize):
* Modules/websockets/WorkerThreadableWebSocketChannel.h:
(WorkerThreadableWebSocketChannel):
Make WorkerContextDidInitializeTask an inner class of WorkerThreadableWebSocketChannel
so it can refer WorkerThreadableWebSocketChannel's static member function (mainThreadDestroy()).

Source/WebKit/chromium:

* src/WebWorkerClientImpl.cpp:
(WebKit::WebWorkerClientImpl::postTaskForModeToWorkerContext):
Correctly propagate the return value of postTaskForModeToWorkerContext().

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

Source/WebCore/ChangeLog
Source/WebCore/Modules/websockets/ThreadableWebSocketChannelClientWrapper.cpp
Source/WebCore/Modules/websockets/ThreadableWebSocketChannelClientWrapper.h
Source/WebCore/Modules/websockets/WorkerThreadableWebSocketChannel.cpp
Source/WebCore/Modules/websockets/WorkerThreadableWebSocketChannel.h
Source/WebKit/chromium/ChangeLog
Source/WebKit/chromium/src/WebWorkerClientImpl.cpp

index ff5d11b..4cb56a3 100644 (file)
@@ -1,3 +1,35 @@
+2012-04-15  Yuta Kitamura  <yutak@chromium.org>
+
+        Leak in WebSocketChannel with workers/worker-reload.html (part 2)
+        https://bugs.webkit.org/show_bug.cgi?id=83749
+
+        Reviewed by David Levin.
+
+        Second attempt to remove leaks around WorkerThreadableWebSocketChannel.
+
+        No new tests, as this patch does not impose any functional change.
+
+        * Modules/websockets/ThreadableWebSocketChannelClientWrapper.cpp:
+        (WebCore::ThreadableWebSocketChannelClientWrapper::ThreadableWebSocketChannelClientWrapper):
+        (WebCore::ThreadableWebSocketChannelClientWrapper::failedWebSocketChannelCreation):
+        (WebCore::ThreadableWebSocketChannelClientWrapper::setFailedWebSocketChannelCreation):
+        * Modules/websockets/ThreadableWebSocketChannelClientWrapper.h:
+        Add a boolean flag indicating whether Bridge::initialize() has exited without receiving
+        a pointer to the peer object.
+        * Modules/websockets/WorkerThreadableWebSocketChannel.cpp:
+        (WebCore::WorkerThreadableWebSocketChannel::WorkerContextDidInitializeTask::create):
+        (WebCore::WorkerThreadableWebSocketChannel::WorkerContextDidInitializeTask::WorkerContextDidInitializeTask):
+        (WebCore::WorkerThreadableWebSocketChannel::WorkerContextDidInitializeTask::performTask):
+        Kick mainThreadDestroy() to delete the peer if the bridge has failed to receive
+        a pointer to the peer (waitForMethodCompletion() exited due to message queue's
+        termination).
+        (WebCore::WorkerThreadableWebSocketChannel::Bridge::mainThreadInitialize):
+        (WebCore::WorkerThreadableWebSocketChannel::Bridge::initialize):
+        * Modules/websockets/WorkerThreadableWebSocketChannel.h:
+        (WorkerThreadableWebSocketChannel):
+        Make WorkerContextDidInitializeTask an inner class of WorkerThreadableWebSocketChannel
+        so it can refer WorkerThreadableWebSocketChannel's static member function (mainThreadDestroy()).
+
 2012-04-14  Emil A Eklund  <eae@chromium.org>
 
         Fix pixelSnapping for CalendarPicker, MediaControl and ScrollbarPart
index 50e7dc0..8ec8257 100644 (file)
@@ -45,6 +45,7 @@ ThreadableWebSocketChannelClientWrapper::ThreadableWebSocketChannelClientWrapper
     : m_context(context)
     , m_client(client)
     , m_peer(0)
+    , m_failedWebSocketChannelCreation(false)
     , m_syncMethodDone(true)
     , m_useHixie76Protocol(true)
     , m_sendRequestResult(ThreadableWebSocketChannel::SendFail)
@@ -90,6 +91,16 @@ void ThreadableWebSocketChannelClientWrapper::clearPeer()
     m_peer = 0;
 }
 
+bool ThreadableWebSocketChannelClientWrapper::failedWebSocketChannelCreation() const
+{
+    return m_failedWebSocketChannelCreation;
+}
+
+void ThreadableWebSocketChannelClientWrapper::setFailedWebSocketChannelCreation()
+{
+    m_failedWebSocketChannelCreation = true;
+}
+
 bool ThreadableWebSocketChannelClientWrapper::useHixie76Protocol() const
 {
     return m_useHixie76Protocol;
index d71a387..e7e4875 100644 (file)
@@ -61,6 +61,9 @@ public:
     void didCreateWebSocketChannel(WorkerThreadableWebSocketChannel::Peer*, bool useHixie76Protocol);
     void clearPeer();
 
+    bool failedWebSocketChannelCreation() const;
+    void setFailedWebSocketChannelCreation();
+
     // The value of useHixie76Protocol flag is cachable; this value is saved after WebSocketChannel (on the main
     // thread) is constructed.
     bool useHixie76Protocol() const;
@@ -105,6 +108,7 @@ private:
     ScriptExecutionContext* m_context;
     WebSocketChannelClient* m_client;
     WorkerThreadableWebSocketChannel::Peer* m_peer;
+    bool m_failedWebSocketChannelCreation;
     bool m_syncMethodDone;
     bool m_useHixie76Protocol;
     // ThreadSafeRefCounted must not have String member variables.
index dc54764..57251a3 100644 (file)
@@ -360,34 +360,44 @@ WorkerThreadableWebSocketChannel::Bridge::~Bridge()
     disconnect();
 }
 
-class WorkerContextDidInitializeTask : public ScriptExecutionContext::Task {
+class WorkerThreadableWebSocketChannel::WorkerContextDidInitializeTask : public ScriptExecutionContext::Task {
 public:
     static PassOwnPtr<ScriptExecutionContext::Task> create(WorkerThreadableWebSocketChannel::Peer* peer,
+                                                           WorkerLoaderProxy* loaderProxy,
                                                            PassRefPtr<ThreadableWebSocketChannelClientWrapper> workerClientWrapper,
                                                            bool useHixie76Protocol)
     {
-        return adoptPtr(new WorkerContextDidInitializeTask(peer, workerClientWrapper, useHixie76Protocol));
+        return adoptPtr(new WorkerContextDidInitializeTask(peer, loaderProxy, workerClientWrapper, useHixie76Protocol));
     }
 
     virtual ~WorkerContextDidInitializeTask() { }
     virtual void performTask(ScriptExecutionContext* context) OVERRIDE
     {
         ASSERT_UNUSED(context, context->isWorkerContext());
-        m_workerClientWrapper->didCreateWebSocketChannel(m_peer, m_useHixie76Protocol);
+        if (m_workerClientWrapper->failedWebSocketChannelCreation()) {
+            // If Bridge::initialize() quitted earlier, we need to kick mainThreadDestroy() to delete the peer.
+            OwnPtr<WorkerThreadableWebSocketChannel::Peer> peer = adoptPtr(m_peer);
+            m_peer = 0;
+            m_loaderProxy->postTaskToLoader(createCallbackTask(&WorkerThreadableWebSocketChannel::mainThreadDestroy, peer.release()));
+        } else
+            m_workerClientWrapper->didCreateWebSocketChannel(m_peer, m_useHixie76Protocol);
     }
     virtual bool isCleanupTask() const OVERRIDE { return true; }
 
 private:
     WorkerContextDidInitializeTask(WorkerThreadableWebSocketChannel::Peer* peer,
+                                   WorkerLoaderProxy* loaderProxy,
                                    PassRefPtr<ThreadableWebSocketChannelClientWrapper> workerClientWrapper,
                                    bool useHixie76Protocol)
         : m_peer(peer)
+        , m_loaderProxy(loaderProxy)
         , m_workerClientWrapper(workerClientWrapper)
         , m_useHixie76Protocol(useHixie76Protocol)
     {
     }
 
     WorkerThreadableWebSocketChannel::Peer* m_peer;
+    WorkerLoaderProxy* m_loaderProxy;
     RefPtr<ThreadableWebSocketChannelClientWrapper> m_workerClientWrapper;
     bool m_useHixie76Protocol;
 };
@@ -401,7 +411,7 @@ void WorkerThreadableWebSocketChannel::Bridge::mainThreadInitialize(ScriptExecut
 
     Peer* peer = Peer::create(clientWrapper, *loaderProxy, context, taskMode);
     bool sent = loaderProxy->postTaskForModeToWorkerContext(
-        WorkerContextDidInitializeTask::create(peer, clientWrapper, peer->useHixie76Protocol()), taskMode);
+        WorkerThreadableWebSocketChannel::WorkerContextDidInitializeTask::create(peer, loaderProxy, clientWrapper, peer->useHixie76Protocol()), taskMode);
     if (!sent) {
         clientWrapper->clearPeer();
         delete peer;
@@ -419,6 +429,8 @@ void WorkerThreadableWebSocketChannel::Bridge::initialize()
     waitForMethodCompletion();
     // m_peer may be null when the nested runloop exited before a peer has created.
     m_peer = m_workerClientWrapper->peer();
+    if (!m_peer)
+        m_workerClientWrapper->setFailedWebSocketChannelCreation();
 }
 
 void WorkerThreadableWebSocketChannel::mainThreadConnect(ScriptExecutionContext* context, Peer* peer, const KURL& url, const String& protocol)
index 752947a..3b836c3 100644 (file)
@@ -181,6 +181,8 @@ private:
     static void mainThreadSuspend(ScriptExecutionContext*, Peer*);
     static void mainThreadResume(ScriptExecutionContext*, Peer*);
 
+    class WorkerContextDidInitializeTask;
+
     RefPtr<WorkerContext> m_workerContext;
     RefPtr<ThreadableWebSocketChannelClientWrapper> m_workerClientWrapper;
     RefPtr<Bridge> m_bridge;
index 6829267..be0ae8e 100644 (file)
@@ -1,3 +1,14 @@
+2012-04-15  Yuta Kitamura  <yutak@chromium.org>
+
+        Leak in WebSocketChannel with workers/worker-reload.html (part 2)
+        https://bugs.webkit.org/show_bug.cgi?id=83749
+
+        Reviewed by David Levin.
+
+        * src/WebWorkerClientImpl.cpp:
+        (WebKit::WebWorkerClientImpl::postTaskForModeToWorkerContext):
+        Correctly propagate the return value of postTaskForModeToWorkerContext().
+
 2012-04-13  David Reveman  <reveman@chromium.org>
 
         [Chromium] Avoid unnecessary full tile updates for checkerboard tiles.
index 4481ac9..7db5445 100644 (file)
@@ -151,8 +151,7 @@ void WebWorkerClientImpl::postTaskToLoader(PassOwnPtr<ScriptExecutionContext::Ta
 
 bool WebWorkerClientImpl::postTaskForModeToWorkerContext(PassOwnPtr<ScriptExecutionContext::Task> task, const String& mode)
 {
-    m_proxy->postTaskForModeToWorkerContext(task, mode);
-    return true;
+    return m_proxy->postTaskForModeToWorkerContext(task, mode);
 }
 
 void WebWorkerClientImpl::postMessageToWorkerObject(PassRefPtr<SerializedScriptValue> value, PassOwnPtr<MessagePortChannelArray> ports)