Crash in WebCore::WorkerThreadableWebSocketChannel::Bridge::mainThreadCreateWebSocket...
authoryutak@chromium.org <yutak@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 4 Apr 2012 05:00:54 +0000 (05:00 +0000)
committeryutak@chromium.org <yutak@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 4 Apr 2012 05:00:54 +0000 (05:00 +0000)
https://bugs.webkit.org/show_bug.cgi?id=82873

Reviewed by David Levin.

Source/WebCore:

WorkerThreadableWebSocketChannel::Bridge should properly handle the cases where inter-thread
callback is not called due to the termination of the worker run loop. Specifically, the bridge
should not send its "this" pointer to the main thread, because the bridge object may be freed
in the worker thread before the main thread starts to process.

Test: http/tests/websocket/tests/hybi/workers/worker-reload.html

* Modules/websockets/ThreadableWebSocketChannelClientWrapper.cpp:
(WebCore::ThreadableWebSocketChannelClientWrapper::ThreadableWebSocketChannelClientWrapper):
(WebCore::ThreadableWebSocketChannelClientWrapper::peer):
(WebCore::ThreadableWebSocketChannelClientWrapper::didCreateWebSocketChannel):
Renamed from setUseHixie76Protocol, as this funtion now also sets m_peer.
Sets m_syncMethodDone to true, because this function is called in the end of
synchronous wait of Bridge::initialize().
(WebCore::ThreadableWebSocketChannelClientWrapper::clearPeer):
(WebCore::ThreadableWebSocketChannelClientWrapper::useHixie76Protocol):
* Modules/websockets/ThreadableWebSocketChannelClientWrapper.h:
Add WorkerThreadableWebSocketChannel::Peer which is initialized after the creation of
WebSocketChannel in the main thread.
(ThreadableWebSocketChannelClientWrapper):
* Modules/websockets/WorkerThreadableWebSocketChannel.cpp:
(WebCore::WorkerThreadableWebSocketChannel::WorkerThreadableWebSocketChannel):
Don't do synchronous wait in the constructor, as a member function may be called
during the wait before the constructor finishes. The meat of the constructor has
moved to initialize() function.
(WebCore::WorkerThreadableWebSocketChannel::Bridge::Bridge):
(WebCore::WorkerThreadableWebSocketChannel::Bridge::~Bridge):
(WorkerContextDidInitializeTask):
(WebCore::WorkerContextDidInitializeTask::create):
(WebCore::WorkerContextDidInitializeTask::~WorkerContextDidInitializeTask):
(WebCore::WorkerContextDidInitializeTask::WorkerContextDidInitializeTask):
(WebCore::WorkerThreadableWebSocketChannel::Bridge::mainThreadInitialize):
(WebCore::WorkerThreadableWebSocketChannel::Bridge::initialize):
Don't pass "this" object to the main thread. Receive the pointer to the peer object
via ThreadableWebSocketChannelClientWrapper which is ThreadSafeRefCounted<>.
(WebCore::WorkerThreadableWebSocketChannel::Bridge::connect):
m_peer may be NULL, and we should not do anything in that case.
(WebCore::WorkerThreadableWebSocketChannel::Bridge::send):
(WebCore::WorkerThreadableWebSocketChannel::Bridge::bufferedAmount):
(WebCore::WorkerThreadableWebSocketChannel::mainThreadClose):
(WebCore::WorkerThreadableWebSocketChannel::Bridge::close):
(WebCore::WorkerThreadableWebSocketChannel::Bridge::fail):
(WebCore::WorkerThreadableWebSocketChannel::Bridge::suspend):
(WebCore::WorkerThreadableWebSocketChannel::Bridge::resume):
* Modules/websockets/WorkerThreadableWebSocketChannel.h:
(WorkerThreadableWebSocketChannel):
(WebCore::WorkerThreadableWebSocketChannel::refThreadableWebSocketChannel):
(WebCore::WorkerThreadableWebSocketChannel::derefThreadableWebSocketChannel):
(Bridge):
* workers/DefaultSharedWorkerRepository.cpp:
(SharedWorkerProxy):
(WebCore::SharedWorkerProxy::postTaskForModeToWorkerContext):
* workers/WorkerLoaderProxy.h:
(WorkerLoaderProxy::postTaskForModeToWorkerContext):
Return bool to indicate whether postTask was successful or not. This is necessary
to avoid memory leaks of Peer object in Bridge::initialize() function.
* workers/WorkerMessagingProxy.cpp:
(WebCore::WorkerMessagingProxy::postTaskForModeToWorkerContext):
* workers/WorkerMessagingProxy.h:
(WorkerMessagingProxy):

Source/WebKit/chromium:

Change the function signature of WorkerLoaderProxy::postTaskForModeToWorkerContext().

* src/WebSharedWorkerImpl.cpp:
(WebKit::WebSharedWorkerImpl::postTaskForModeToWorkerContext):
* src/WebSharedWorkerImpl.h:
(WebSharedWorkerImpl):
* src/WebWorkerClientImpl.cpp:
(WebKit::WebWorkerClientImpl::postTaskForModeToWorkerContext):
* src/WebWorkerClientImpl.h:
(WebWorkerClientImpl):

LayoutTests:

