compositor-worker: Keep worker backing thread alive for the lifetime of the compositor thread.

third_party/WebKit/Source/modules/compositorworker/CompositorWorkerThread.cpp

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "modules/compositorworker/CompositorWorkerThread.h"
 
 #include "bindings/core/v8/V8GCController.h"
 #include "bindings/core/v8/V8Initializer.h"
 #include "core/workers/InProcessWorkerObjectProxy.h"
 #include "core/workers/WorkerBackingThread.h"
 #include "core/workers/WorkerThreadStartupData.h"
 #include "modules/compositorworker/CompositorWorkerGlobalScope.h"
 #include "platform/ThreadSafeFunctional.h"
 #include "platform/TraceEvent.h"
+#include "platform/WaitableEvent.h"
 #include "public/platform/Platform.h"
 
 namespace blink {
 
 namespace {
 
+class BackingThreadHolder;
+static BackingThreadHolder* s_holderInstance = nullptr;
+
 // This is a singleton class holding the compositor worker thread in this
-// renderrer process. BackingThreadHolst::m_thread will never be cleared,
-// but Oilpan and V8 are detached from the thread when the last compositor
-// worker thread is gone.
+// renderer process. BackingThreadHolder::m_thread is cleared by
+// ModulesInitializer::shutdown.
 // See WorkerThread::terminateAndWaitForAllWorkers for the process shutdown
 // case.
 class BackingThreadHolder {
 public:
     static BackingThreadHolder& instance()
     {
-        DEFINE_THREAD_SAFE_STATIC_LOCAL(BackingThreadHolder, holder, new BackingThreadHolder);
-        return holder;
+        if (!s_holderInstance)

[yhirano, 2016/05/19 05:35:37]

You need a mutex.

[flackr, 2016/05/19 22:24:16]

Done. But are you sure? It seems to me like instance() and clear() are always
accessed from the main thread.

[haraken, 2016/05/20 00:24:44]

Yeah, as I commented on
https://codereview.chromium.org/1955693003/diff/1/third_party/WebKit/Source/m...,
I don't think the mutex is needed.

[yhirano, 2016/05/20 02:24:54]

instance() can be called from worker threads via
WorkerThread::workerBackingThread().
I'm a bit nervous because the native thread exists before the first call of
ensureSharedBackingThread.

With enough comments and assertions it might be OK to remove mutex.

[haraken, 2016/05/20 02:35:24]

If I'm understanding correctly, it *should not* happen because the main thread
must call ensureBackingThread before any compositor worker thread starts
running.

[flackr, 2016/05/20 15:51:18]

Correct, that is how it is supposed to work.

I see your point on the Mutex, it shouldn't ever happen that there is contention
calling instance() since we ensure the instance exists before the first worker
is created and the workers should be shut down before we shut down the instance
but can't hurt to be safe.

+            s_holderInstance = new BackingThreadHolder;
+        return *s_holderInstance;
+    }
+
+    static void clear()
+    {
+        if (s_holderInstance) {
+            s_holderInstance->shutdownAndWait();
+            delete s_holderInstance;
+            s_holderInstance = nullptr;
+        }
     }
 
     WorkerBackingThread* thread() { return m_thread.get(); }
+
     void resetForTest()
     {
         ASSERT(!m_thread || (m_thread->workerScriptCount() == 0));
-        m_thread = nullptr;
         m_thread = WorkerBackingThread::createForTest(Platform::current()->compositorThread());
     }
 
 private:
-    BackingThreadHolder() : m_thread(WorkerBackingThread::create(Platform::current()->compositorThread())) {}
+    BackingThreadHolder() : m_thread(WorkerBackingThread::create(Platform::current()->compositorThread()))
+    {
+        Platform::current()->compositorThread()->getWebTaskRunner()->postTask(BLINK_FROM_HERE, threadSafeBind(&BackingThreadHolder::initializeOnThread, AllowCrossThreadAccess(this)));
+    }
+
+    void initializeOnThread()

[yhirano, 2016/05/19 05:35:37]

DCHECK(0, m_thread->workerScriptCount());

[flackr, 2016/05/19 22:24:16]

Done.

+    {
+        m_thread->attach();
+    }
+
+    void shutdownAndWait()
+    {
+        WaitableEvent doneEvent;
+        Platform::current()->compositorThread()->getWebTaskRunner()->postTask(BLINK_FROM_HERE, threadSafeBind(&BackingThreadHolder::shutdownOnThread, AllowCrossThreadAccess(&instance()), AllowCrossThreadAccess(&doneEvent)));
+        doneEvent.wait();
+    }
+
+    void shutdownOnThread(WaitableEvent* doneEvent)

[yhirano, 2016/05/19 05:35:37]

DCHECK(1, m_thread->workerScriptCount());

[flackr, 2016/05/19 22:24:16]

Done.

+    {
+        m_thread->detach();
+        doneEvent->signal();
+    }
 
     OwnPtr<WorkerBackingThread> m_thread;
 };
 
 } // namespace
 
 PassOwnPtr<CompositorWorkerThread> CompositorWorkerThread::create(PassRefPtr<WorkerLoaderProxy> workerLoaderProxy, InProcessWorkerObjectProxy& workerObjectProxy, double timeOrigin)
 {
     TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("compositor-worker"), "CompositorWorkerThread::create");
     ASSERT(isMainThread());
@@ skipping 15 matching lines @@
 {
     return *BackingThreadHolder::instance().thread();
 }
 
 WorkerGlobalScope*CompositorWorkerThread::createWorkerGlobalScope(PassOwnPtr<WorkerThreadStartupData> startupData)
 {
     TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("compositor-worker"), "CompositorWorkerThread::createWorkerGlobalScope");
     return CompositorWorkerGlobalScope::create(this, std::move(startupData), m_timeOrigin);
 }
 
+void CompositorWorkerThread::ensureSharedBackingThread()
+{
+    BackingThreadHolder::instance();
+}
+
+void CompositorWorkerThread::clearSharedBackingThread()
+{
+    BackingThreadHolder::clear();
+}
+
 void CompositorWorkerThread::resetSharedBackingThreadForTest()
 {
     BackingThreadHolder::instance().resetForTest();
 }
 
 } // namespace blink