Move WorkerThreadable internal classes to Oilpan heap

third_party/WebKit/Source/core/loader/WorkerThreadableLoader.cpp

Index: third_party/WebKit/Source/core/loader/WorkerThreadableLoader.cpp
diff --git a/third_party/WebKit/Source/core/loader/WorkerThreadableLoader.cpp b/third_party/WebKit/Source/core/loader/WorkerThreadableLoader.cpp
index 3f1bb858dc5ace992f3aba7a13eb26f7da56d4cb..6bebf90f8b9b25eadd57ca51019ed798edf64be0 100644
--- a/third_party/WebKit/Source/core/loader/WorkerThreadableLoader.cpp
+++ b/third_party/WebKit/Source/core/loader/WorkerThreadableLoader.cpp
@@ -36,38 +36,38 @@
 #include "core/timing/WorkerGlobalScopePerformance.h"
 #include "core/workers/WorkerGlobalScope.h"
 #include "core/workers/WorkerLoaderProxy.h"
-#include "core/workers/WorkerThread.h"
 #include "platform/CrossThreadFunctional.h"
-#include "platform/WaitableEvent.h"
 #include "platform/heap/SafePoint.h"
 #include "platform/network/ResourceError.h"
 #include "platform/network/ResourceRequest.h"
 #include "platform/network/ResourceResponse.h"
 #include "platform/network/ResourceTimingInfo.h"
 #include "platform/weborigin/SecurityPolicy.h"