* http/tests/websocket/tests/hybi/workers/resources/worker-reload-iframe.html: Added.
* http/tests/websocket/tests/hybi/workers/resources/worker-reload.js: Added.
* http/tests/websocket/tests/hybi/workers/worker-reload-expected.txt: Added.
* http/tests/websocket/tests/hybi/workers/worker-reload.html: Added.

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

19 files changed:
LayoutTests/ChangeLog
LayoutTests/http/tests/websocket/tests/hybi/workers/resources/worker-reload-iframe.html [new file with mode: 0644]
LayoutTests/http/tests/websocket/tests/hybi/workers/resources/worker-reload.js [new file with mode: 0644]
LayoutTests/http/tests/websocket/tests/hybi/workers/worker-reload-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/websocket/tests/hybi/workers/worker-reload.html [new file with mode: 0644]
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/WebCore/workers/DefaultSharedWorkerRepository.cpp
Source/WebCore/workers/WorkerLoaderProxy.h
Source/WebCore/workers/WorkerMessagingProxy.cpp
Source/WebCore/workers/WorkerMessagingProxy.h
Source/WebKit/chromium/ChangeLog
Source/WebKit/chromium/src/WebSharedWorkerImpl.cpp
Source/WebKit/chromium/src/WebSharedWorkerImpl.h
Source/WebKit/chromium/src/WebWorkerClientImpl.cpp
Source/WebKit/chromium/src/WebWorkerClientImpl.h

index d96f746..c40bd29 100644 (file)
@@ -1,3 +1,15 @@
+2012-04-03  Yuta Kitamura  <yutak@chromium.org>
+
+        Crash in WebCore::WorkerThreadableWebSocketChannel::Bridge::mainThreadCreateWebSocketChannel
+        https://bugs.webkit.org/show_bug.cgi?id=82873
+
+        Reviewed by David Levin.
+
+        * http/tests/websocket/tests/hybi/workers/resources/worker-reload-iframe.html: Added.
+        * http/tests/websocket/tests/hybi/workers/resources/worker-reload.js: Added.
+        * http/tests/websocket/tests/hybi/workers/worker-reload-expected.txt: Added.
+        * http/tests/websocket/tests/hybi/workers/worker-reload.html: Added.
+
 2012-04-03  Keishi Hattori  <keishi@webkit.org>
 
         Disable ENABLE_DATALIST for now
diff --git a/LayoutTests/http/tests/websocket/tests/hybi/workers/resources/worker-reload-iframe.html b/LayoutTests/http/tests/websocket/tests/hybi/workers/resources/worker-reload-iframe.html
new file mode 100644 (file)
index 0000000..f46307b
--- /dev/null
@@ -0,0 +1,22 @@
+<!DOCTYPE html>
+<html>
+<body>
+<div id="result">Running...</div>
+<script type="text/javascript">
+var repeat = 0;
+var regex = new RegExp("#repeat(\\d+)");
+var match = regex.exec(location.hash);
+if (match)
+    repeat = parseInt(match[1]);
+
+if (repeat === 100) {
+    document.getElementById("result").innerHTML = "Done.";
+    parent.document.iframeFinished();
+} else {
+    new Worker("worker-reload.js");
+    location.href = "worker-reload-iframe.html#repeat" + (repeat + 1);
+    setTimeout('location.reload()', 1);
+}
+</script>
+</body>
+</html>
diff --git a/LayoutTests/http/tests/websocket/tests/hybi/workers/resources/worker-reload.js b/LayoutTests/http/tests/websocket/tests/hybi/workers/resources/worker-reload.js
new file mode 100644 (file)
index 0000000..9d2bcca
--- /dev/null
@@ -0,0 +1 @@
+new WebSocket('ws://localhost:12345');
diff --git a/LayoutTests/http/tests/websocket/tests/hybi/workers/worker-reload-expected.txt b/LayoutTests/http/tests/websocket/tests/hybi/workers/worker-reload-expected.txt
new file mode 100644 (file)
index 0000000..80746f7
--- /dev/null
@@ -0,0 +1,10 @@
+Reload after WebSocket creation should not cause a crash.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+PASS Inserted <iframe>.
+PASS Test finished.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/http/tests/websocket/tests/hybi/workers/worker-reload.html b/LayoutTests/http/tests/websocket/tests/hybi/workers/worker-reload.html
new file mode 100644 (file)
index 0000000..d66a669
--- /dev/null
@@ -0,0 +1,29 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src="../../../../../js-test-resources/js-test-pre.js"></script>
+</head>
+<body>
+<div id="description"></div>
+<div id="console"></div>
+<script type="text/javascript">
+description("Reload after WebSocket creation should not cause a crash.");
+
+window.jsTestIsAsync = true;
+if (window.layoutTestController)
+    layoutTestController.overridePreference("WebKitHixie76WebSocketProtocolEnabled", 0);
+
+document.iframeFinished = function()
+{
+    testPassed("Test finished.");
+    finishJSTest();
+};
+
+var iframeElement = document.createElement("iframe");
+iframeElement.src = "resources/worker-reload-iframe.html";
+document.body.appendChild(iframeElement);
+testPassed("Inserted <iframe>.");
+</script>
+<script src="../../../../../js-test-resources/js-test-post.js"></script>
+</body>
+</html>
index 869721b..d1e423a 100644 (file)
@@ -1,3 +1,71 @@
+2012-04-03  Yuta Kitamura  <yutak@chromium.org>
+
+        Crash in WebCore::WorkerThreadableWebSocketChannel::Bridge::mainThreadCreateWebSocketChannel
+        https://bugs.webkit.org/show_bug.cgi?id=82873
+
+        Reviewed by David Levin.
+
+        WorkerThreadableWebSocketChannel::Bridge should properly handle the cases where inter-thread
+        callback is not called due to the termination of the worker run loop. Specifically, the bridge
+        should not send its "this" pointer to the main thread, because the bridge object may be freed
+        in the worker thread before the main thread starts to process.
+
+        Test: http/tests/websocket/tests/hybi/workers/worker-reload.html
+
+        * Modules/websockets/ThreadableWebSocketChannelClientWrapper.cpp:
+        (WebCore::ThreadableWebSocketChannelClientWrapper::ThreadableWebSocketChannelClientWrapper):
+        (WebCore::ThreadableWebSocketChannelClientWrapper::peer):
+        (WebCore::ThreadableWebSocketChannelClientWrapper::didCreateWebSocketChannel):
+        Renamed from setUseHixie76Protocol, as this funtion now also sets m_peer.
+        Sets m_syncMethodDone to true, because this function is called in the end of
+        synchronous wait of Bridge::initialize().
+        (WebCore::ThreadableWebSocketChannelClientWrapper::clearPeer):
+        (WebCore::ThreadableWebSocketChannelClientWrapper::useHixie76Protocol):
+        * Modules/websockets/ThreadableWebSocketChannelClientWrapper.h:
+        Add WorkerThreadableWebSocketChannel::Peer which is initialized after the creation of
+        WebSocketChannel in the main thread.
+        (ThreadableWebSocketChannelClientWrapper):
+        * Modules/websockets/WorkerThreadableWebSocketChannel.cpp:
+        (WebCore::WorkerThreadableWebSocketChannel::WorkerThreadableWebSocketChannel):
+        Don't do synchronous wait in the constructor, as a member function may be called
+        during the wait before the constructor finishes. The meat of the constructor has
+        moved to initialize() function.
+        (WebCore::WorkerThreadableWebSocketChannel::Bridge::Bridge):
+        (WebCore::WorkerThreadableWebSocketChannel::Bridge::~Bridge):
+        (WorkerContextDidInitializeTask):
+        (WebCore::WorkerContextDidInitializeTask::create):
+        (WebCore::WorkerContextDidInitializeTask::~WorkerContextDidInitializeTask):
+        (WebCore::WorkerContextDidInitializeTask::WorkerContextDidInitializeTask):
+        (WebCore::WorkerThreadableWebSocketChannel::Bridge::mainThreadInitialize):
+        (WebCore::WorkerThreadableWebSocketChannel::Bridge::initialize):
+        Don't pass "this" object to the main thread. Receive the pointer to the peer object
+        via ThreadableWebSocketChannelClientWrapper which is ThreadSafeRefCounted<>.
+        (WebCore::WorkerThreadableWebSocketChannel::Bridge::connect):
+        m_peer may be NULL, and we should not do anything in that case.
+        (WebCore::WorkerThreadableWebSocketChannel::Bridge::send):
+        (WebCore::WorkerThreadableWebSocketChannel::Bridge::bufferedAmount):
+        (WebCore::WorkerThreadableWebSocketChannel::mainThreadClose):
+        (WebCore::WorkerThreadableWebSocketChannel::Bridge::close):
+        (WebCore::WorkerThreadableWebSocketChannel::Bridge::fail):
+        (WebCore::WorkerThreadableWebSocketChannel::Bridge::suspend):
+        (WebCore::WorkerThreadableWebSocketChannel::Bridge::resume):
+        * Modules/websockets/WorkerThreadableWebSocketChannel.h:
+        (WorkerThreadableWebSocketChannel):
+        (WebCore::WorkerThreadableWebSocketChannel::refThreadableWebSocketChannel):
+        (WebCore::WorkerThreadableWebSocketChannel::derefThreadableWebSocketChannel):
+        (Bridge):
+        * workers/DefaultSharedWorkerRepository.cpp:
+        (SharedWorkerProxy):
+        (WebCore::SharedWorkerProxy::postTaskForModeToWorkerContext):
+        * workers/WorkerLoaderProxy.h:
+        (WorkerLoaderProxy::postTaskForModeToWorkerContext):
+        Return bool to indicate whether postTask was successful or not. This is necessary
+        to avoid memory leaks of Peer object in Bridge::initialize() function.
+        * workers/WorkerMessagingProxy.cpp:
+        (WebCore::WorkerMessagingProxy::postTaskForModeToWorkerContext):
+        * workers/WorkerMessagingProxy.h:
+        (WorkerMessagingProxy):
+
 2012-04-03  Keishi Hattori  <keishi@webkit.org>
 
         Disable ENABLE_DATALIST for now