-#include "public/platform/Platform.h"
-#include "wtf/PtrUtil.h"
-#include "wtf/Vector.h"
 #include <memory>
 
 namespace blink {
 
-static std::unique_ptr<Vector<char>> createVectorFromMemoryRegion(const char* data, unsigned dataLength)
+namespace {
+
+std::unique_ptr<Vector<char>> createVectorFromMemoryRegion(const char* data, unsigned dataLength)
 {
     std::unique_ptr<Vector<char>> buffer = wrapUnique(new Vector<char>(dataLength));
     memcpy(buffer->data(), data, dataLength);
     return buffer;
 }
 
-WorkerThreadableLoader::WorkerThreadableLoader(WorkerGlobalScope& workerGlobalScope, ThreadableLoaderClient* client, const ThreadableLoaderOptions& options, const ResourceLoaderOptions& resourceLoaderOptions, BlockingBehavior blockingBehavior)
+} // namespace
+
+WorkerThreadableLoader::WorkerThreadableLoader(
+    WorkerGlobalScope& workerGlobalScope,
+    ThreadableLoaderClient* client,
+    const ThreadableLoaderOptions& options,
+    const ResourceLoaderOptions& resourceLoaderOptions,
+    BlockingBehavior blockingBehavior)
     : m_workerGlobalScope(&workerGlobalScope)
     , m_workerClientWrapper(new ThreadableLoaderClientWrapper(workerGlobalScope, client))
+    , m_bridge(new Bridge(m_workerClientWrapper, workerGlobalScope.thread()->workerLoaderProxy(), options, resourceLoaderOptions, blockingBehavior))
 {
-    if (blockingBehavior == LoadAsynchronously) {
-        m_bridge = new MainThreadAsyncBridge(workerGlobalScope, m_workerClientWrapper, options, resourceLoaderOptions);
-    } else {
-        m_bridge = new MainThreadSyncBridge(workerGlobalScope, m_workerClientWrapper, options, resourceLoaderOptions);
-    }
 }
 
 void WorkerThreadableLoader::loadResourceSynchronously(WorkerGlobalScope& workerGlobalScope, const ResourceRequest& request, ThreadableLoaderClient& client, const ThreadableLoaderOptions& options, const ResourceLoaderOptions& resourceLoaderOptions)
@@ -79,7 +79,6 @@ void WorkerThreadableLoader::loadResourceSynchronously(WorkerGlobalScope& worker
 WorkerThreadableLoader::~WorkerThreadableLoader()

[haraken, 2016/07/21 10:22:29]

Is it guaranteed that WorkerThreadableLoader gets destroyed outside Oilpan's GC?
Otherwise, it would be problematic to touch on-heap objects in
m_bridge->destroy().

[yhirano, 2016/07/21 10:56:09]

No, it's not guaranteed. I thought that calling destroy is safe because m_bridge
is held as Persistent and hence is never unreachable here. Is my understanding
wrong?

 {
     m_bridge->destroy();
-    m_bridge = nullptr;
 }
 
 void WorkerThreadableLoader::start(const ResourceRequest& request)
@@ -92,279 +91,380 @@ void WorkerThreadableLoader::start(const ResourceRequest& request)
 
 void WorkerThreadableLoader::overrideTimeout(unsigned long timeoutMilliseconds)
 {
-    ASSERT(m_bridge);
     m_bridge->overrideTimeout(timeoutMilliseconds);
 }
 
 void WorkerThreadableLoader::cancel()
 {
-    ASSERT(m_bridge);
     m_bridge->cancel();
 }
 
-WorkerThreadableLoader::MainThreadBridgeBase::MainThreadBridgeBase(
-    ThreadableLoaderClientWrapper* workerClientWrapper,
-    PassRefPtr<WorkerLoaderProxy> loaderProxy)
-    : m_workerClientWrapper(workerClientWrapper)
-    , m_loaderProxy(loaderProxy)
+WorkerThreadableLoader::AsyncTaskForwarder::AsyncTaskForwarder(PassRefPtr<WorkerLoaderProxy> loaderProxy)
+    : m_loaderProxy(loaderProxy)
 {
-    ASSERT(m_workerClientWrapper.get());
-    ASSERT(m_loaderProxy.get());
+    DCHECK(isMainThread());
 }
 
-WorkerThreadableLoader::MainThreadBridgeBase::~MainThreadBridgeBase()
+WorkerThreadableLoader::AsyncTaskForwarder::~AsyncTaskForwarder()
 {
+    DCHECK(isMainThread());
 }
 
-void WorkerThreadableLoader::MainThreadBridgeBase::mainThreadCreateLoader(ThreadableLoaderOptions options, ResourceLoaderOptions resourceLoaderOptions, ExecutionContext* context)
+void WorkerThreadableLoader::AsyncTaskForwarder::forwardTask(std::unique_ptr<ExecutionContextTask> task)
 {
-    ASSERT(isMainThread());
-    Document* document = toDocument(context);
-
-    resourceLoaderOptions.requestInitiatorContext = WorkerContext;
-    m_mainThreadLoader = DocumentThreadableLoader::create(*document, this, options, resourceLoaderOptions);
-    ASSERT(m_mainThreadLoader);
+    DCHECK(isMainThread());
+    m_loaderProxy->postTaskToWorkerGlobalScope(std::move(task));
 }
 
-void WorkerThreadableLoader::MainThreadBridgeBase::mainThreadStart(std::unique_ptr<CrossThreadResourceRequestData> requestData)
+void WorkerThreadableLoader::AsyncTaskForwarder::forwardTaskWithDoneSignal(std::unique_ptr<ExecutionContextTask> task)
 {
-    ASSERT(isMainThread());
-    ASSERT(m_mainThreadLoader);
-    m_mainThreadLoader->start(ResourceRequest(requestData.get()));
+    DCHECK(isMainThread());
+    m_loaderProxy->postTaskToWorkerGlobalScope(std::move(task));
 }
 
-void WorkerThreadableLoader::MainThreadBridgeBase::createLoaderInMainThread(const ThreadableLoaderOptions& options, const ResourceLoaderOptions& resourceLoaderOptions)
+void WorkerThreadableLoader::AsyncTaskForwarder::abort()
 {
-    m_loaderProxy->postTaskToLoader(createCrossThreadTask(&MainThreadBridgeBase::mainThreadCreateLoader, crossThreadUnretained(this), options, resourceLoaderOptions));
+    DCHECK(isMainThread());
 }
 
-void WorkerThreadableLoader::MainThreadBridgeBase::startInMainThread(const ResourceRequest& request, const WorkerGlobalScope& workerGlobalScope)
+WorkerThreadableLoader::WaitableEventWithTasks::~WaitableEventWithTasks() {}
+
+void WorkerThreadableLoader::WaitableEventWithTasks::signal()
 {
-    loaderProxy()->postTaskToLoader(createCrossThreadTask(&MainThreadBridgeBase::mainThreadStart, crossThreadUnretained(this), request));
+    m_event.signal();
 }
 
-void WorkerThreadableLoader::MainThreadBridgeBase::mainThreadDestroy(ExecutionContext* context)
+void WorkerThreadableLoader::WaitableEventWithTasks::wait()
 {
-    ASSERT(isMainThread());
-    ASSERT_UNUSED(context, context->isDocument());
-    delete this;
+    m_event.wait();

[haraken, 2016/07/21 10:22:29]

Don't we need to add:

  SafePointScope scope(BlinkGC::HeapPointersOnStack);

?

[yhirano, 2016/07/21 10:56:08]

I placed at the call site. Do you think here is the better place?

 }
 
-void WorkerThreadableLoader::MainThreadBridgeBase::destroy()
+Vector<std::unique_ptr<ExecutionContextTask>> WorkerThreadableLoader::WaitableEventWithTasks::take()
 {
-    // Ensure that no more client callbacks are done in the worker context's
-    // thread.
-    // ThreadableLoaderClientWrapper is an on-heap class and this function can
-    // be called in the finalization step but it is safe because
-    // m_workerClientWrapper is a CrossThreadPersistent.
-    m_workerClientWrapper->clearClient();
-
-    // "delete this" and m_mainThreadLoader::deref() on the worker object's
-    // thread.
-    m_loaderProxy->postTaskToLoader(createCrossThreadTask(&MainThreadBridgeBase::mainThreadDestroy, crossThreadUnretained(this)));
+    return std::move(m_tasks);
 }
 
-void WorkerThreadableLoader::MainThreadBridgeBase::mainThreadOverrideTimeout(unsigned long timeoutMilliseconds, ExecutionContext* context)
+void WorkerThreadableLoader::WaitableEventWithTasks::append(std::unique_ptr<ExecutionContextTask> task)
 {
-    ASSERT(isMainThread());
-    ASSERT_UNUSED(context, context->isDocument());
-
-    if (!m_mainThreadLoader)
-        return;
-    m_mainThreadLoader->overrideTimeout(timeoutMilliseconds);
+    m_tasks.append(std::move(task));
 }
 
-void WorkerThreadableLoader::MainThreadBridgeBase::overrideTimeout(unsigned long timeoutMilliseconds)
+WorkerThreadableLoader::SyncTaskForwarder::SyncTaskForwarder(PassRefPtr<WaitableEventWithTasks> eventWithTasks)
+    : m_eventWithTasks(eventWithTasks)
 {
-    m_loaderProxy->postTaskToLoader(createCrossThreadTask(&MainThreadBridgeBase::mainThreadOverrideTimeout, crossThreadUnretained(this), timeoutMilliseconds));
+    DCHECK(isMainThread());
 }
 
-void WorkerThreadableLoader::MainThreadBridgeBase::mainThreadCancel(ExecutionContext* context)
+WorkerThreadableLoader::SyncTaskForwarder::~SyncTaskForwarder()
 {
-    ASSERT(isMainThread());
-    ASSERT_UNUSED(context, context->isDocument());
-
-    if (!m_mainThreadLoader)
-        return;
-    m_mainThreadLoader->cancel();
-    m_mainThreadLoader = nullptr;
+    DCHECK(isMainThread());
 }
 
-void WorkerThreadableLoader::MainThreadBridgeBase::cancel()
+void WorkerThreadableLoader::SyncTaskForwarder::forwardTask(std::unique_ptr<ExecutionContextTask> task)
 {
-    m_loaderProxy->postTaskToLoader(createCrossThreadTask(&MainThreadBridgeBase::mainThreadCancel, crossThreadUnretained(this)));
-    ThreadableLoaderClientWrapper* clientWrapper = m_workerClientWrapper;
-    if (!clientWrapper->done()) {
-        // If the client hasn't reached a termination state, then transition it
-        // by sending a cancellation error.
-        // Note: no more client callbacks will be done after this method -- the
-        // m_workerClientWrapper->clearClient() call ensures that.
-        ResourceError error(String(), 0, String(), String());
-        error.setIsCancellation(true);
-        clientWrapper->didFail(error);
-    }
-    // |this| might be already destructed here because didFail() might
-    // clear a reference to ThreadableLoader, which might destruct
-    // WorkerThreadableLoader and then MainThreadBridge.
-    // Therefore we call clearClient() directly, rather than calling
-    // this->m_workerClientWrapper->clearClient().
-    clientWrapper->clearClient();
+    DCHECK(isMainThread());
+    m_eventWithTasks->append(std::move(task));
 }
 
-void WorkerThreadableLoader::MainThreadBridgeBase::didSendData(unsigned long long bytesSent, unsigned long long totalBytesToBeSent)
+void WorkerThreadableLoader::SyncTaskForwarder::forwardTaskWithDoneSignal(std::unique_ptr<ExecutionContextTask> task)
 {
-    forwardTaskToWorker(createCrossThreadTask(&ThreadableLoaderClientWrapper::didSendData, m_workerClientWrapper, bytesSent, totalBytesToBeSent));
+    DCHECK(isMainThread());
+    m_eventWithTasks->append(std::move(task));
+    m_eventWithTasks->signal();
 }
 
-void WorkerThreadableLoader::MainThreadBridgeBase::didReceiveResponse(unsigned long identifier, const ResourceResponse& response, std::unique_ptr<WebDataConsumerHandle> handle)
+void WorkerThreadableLoader::SyncTaskForwarder::abort()
 {
-    forwardTaskToWorker(createCrossThreadTask(&ThreadableLoaderClientWrapper::didReceiveResponse, m_workerClientWrapper, identifier, response, passed(std::move(handle))));
+    DCHECK(isMainThread());
+    m_eventWithTasks->setIsAborted();
+    m_eventWithTasks->signal();
 }
 
-void WorkerThreadableLoader::MainThreadBridgeBase::didReceiveData(const char* data, unsigned dataLength)
+WorkerThreadableLoader::Bridge::Bridge(
+    ThreadableLoaderClientWrapper* clientWrapper,
+    PassRefPtr<WorkerLoaderProxy> loaderProxy,
+    const ThreadableLoaderOptions& threadableLoaderOptions,
+    const ResourceLoaderOptions& resourceLoaderOptions,
+    BlockingBehavior blockingBehavior)
+    : m_clientWrapper(clientWrapper)
+    , m_loaderProxy(loaderProxy)
+    , m_threadableLoaderOptions(threadableLoaderOptions)
+    , m_resourceLoaderOptions(resourceLoaderOptions)
+    , m_blockingBehavior(blockingBehavior)
 {
-    forwardTaskToWorker(createCrossThreadTask(&ThreadableLoaderClientWrapper::didReceiveData, m_workerClientWrapper, passed(createVectorFromMemoryRegion(data, dataLength))));
+    DCHECK(!isMainThread());
 }
 
-void WorkerThreadableLoader::MainThreadBridgeBase::didDownloadData(int dataLength)
+WorkerThreadableLoader::Bridge::~Bridge()
 {
-    forwardTaskToWorker(createCrossThreadTask(&ThreadableLoaderClientWrapper::didDownloadData, m_workerClientWrapper, dataLength));
+    DCHECK(!isMainThread());
 }

[haraken, 2016/07/21 10:22:29]

Can we add DCHECK(!m_peer)?

[yhirano, 2016/07/21 11:44:55]

Done.

[yhirano, 2016/07/21 15:06:27]

memo: this breaks a test because didStart may be called after destroy. I will
fix this tomorrow.

 
-void WorkerThreadableLoader::MainThreadBridgeBase::didReceiveCachedMetadata(const char* data, int dataLength)
+void WorkerThreadableLoader::Bridge::start(const ResourceRequest& request, const WorkerGlobalScope& workerGlobalScope)
 {
-    forwardTaskToWorker(createCrossThreadTask(&ThreadableLoaderClientWrapper::didReceiveCachedMetadata, m_workerClientWrapper, passed(createVectorFromMemoryRegion(data, dataLength))));
+    DCHECK(!isMainThread());
+    RefPtr<WaitableEventWithTasks> eventWithTasks;
+    if (m_blockingBehavior == LoadSynchronously)
+        eventWithTasks = WaitableEventWithTasks::create();
+
+    m_loaderProxy->postTaskToLoader(createCrossThreadTask(
+        &Peer::createAndStart,
+        CrossThreadPersistent<Bridge>(this),
+        m_loaderProxy,
+        CrossThreadPersistent<WorkerThreadLifecycleContext>(workerGlobalScope.thread()->getWorkerThreadLifecycleContext()),
+        request,
+        threadableLoaderOptions(),
+        resourceLoaderOptions(),
+        eventWithTasks));
+
+    if (m_blockingBehavior == LoadAsynchronously)
+        return;
+
+    {
+        SafePointScope scope(BlinkGC::HeapPointersOnStack);
+        eventWithTasks->wait();
+    }
+
+    if (eventWithTasks->isAborted()) {
+        // This thread is going to terminate.
+        cancel();
+        return;
+    }
+
+    for (const auto& task : eventWithTasks->take()) {
+        // m_clientTask contains only CallClosureTasks. So, it's ok to pass
+        // the nullptr.
+        task->performTask(nullptr);
+    }
 }
 
-void WorkerThreadableLoader::MainThreadBridgeBase::didFinishLoading(unsigned long identifier, double finishTime)
+void WorkerThreadableLoader::Bridge::overrideTimeout(unsigned long timeoutMilliseconds)
 {
-    forwardTaskToWorkerOnLoaderDone(createCrossThreadTask(&ThreadableLoaderClientWrapper::didFinishLoading, m_workerClientWrapper, identifier, finishTime));
+    DCHECK(!isMainThread());
+    if (!m_peer)
+        return;
+    m_loaderProxy->postTaskToLoader(createCrossThreadTask(&Peer::overrideTimeout, m_peer, timeoutMilliseconds));
 }
 
-void WorkerThreadableLoader::MainThreadBridgeBase::didFail(const ResourceError& error)
+void WorkerThreadableLoader::Bridge::cancel()
 {
-    forwardTaskToWorkerOnLoaderDone(createCrossThreadTask(&ThreadableLoaderClientWrapper::didFail, m_workerClientWrapper, error));
+    DCHECK(!isMainThread());
+    if (!m_peer)
+        return;
+    m_loaderProxy->postTaskToLoader(createCrossThreadTask(&Peer::cancel, m_peer));
+    m_peer = nullptr;
+
+    auto clientWrapper = m_clientWrapper.get();
+
+    if (clientWrapper->done())
+        return;
+    // If the client hasn't reached a termination state, then transition it
+    // by sending a cancellation error.
+    // Note: no more client callbacks will be done after this method -- the
+    // clearClient() call ensures that.
+    ResourceError error(String(), 0, String(), String());
+    error.setIsCancellation(true);
+    clientWrapper->didFail(error);
+    clientWrapper->clearClient();
 }
 
-void WorkerThreadableLoader::MainThreadBridgeBase::didFailAccessControlCheck(const ResourceError& error)
+void WorkerThreadableLoader::Bridge::destroy()
 {
-    forwardTaskToWorkerOnLoaderDone(createCrossThreadTask(&ThreadableLoaderClientWrapper::didFailAccessControlCheck, m_workerClientWrapper, error));
+    DCHECK(!isMainThread());
+    m_clientWrapper->clearClient();
+    if (!m_peer)
+        return;
+    m_loaderProxy->postTaskToLoader(createCrossThreadTask(&Peer::cancel, m_peer));
+    m_peer = nullptr;
 }
 
-void WorkerThreadableLoader::MainThreadBridgeBase::didFailRedirectCheck()
+void WorkerThreadableLoader::Bridge::didStart(Peer* peer)
 {
-    forwardTaskToWorkerOnLoaderDone(createCrossThreadTask(&ThreadableLoaderClientWrapper::didFailRedirectCheck, m_workerClientWrapper));
+    DCHECK(!isMainThread());
+    DCHECK(!m_peer);
+    DCHECK(peer);
+    m_peer = peer;
 }
 
-void WorkerThreadableLoader::MainThreadBridgeBase::didReceiveResourceTiming(const ResourceTimingInfo& info)
+DEFINE_TRACE(WorkerThreadableLoader::Bridge)
 {
-    forwardTaskToWorker(createCrossThreadTask(&ThreadableLoaderClientWrapper::didReceiveResourceTiming, m_workerClientWrapper, info));
+    visitor->trace(m_clientWrapper);
 }
 
-WorkerThreadableLoader::MainThreadAsyncBridge::MainThreadAsyncBridge(
-    WorkerGlobalScope& workerGlobalScope,
-    ThreadableLoaderClientWrapper* workerClientWrapper,
+void WorkerThreadableLoader::Peer::createAndStart(
+    Bridge* bridge,
+    PassRefPtr<WorkerLoaderProxy> passLoaderProxy,
+    WorkerThreadLifecycleContext* workerThreadLifecycleContext,
+    std::unique_ptr<CrossThreadResourceRequestData> request,
     const ThreadableLoaderOptions& options,
-    const ResourceLoaderOptions& resourceLoaderOptions)
-    : MainThreadBridgeBase(workerClientWrapper, workerGlobalScope.thread()->workerLoaderProxy())
-{
-    createLoaderInMainThread(options, resourceLoaderOptions);
+    const ResourceLoaderOptions& resourceLoaderOptions,
+    PassRefPtr<WaitableEventWithTasks> eventWithTasks,
+    ExecutionContext* executionContext)
+{
+    DCHECK(isMainThread());
+    TaskForwarder* forwarder;
+    RefPtr<WorkerLoaderProxy> loaderProxy = passLoaderProxy;
+    if (eventWithTasks)
+        forwarder = new SyncTaskForwarder(eventWithTasks);
+    else
+        forwarder = new AsyncTaskForwarder(loaderProxy);
+
+    Peer* peer = new Peer(forwarder, workerThreadLifecycleContext);
+    if (peer->wasContextDestroyedBeforeObserverCreation()) {
+        // The thread is already terminating.
+        forwarder->abort();
+        return;
+    }
+    peer->m_clientWrapper = bridge->clientWrapper();
+    peer->start(*toDocument(executionContext), std::move(request), options, resourceLoaderOptions);
+    forwarder->forwardTask(createCrossThreadTask(&Bridge::didStart, CrossThreadPersistent<Bridge>(bridge), CrossThreadPersistent<Peer>(peer)));
 }
 
-void WorkerThreadableLoader::MainThreadAsyncBridge::start(const ResourceRequest& request, const WorkerGlobalScope& workerGlobalScope)
+WorkerThreadableLoader::Peer::~Peer()
 {
-    startInMainThread(request, workerGlobalScope);
+    DCHECK(isMainThread());
+    DCHECK(!m_mainThreadLoader);
 }
 
-WorkerThreadableLoader::MainThreadAsyncBridge::~MainThreadAsyncBridge()
+void WorkerThreadableLoader::Peer::overrideTimeout(unsigned long timeoutMilliseconds)
 {
+    DCHECK(isMainThread());
+    if (!m_mainThreadLoader)
+        return;
+    m_mainThreadLoader->overrideTimeout(timeoutMilliseconds);
 }
 
-void WorkerThreadableLoader::MainThreadAsyncBridge::forwardTaskToWorker(std::unique_ptr<ExecutionContextTask> task)
+void WorkerThreadableLoader::Peer::cancel()
 {

[haraken, 2016/07/21 10:22:29]

Don't we need to call:

  m_forwarder->abort();
  m_clientWrapper = nullptr;

in cancel()?

[yhirano, 2016/07/21 10:56:09]

We don't need to call them when this function is called from Bridge::cancel.
Peer::contextDestroyed also calls this function but it calls them by itself.

-    loaderProxy()->postTaskToWorkerGlobalScope(std::move(task));
+    DCHECK(isMainThread());
+    if (!m_mainThreadLoader)
+        return;
+    m_mainThreadLoader->cancel();
+    m_mainThreadLoader = nullptr;
 }
 
-void WorkerThreadableLoader::MainThreadAsyncBridge::forwardTaskToWorkerOnLoaderDone(std::unique_ptr<ExecutionContextTask> task)
+void WorkerThreadableLoader::Peer::didSendData(unsigned long long bytesSent, unsigned long long totalBytesToBeSent)
 {
-    loaderProxy()->postTaskToWorkerGlobalScope(std::move(task));
+    DCHECK(isMainThread());
+    CrossThreadPersistent<ThreadableLoaderClientWrapper> clientWrapper = m_clientWrapper.get();

[haraken, 2016/07/21 10:22:29]

Shall we use wrapCrossThreadPersistent()?

The same comment for other parts in this file.

[yhirano, 2016/07/21 11:44:55]

Let me confirm: Are you suggesting

(1)  {
    DCHECK(isMainThread());
    CrossThreadPersistent<ThreadableLoaderClientWrapper> clientWrapper =
        wrapCrossThreadPersistent(m_clientWrapper.get());
    if (!clientWrapper)
        return;
    forwardTask(createCrossThreadTask(
        ..., clientWrapper, ...));
}

or 

(2) {
    DCHECK(isMainThread());
    forwardTask(createCrossThreadTask(
        ..., wrapCrossThreadPersistent(m_clientWrapper.get()), ...));
}

?

(1) doens't have a merit, and (2) may bind nullptr for the task as
m_clientWrapper is a weak pointer.

[haraken, 2016/07/21 15:56:14]

I was suggesting 2).

if (!m_clientWrapper)
  return;
createCrossThreadTask(..., wrapCrossThreadPersistent(m_clientWrapper.get())

[yhirano, 2016/07/22 04:42:14]

The pattern is not safe as m_clientWrapper is weak. We need to hold a strong
pointer before checking the nullity.

+    if (!clientWrapper)
+        return;
+    m_forwarder->forwardTask(createCrossThreadTask(&ThreadableLoaderClientWrapper::didSendData, clientWrapper, bytesSent, totalBytesToBeSent));
 }
 
-WorkerThreadableLoader::MainThreadSyncBridge::MainThreadSyncBridge(
-    WorkerGlobalScope& workerGlobalScope,
-    ThreadableLoaderClientWrapper* workerClientWrapper,
-    const ThreadableLoaderOptions& options,
-    const ResourceLoaderOptions& resourceLoaderOptions)
-    : MainThreadBridgeBase(workerClientWrapper, workerGlobalScope.thread()->workerLoaderProxy())
-    , m_done(false)
+void WorkerThreadableLoader::Peer::didReceiveResponse(unsigned long identifier, const ResourceResponse& response, std::unique_ptr<WebDataConsumerHandle> handle)
 {
-    createLoaderInMainThread(options, resourceLoaderOptions);
+    DCHECK(isMainThread());
+    CrossThreadPersistent<ThreadableLoaderClientWrapper> clientWrapper = m_clientWrapper.get();
+    if (!clientWrapper)
+        return;
+    m_forwarder->forwardTask(createCrossThreadTask(&ThreadableLoaderClientWrapper::didReceiveResponse, clientWrapper, identifier, response, passed(std::move(handle))));
 }
 
-void WorkerThreadableLoader::MainThreadSyncBridge::start(const ResourceRequest& request, const WorkerGlobalScope& workerGlobalScope)
+void WorkerThreadableLoader::Peer::didReceiveData(const char* data, unsigned dataLength)
 {
-    WaitableEvent* terminationEvent = workerGlobalScope.thread()->terminationEvent();
-    m_loaderDoneEvent = wrapUnique(new WaitableEvent());
-
-    startInMainThread(request, workerGlobalScope);
+    DCHECK(isMainThread());
+    CrossThreadPersistent<ThreadableLoaderClientWrapper> clientWrapper = m_clientWrapper.get();
+    if (!clientWrapper)
+        return;
+    m_forwarder->forwardTask(createCrossThreadTask(&ThreadableLoaderClientWrapper::didReceiveData, clientWrapper, passed(createVectorFromMemoryRegion(data, dataLength))));
+}
 
-    size_t signaledIndex;
-    {
-        Vector<WaitableEvent*> events;
-        // Order is important; indicies are used later.
-        events.append(terminationEvent);
-        events.append(m_loaderDoneEvent.get());
+void WorkerThreadableLoader::Peer::didDownloadData(int dataLength)
+{
+    DCHECK(isMainThread());
+    CrossThreadPersistent<ThreadableLoaderClientWrapper> clientWrapper = m_clientWrapper.get();
+    if (!clientWrapper)
+        return;
+    m_forwarder->forwardTask(createCrossThreadTask(&ThreadableLoaderClientWrapper::didDownloadData, clientWrapper, dataLength));
+}
 
-        SafePointScope scope(BlinkGC::HeapPointersOnStack);
-        signaledIndex = WaitableEvent::waitMultiple(events);
-    }
-    // |signaledIndex| is 0; which is terminationEvent.
-    if (signaledIndex == 0) {
-        cancel();
+void WorkerThreadableLoader::Peer::didReceiveCachedMetadata(const char* data, int dataLength)
+{
+    DCHECK(isMainThread());
+    CrossThreadPersistent<ThreadableLoaderClientWrapper> clientWrapper = m_clientWrapper.get();
+    if (!clientWrapper)
         return;
-    }
+    m_forwarder->forwardTask(createCrossThreadTask(&ThreadableLoaderClientWrapper::didReceiveCachedMetadata, clientWrapper, passed(createVectorFromMemoryRegion(data, dataLength))));
+}
 
-    // The following code must be run only after |m_loaderDoneEvent| is
-    // signalled.
+void WorkerThreadableLoader::Peer::didFinishLoading(unsigned long identifier, double finishTime)
+{
+    DCHECK(isMainThread());
+    CrossThreadPersistent<ThreadableLoaderClientWrapper> clientWrapper = m_clientWrapper.get();
+    if (!clientWrapper)
+        return;
+    m_forwarder->forwardTaskWithDoneSignal(createCrossThreadTask(&ThreadableLoaderClientWrapper::didFinishLoading, clientWrapper, identifier, finishTime));
+}
 
-    Vector<std::unique_ptr<ExecutionContextTask>> tasks;
-    {
-        MutexLocker lock(m_lock);
-        ASSERT(m_done);
-        m_clientTasks.swap(tasks);
-    }
-    for (const auto& task : tasks) {
-        // m_clientTask contains only CallClosureTasks. So, it's ok to pass
-        // the nullptr.
-        task->performTask(nullptr);
-    }
+void WorkerThreadableLoader::Peer::didFail(const ResourceError& error)
+{
+    DCHECK(isMainThread());
+    CrossThreadPersistent<ThreadableLoaderClientWrapper> clientWrapper = m_clientWrapper.get();
+    if (!clientWrapper)
+        return;
+    m_forwarder->forwardTaskWithDoneSignal(createCrossThreadTask(&ThreadableLoaderClientWrapper::didFail, clientWrapper, error));
 }
 
-WorkerThreadableLoader::MainThreadSyncBridge::~MainThreadSyncBridge()
+void WorkerThreadableLoader::Peer::didFailAccessControlCheck(const ResourceError& error)
 {
-    ASSERT(isMainThread());
+    DCHECK(isMainThread());
+    CrossThreadPersistent<ThreadableLoaderClientWrapper> clientWrapper = m_clientWrapper.get();
+    if (!clientWrapper)
+        return;
+    m_forwarder->forwardTaskWithDoneSignal(createCrossThreadTask(&ThreadableLoaderClientWrapper::didFailAccessControlCheck, clientWrapper, error));
 }
 
-void WorkerThreadableLoader::MainThreadSyncBridge::forwardTaskToWorker(std::unique_ptr<ExecutionContextTask> task)
+void WorkerThreadableLoader::Peer::didFailRedirectCheck()
 {
-    ASSERT(isMainThread());
+    DCHECK(isMainThread());
+    CrossThreadPersistent<ThreadableLoaderClientWrapper> clientWrapper = m_clientWrapper.get();
+    if (!clientWrapper)
+        return;
+    m_forwarder->forwardTaskWithDoneSignal(createCrossThreadTask(&ThreadableLoaderClientWrapper::didFailRedirectCheck, clientWrapper));
+}
 
-    MutexLocker lock(m_lock);
-    RELEASE_ASSERT(!m_done);
+void WorkerThreadableLoader::Peer::didReceiveResourceTiming(const ResourceTimingInfo& info)
+{
+    DCHECK(isMainThread());
+    CrossThreadPersistent<ThreadableLoaderClientWrapper> clientWrapper = m_clientWrapper.get();
+    if (!clientWrapper)
+        return;
+    m_forwarder->forwardTask(createCrossThreadTask(&ThreadableLoaderClientWrapper::didReceiveResourceTiming, clientWrapper, info));
+}
 
-    m_clientTasks.append(std::move(task));
+void WorkerThreadableLoader::Peer::contextDestroyed()
+{
+    DCHECK(isMainThread());
+    m_forwarder->abort();
+    m_clientWrapper = nullptr;
+    cancel();
 }
 
-void WorkerThreadableLoader::MainThreadSyncBridge::forwardTaskToWorkerOnLoaderDone(std::unique_ptr<ExecutionContextTask> task)
+DEFINE_TRACE(WorkerThreadableLoader::Peer)
 {
-    ASSERT(isMainThread());
+    visitor->trace(m_forwarder);
+    WorkerThreadLifecycleObserver::trace(visitor);
+}
 
-    MutexLocker lock(m_lock);
-    RELEASE_ASSERT(!m_done);
+WorkerThreadableLoader::Peer::Peer(TaskForwarder* forwarder, WorkerThreadLifecycleContext* context)
+    : WorkerThreadLifecycleObserver(context)
+    , m_forwarder(forwarder)
+{
+    DCHECK(isMainThread());
+}
 
-    m_clientTasks.append(std::move(task));
-    m_done = true;
-    m_loaderDoneEvent->signal();
+void WorkerThreadableLoader::Peer::start(
+    Document& document,
+    std::unique_ptr<CrossThreadResourceRequestData> request,
+    const ThreadableLoaderOptions& options,
+    const ResourceLoaderOptions& originalResourceLoaderOptions)
+{
+    DCHECK(isMainThread());
+    ResourceLoaderOptions resourceLoaderOptions = originalResourceLoaderOptions;
+    resourceLoaderOptions.requestInitiatorContext = WorkerContext;
+    m_mainThreadLoader = DocumentThreadableLoader::create(document, this, options, resourceLoaderOptions);
+    m_mainThreadLoader->start(ResourceRequest(request.get()));
 }
 
 } // namespace blink