index 9a4f3fa..50e7dc0 100644 (file)
@@ -44,6 +44,7 @@ namespace WebCore {
 ThreadableWebSocketChannelClientWrapper::ThreadableWebSocketChannelClientWrapper(ScriptExecutionContext* context, WebSocketChannelClient* client)
     : m_context(context)
     , m_client(client)
+    , m_peer(0)
     , m_syncMethodDone(true)
     , m_useHixie76Protocol(true)
     , m_sendRequestResult(ThreadableWebSocketChannel::SendFail)
@@ -72,14 +73,26 @@ bool ThreadableWebSocketChannelClientWrapper::syncMethodDone() const
     return m_syncMethodDone;
 }
 
-bool ThreadableWebSocketChannelClientWrapper::useHixie76Protocol() const
+WorkerThreadableWebSocketChannel::Peer* ThreadableWebSocketChannelClientWrapper::peer() const
 {
-    return m_useHixie76Protocol;
+    return m_peer;
 }
 
-void ThreadableWebSocketChannelClientWrapper::setUseHixie76Protocol(bool useHixie76Protocol)
+void ThreadableWebSocketChannelClientWrapper::didCreateWebSocketChannel(WorkerThreadableWebSocketChannel::Peer* peer, bool useHixie76Protocol)
 {
+    m_peer = peer;
     m_useHixie76Protocol = useHixie76Protocol;
+    m_syncMethodDone = true;
+}
+
+void ThreadableWebSocketChannelClientWrapper::clearPeer()
+{
+    m_peer = 0;
+}
+
+bool ThreadableWebSocketChannelClientWrapper::useHixie76Protocol() const
+{
+    return m_useHixie76Protocol;
 }
 
 String ThreadableWebSocketChannelClientWrapper::subprotocol() const
index bfd1063..d71a387 100644 (file)
@@ -37,6 +37,7 @@
 #include "ScriptExecutionContext.h"
 #include "ThreadableWebSocketChannel.h"
 #include "WebSocketChannelClient.h"
+#include "WorkerThreadableWebSocketChannel.h"
 #include <wtf/Forward.h>
 #include <wtf/OwnPtr.h>
 #include <wtf/PassOwnPtr.h>
@@ -56,10 +57,13 @@ public:
     void setSyncMethodDone();
     bool syncMethodDone() const;
 
+    WorkerThreadableWebSocketChannel::Peer* peer() const;
+    void didCreateWebSocketChannel(WorkerThreadableWebSocketChannel::Peer*, bool useHixie76Protocol);
+    void clearPeer();
+
     // The value of useHixie76Protocol flag is cachable; this value is saved after WebSocketChannel (on the main
     // thread) is constructed.
     bool useHixie76Protocol() const;
-    void setUseHixie76Protocol(bool);
 
     // Subprotocol and extensions are cached too. Will be available when didConnect() callback is invoked.
     String subprotocol() const;
@@ -100,6 +104,7 @@ private:
 
     ScriptExecutionContext* m_context;
     WebSocketChannelClient* m_client;
+    WorkerThreadableWebSocketChannel::Peer* m_peer;
     bool m_syncMethodDone;
     bool m_useHixie76Protocol;
     // ThreadSafeRefCounted must not have String member variables.
index 762dc43..ae9e2ff 100644 (file)
@@ -57,6 +57,7 @@ WorkerThreadableWebSocketChannel::WorkerThreadableWebSocketChannel(WorkerContext
     , m_workerClientWrapper(ThreadableWebSocketChannelClientWrapper::create(context, client))
     , m_bridge(Bridge::create(m_workerClientWrapper, m_workerContext, taskMode))
 {
+    m_bridge->initialize();
 }
 
 WorkerThreadableWebSocketChannel::~WorkerThreadableWebSocketChannel()
@@ -344,47 +345,80 @@ void WorkerThreadableWebSocketChannel::Peer::didClose(unsigned long unhandledBuf
     m_loaderProxy.postTaskForModeToWorkerContext(createCallbackTask(&workerContextDidClose, m_workerClientWrapper, unhandledBufferedAmount, closingHandshakeCompletion, code, reason), m_taskMode);
 }
 
-void WorkerThreadableWebSocketChannel::Bridge::setWebSocketChannel(ScriptExecutionContext* context, Bridge* thisPtr, Peer* peer, PassRefPtr<ThreadableWebSocketChannelClientWrapper> workerClientWrapper, bool useHixie76Protocol)
+WorkerThreadableWebSocketChannel::Bridge::Bridge(PassRefPtr<ThreadableWebSocketChannelClientWrapper> workerClientWrapper, PassRefPtr<WorkerContext> workerContext, const String& taskMode)
+    : m_workerClientWrapper(workerClientWrapper)
+    , m_workerContext(workerContext)
+    , m_loaderProxy(m_workerContext->thread()->workerLoaderProxy())
+    , m_taskMode(taskMode)
+    , m_peer(0)
 {
-    ASSERT_UNUSED(context, context->isWorkerContext());
-    thisPtr->m_peer = peer;
-    workerClientWrapper->setUseHixie76Protocol(useHixie76Protocol);
-    workerClientWrapper->setSyncMethodDone();
+    ASSERT(m_workerClientWrapper.get());
 }
 
-void WorkerThreadableWebSocketChannel::Bridge::mainThreadCreateWebSocketChannel(ScriptExecutionContext* context, Bridge* thisPtr, PassRefPtr<ThreadableWebSocketChannelClientWrapper> prpClientWrapper, const String& taskMode)
+WorkerThreadableWebSocketChannel::Bridge::~Bridge()
+{
+    disconnect();
+}
+
+class WorkerContextDidInitializeTask : public ScriptExecutionContext::Task {
+public:
+    static PassOwnPtr<ScriptExecutionContext::Task> create(WorkerThreadableWebSocketChannel::Peer* peer,
+                                                           PassRefPtr<ThreadableWebSocketChannelClientWrapper> workerClientWrapper,
+                                                           bool useHixie76Protocol)
+    {
+        return adoptPtr(new WorkerContextDidInitializeTask(peer, workerClientWrapper, useHixie76Protocol));
+    }
+
+    virtual ~WorkerContextDidInitializeTask() { }
+    virtual void performTask(ScriptExecutionContext* context) OVERRIDE
+    {
+        ASSERT_UNUSED(context, context->isWorkerContext());
+        m_workerClientWrapper->didCreateWebSocketChannel(m_peer, m_useHixie76Protocol);
+    }
+    virtual bool isCleanupTask() const OVERRIDE { return true; }
+
+private:
+    WorkerContextDidInitializeTask(WorkerThreadableWebSocketChannel::Peer* peer,
+                                   PassRefPtr<ThreadableWebSocketChannelClientWrapper> workerClientWrapper,
+                                   bool useHixie76Protocol)
+        : m_peer(peer)
+        , m_workerClientWrapper(workerClientWrapper)
+        , m_useHixie76Protocol(useHixie76Protocol)
+    {
+    }
+
+    WorkerThreadableWebSocketChannel::Peer* m_peer;
+    RefPtr<ThreadableWebSocketChannelClientWrapper> m_workerClientWrapper;
+    bool m_useHixie76Protocol;
+};
+
+void WorkerThreadableWebSocketChannel::Bridge::mainThreadInitialize(ScriptExecutionContext* context, WorkerLoaderProxy* loaderProxy, PassRefPtr<ThreadableWebSocketChannelClientWrapper> prpClientWrapper, const String& taskMode)
 {
     ASSERT(isMainThread());
     ASSERT_UNUSED(context, context->isDocument());
 
     RefPtr<ThreadableWebSocketChannelClientWrapper> clientWrapper = prpClientWrapper;
 
-    Peer* peer = Peer::create(clientWrapper, thisPtr->m_loaderProxy, context, taskMode);
-    thisPtr->m_loaderProxy.postTaskForModeToWorkerContext(
-        createCallbackTask(&Bridge::setWebSocketChannel,
-                           AllowCrossThreadAccess(thisPtr),
-                           AllowCrossThreadAccess(peer), clientWrapper, peer->useHixie76Protocol()), taskMode);
+    Peer* peer = Peer::create(clientWrapper, *loaderProxy, context, taskMode);
+    bool sent = loaderProxy->postTaskForModeToWorkerContext(
+        WorkerContextDidInitializeTask::create(peer, clientWrapper, peer->useHixie76Protocol()), taskMode);
+    if (!sent) {
+        clientWrapper->clearPeer();
+        delete peer;
+    }
 }
 
-WorkerThreadableWebSocketChannel::Bridge::Bridge(PassRefPtr<ThreadableWebSocketChannelClientWrapper> workerClientWrapper, PassRefPtr<WorkerContext> workerContext, const String& taskMode)
-    : m_workerClientWrapper(workerClientWrapper)
-    , m_workerContext(workerContext)
-    , m_loaderProxy(m_workerContext->thread()->workerLoaderProxy())
-    , m_taskMode(taskMode)
-    , m_peer(0)
+void WorkerThreadableWebSocketChannel::Bridge::initialize()
 {
-    ASSERT(m_workerClientWrapper.get());
+    ASSERT(!m_peer);
     setMethodNotCompleted();
+    RefPtr<Bridge> protect(this);
     m_loaderProxy.postTaskToLoader(
-        createCallbackTask(&Bridge::mainThreadCreateWebSocketChannel,
-                           AllowCrossThreadAccess(this), m_workerClientWrapper, m_taskMode));
+        createCallbackTask(&Bridge::mainThreadInitialize,
+                           AllowCrossThreadAccess(&m_loaderProxy), m_workerClientWrapper, m_taskMode));
     waitForMethodCompletion();
-    ASSERT(m_peer);
-}
-
-WorkerThreadableWebSocketChannel::Bridge::~Bridge()
-{
-    disconnect();
+    // m_peer may be null when the nested runloop exited before a peer has created.
+    m_peer = m_workerClientWrapper->peer();
 }
 
 void WorkerThreadableWebSocketChannel::mainThreadConnect(ScriptExecutionContext* context, Peer* peer, const KURL& url, const String& protocol)
@@ -399,7 +433,8 @@ void WorkerThreadableWebSocketChannel::mainThreadConnect(ScriptExecutionContext*
 void WorkerThreadableWebSocketChannel::Bridge::connect(const KURL& url, const String& protocol)
 {
     ASSERT(m_workerClientWrapper);
-    ASSERT(m_peer);
+    if (!m_peer)
+        return;
     m_loaderProxy.postTaskToLoader(createCallbackTask(&WorkerThreadableWebSocketChannel::mainThreadConnect, AllowCrossThreadAccess(m_peer), url, protocol));
 }
 
@@ -434,9 +469,8 @@ void WorkerThreadableWebSocketChannel::mainThreadSendBlob(ScriptExecutionContext
 
 ThreadableWebSocketChannel::SendResult WorkerThreadableWebSocketChannel::Bridge::send(const String& message)
 {
-    if (!m_workerClientWrapper)
+    if (!m_workerClientWrapper || !m_peer)
         return ThreadableWebSocketChannel::SendFail;
-    ASSERT(m_peer);
     setMethodNotCompleted();
     m_loaderProxy.postTaskToLoader(createCallbackTask(&WorkerThreadableWebSocketChannel::mainThreadSend, AllowCrossThreadAccess(m_peer), message));
     RefPtr<Bridge> protect(this);
@@ -449,9 +483,8 @@ ThreadableWebSocketChannel::SendResult WorkerThreadableWebSocketChannel::Bridge:
 
 ThreadableWebSocketChannel::SendResult WorkerThreadableWebSocketChannel::Bridge::send(const ArrayBuffer& binaryData)
 {
-    if (!m_workerClientWrapper)
+    if (!m_workerClientWrapper || !m_peer)
         return ThreadableWebSocketChannel::SendFail;
-    ASSERT(m_peer);
     // ArrayBuffer isn't thread-safe, hence the content of ArrayBuffer is copied into Vector<char>.
     OwnPtr<Vector<char> > data = adoptPtr(new Vector<char>(binaryData.byteLength()));
     if (binaryData.byteLength())
@@ -468,9 +501,8 @@ ThreadableWebSocketChannel::SendResult WorkerThreadableWebSocketChannel::Bridge:
 
 ThreadableWebSocketChannel::SendResult WorkerThreadableWebSocketChannel::Bridge::send(const Blob& binaryData)
 {
-    if (!m_workerClientWrapper)
+    if (!m_workerClientWrapper || !m_peer)
         return ThreadableWebSocketChannel::SendFail;
-    ASSERT(m_peer);
     setMethodNotCompleted();
     m_loaderProxy.postTaskToLoader(createCallbackTask(&WorkerThreadableWebSocketChannel::mainThreadSendBlob, AllowCrossThreadAccess(m_peer), binaryData.url(), binaryData.type(), binaryData.size()));
     RefPtr<Bridge> protect(this);
@@ -492,9 +524,8 @@ void WorkerThreadableWebSocketChannel::mainThreadBufferedAmount(ScriptExecutionC
 
 unsigned long WorkerThreadableWebSocketChannel::Bridge::bufferedAmount()
 {
-    if (!m_workerClientWrapper)
+    if (!m_workerClientWrapper || !m_peer)
         return 0;
-    ASSERT(m_peer);
     setMethodNotCompleted();
     m_loaderProxy.postTaskToLoader(createCallbackTask(&WorkerThreadableWebSocketChannel::mainThreadBufferedAmount, AllowCrossThreadAccess(m_peer)));
     RefPtr<Bridge> protect(this);
@@ -505,7 +536,7 @@ unsigned long WorkerThreadableWebSocketChannel::Bridge::bufferedAmount()
     return 0;
 }
 
-void WorkerThreadableWebSocketChannel::mainThreadClose(ScriptExecutionContext* context, Peer* peer, int code, const String&reason)
+void WorkerThreadableWebSocketChannel::mainThreadClose(ScriptExecutionContext* context, Peer* peer, int code, const String& reason)
 {
     ASSERT(isMainThread());
     ASSERT_UNUSED(context, context->isDocument());
@@ -516,7 +547,8 @@ void WorkerThreadableWebSocketChannel::mainThreadClose(ScriptExecutionContext* c
 
 void WorkerThreadableWebSocketChannel::Bridge::close(int code, const String& reason)
 {
-    ASSERT(m_peer);
+    if (!m_peer)
+        return;
     m_loaderProxy.postTaskToLoader(createCallbackTask(&WorkerThreadableWebSocketChannel::mainThreadClose, AllowCrossThreadAccess(m_peer), code, reason));
 }
 
@@ -531,7 +563,8 @@ void WorkerThreadableWebSocketChannel::mainThreadFail(ScriptExecutionContext* co
 
 void WorkerThreadableWebSocketChannel::Bridge::fail(const String& reason)
 {
-    ASSERT(m_peer);
+    if (!m_peer)
+        return;
     m_loaderProxy.postTaskToLoader(createCallbackTask(&WorkerThreadableWebSocketChannel::mainThreadFail, AllowCrossThreadAccess(m_peer), reason));
 }
 
@@ -566,7 +599,8 @@ void WorkerThreadableWebSocketChannel::mainThreadSuspend(ScriptExecutionContext*
 
 void WorkerThreadableWebSocketChannel::Bridge::suspend()
 {
-    ASSERT(m_peer);
+    if (!m_peer)
+        return;
     m_loaderProxy.postTaskToLoader(createCallbackTask(&WorkerThreadableWebSocketChannel::mainThreadSuspend, AllowCrossThreadAccess(m_peer)));
 }
 
@@ -581,7 +615,8 @@ void WorkerThreadableWebSocketChannel::mainThreadResume(ScriptExecutionContext*
 
 void WorkerThreadableWebSocketChannel::Bridge::resume()
 {
-    ASSERT(m_peer);
+    if (!m_peer)
+        return;
     m_loaderProxy.postTaskToLoader(createCallbackTask(&WorkerThreadableWebSocketChannel::mainThreadResume, AllowCrossThreadAccess(m_peer)));
 }
 
index fa816da..953e354 100644 (file)
@@ -36,6 +36,7 @@
 #include "PlatformString.h"
 #include "ThreadableWebSocketChannel.h"
 #include "WebSocketChannelClient.h"
+#include "WorkerContext.h"
 
 #include <wtf/PassRefPtr.h>
 #include <wtf/RefCounted.h>
@@ -75,14 +76,6 @@ public:
     virtual void suspend() OVERRIDE;
     virtual void resume() OVERRIDE;
 
-    using RefCounted<WorkerThreadableWebSocketChannel>::ref;
-    using RefCounted<WorkerThreadableWebSocketChannel>::deref;
-
-protected:
-    virtual void refThreadableWebSocketChannel() { ref(); }
-    virtual void derefThreadableWebSocketChannel() { deref(); }
-
-private:
     // Generated by the bridge.  The Peer and its bridge should have identical
     // lifetimes.
     class Peer : public WebSocketChannelClient {
@@ -123,6 +116,14 @@ private:
         String m_taskMode;
     };
 
+    using RefCounted<WorkerThreadableWebSocketChannel>::ref;
+    using RefCounted<WorkerThreadableWebSocketChannel>::deref;
+
+protected:
+    virtual void refThreadableWebSocketChannel() { ref(); }
+    virtual void derefThreadableWebSocketChannel() { deref(); }
+
+private:
     // Bridge for Peer.  Running on the worker thread.
     class Bridge : public RefCounted<Bridge> {
     public:
@@ -131,6 +132,7 @@ private:
             return adoptRef(new Bridge(workerClientWrapper, workerContext, taskMode));
         }
         ~Bridge();
+        void initialize();
         void connect(const KURL&, const String& protocol);
         ThreadableWebSocketChannel::SendResult send(const String& message);
         ThreadableWebSocketChannel::SendResult send(const ArrayBuffer&);
@@ -151,7 +153,7 @@ private:
         static void setWebSocketChannel(ScriptExecutionContext*, Bridge* thisPtr, Peer*, PassRefPtr<ThreadableWebSocketChannelClientWrapper>, bool useHixie76Protocol);
 
         // Executed on the main thread to create a Peer for this bridge.
-        static void mainThreadCreateWebSocketChannel(ScriptExecutionContext*, Bridge* thisPtr, PassRefPtr<ThreadableWebSocketChannelClientWrapper>, const String& taskMode);
+        static void mainThreadInitialize(ScriptExecutionContext*, WorkerLoaderProxy*, PassRefPtr<ThreadableWebSocketChannelClientWrapper>, const String& taskMode);
 
         // Executed on the worker context's thread.
         void clearClientWrapper();
index 773ccd9..4094e3b 100644 (file)
@@ -77,7 +77,7 @@ public:
 
     // WorkerLoaderProxy
     virtual void postTaskToLoader(PassOwnPtr<ScriptExecutionContext::Task>);
-    virtual void postTaskForModeToWorkerContext(PassOwnPtr<ScriptExecutionContext::Task>, const String&);
+    virtual bool postTaskForModeToWorkerContext(PassOwnPtr<ScriptExecutionContext::Task>, const String&);
 
     // WorkerReportingProxy
     virtual void postExceptionToWorkerObject(const String& errorMessage, int lineNumber, const String& sourceURL);
@@ -151,12 +151,13 @@ void SharedWorkerProxy::postTaskToLoader(PassOwnPtr<ScriptExecutionContext::Task
     document->postTask(task);
 }
 
-void SharedWorkerProxy::postTaskForModeToWorkerContext(PassOwnPtr<ScriptExecutionContext::Task> task, const String& mode)
+bool SharedWorkerProxy::postTaskForModeToWorkerContext(PassOwnPtr<ScriptExecutionContext::Task> task, const String& mode)
 {
     if (isClosing())
-        return;
+        return false;
     ASSERT(m_thread);
     m_thread->runLoop().postTaskForMode(task, mode);
+    return true;
 }
 
 static void postExceptionTask(ScriptExecutionContext* context, const String& errorMessage, int lineNumber, const String& sourceURL)
index 219dab4..d2fcf96 100644 (file)
@@ -53,7 +53,8 @@ namespace WebCore {
 
         // Posts callbacks from loading code to the WorkerContext. The 'mode' is used to differentiate
         // specific synchronous loading requests so they can be 'nested', per spec.
-        virtual void postTaskForModeToWorkerContext(PassOwnPtr<ScriptExecutionContext::Task>, const String& mode) = 0;
+        // Returns true if the task was posted successfully.
+        virtual bool postTaskForModeToWorkerContext(PassOwnPtr<ScriptExecutionContext::Task>, const String& mode) = 0;
     };
 
 } // namespace WebCore
index 1fdccbd..7e8e5da 100644 (file)
@@ -296,13 +296,14 @@ void WorkerMessagingProxy::postMessageToWorkerContext(PassRefPtr<SerializedScrip
         m_queuedEarlyTasks.append(MessageWorkerContextTask::create(message, channels));
 }
 
-void WorkerMessagingProxy::postTaskForModeToWorkerContext(PassOwnPtr<ScriptExecutionContext::Task> task, const String& mode)
+bool WorkerMessagingProxy::postTaskForModeToWorkerContext(PassOwnPtr<ScriptExecutionContext::Task> task, const String& mode)
 {
     if (m_askedToTerminate)
-        return;
+        return false;
 
     ASSERT(m_workerThread);
     m_workerThread->runLoop().postTaskForMode(task, mode);
+    return true;
 }
 
 void WorkerMessagingProxy::postTaskToLoader(PassOwnPtr<ScriptExecutionContext::Task> task)
index 8536113..c06fc0b 100644 (file)
@@ -82,7 +82,7 @@ namespace WebCore {
         // These methods are called on different threads to schedule loading
         // requests and to send callbacks back to WorkerContext.
         virtual void postTaskToLoader(PassOwnPtr<ScriptExecutionContext::Task>);
-        virtual void postTaskForModeToWorkerContext(PassOwnPtr<ScriptExecutionContext::Task>, const String& mode);
+        virtual bool postTaskForModeToWorkerContext(PassOwnPtr<ScriptExecutionContext::Task>, const String& mode);
 
         void workerThreadCreated(PassRefPtr<DedicatedWorkerThread>);
 
index c026789..e9a87d8 100644 (file)
@@ -1,3 +1,21 @@
+2012-04-03  Yuta Kitamura  <yutak@chromium.org>
+
+        Crash in WebCore::WorkerThreadableWebSocketChannel::Bridge::mainThreadCreateWebSocketChannel
+        https://bugs.webkit.org/show_bug.cgi?id=82873
+
+        Reviewed by David Levin.
+
+        Change the function signature of WorkerLoaderProxy::postTaskForModeToWorkerContext().
+
+        * src/WebSharedWorkerImpl.cpp:
+        (WebKit::WebSharedWorkerImpl::postTaskForModeToWorkerContext):
+        * src/WebSharedWorkerImpl.h:
+        (WebSharedWorkerImpl):
+        * src/WebWorkerClientImpl.cpp:
+        (WebKit::WebWorkerClientImpl::postTaskForModeToWorkerContext):
+        * src/WebWorkerClientImpl.h:
+        (WebWorkerClientImpl):
+
 2012-04-03  Ian Vollick  <vollick@chromium.org>
 
         [chromium] Include Image.h in TextFieldDecoratorImpl.cpp
index 3fa19d2..301c75e 100644 (file)
@@ -320,10 +320,11 @@ void WebSharedWorkerImpl::postTaskToLoader(PassOwnPtr<ScriptExecutionContext::Ta
     m_loadingDocument->postTask(task);
 }
 
-void WebSharedWorkerImpl::postTaskForModeToWorkerContext(
+bool WebSharedWorkerImpl::postTaskForModeToWorkerContext(
     PassOwnPtr<ScriptExecutionContext::Task> task, const String& mode)
 {
     m_workerThread->runLoop().postTaskForMode(task, mode);
+    return true;
 }
 
 
index b98a2c9..4041592 100644 (file)
@@ -86,7 +86,7 @@ public:
 
     // WebCore::WorkerLoaderProxy methods:
     virtual void postTaskToLoader(PassOwnPtr<WebCore::ScriptExecutionContext::Task>);
-    virtual void postTaskForModeToWorkerContext(
+    virtual bool postTaskForModeToWorkerContext(
         PassOwnPtr<WebCore::ScriptExecutionContext::Task>, const WTF::String& mode);
 
     // WebFrameClient methods to support resource loading thru the 'shadow page'.
index a69f01a..4481ac9 100644 (file)
@@ -149,9 +149,10 @@ void WebWorkerClientImpl::postTaskToLoader(PassOwnPtr<ScriptExecutionContext::Ta
     m_proxy->postTaskToLoader(task);
 }
 
-void WebWorkerClientImpl::postTaskForModeToWorkerContext(PassOwnPtr<ScriptExecutionContext::Task> task, const String& mode)
+bool WebWorkerClientImpl::postTaskForModeToWorkerContext(PassOwnPtr<ScriptExecutionContext::Task> task, const String& mode)
 {
     m_proxy->postTaskForModeToWorkerContext(task, mode);
+    return true;
 }
 
 void WebWorkerClientImpl::postMessageToWorkerObject(PassRefPtr<SerializedScriptValue> value, PassOwnPtr<MessagePortChannelArray> ports)
index d47744c..2a86548 100644 (file)
@@ -86,7 +86,7 @@ public:
 #endif
     // WebCore::WorkerLoaderProxy methods:
     virtual void postTaskToLoader(PassOwnPtr<WebCore::ScriptExecutionContext::Task>) OVERRIDE;
-    virtual void postTaskForModeToWorkerContext(PassOwnPtr<WebCore::ScriptExecutionContext::Task>, const String& mode) OVERRIDE;
+    virtual bool postTaskForModeToWorkerContext(PassOwnPtr<WebCore::ScriptExecutionContext::Task>, const String& mode) OVERRIDE;
 
     // WebCore::WorkerObjectProxy methods:
     virtual void postMessageToWorkerObject(PassRefPtr<WebCore::SerializedScriptValue>, PassOwnPtr<WebCore::MessagePortChannelArray>) OVERRIDE;