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

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

Index: third_party/WebKit/Source/modules/compositorworker/CompositorWorkerThread.cpp
diff --git a/third_party/WebKit/Source/modules/compositorworker/CompositorWorkerThread.cpp b/third_party/WebKit/Source/modules/compositorworker/CompositorWorkerThread.cpp
index 78f1ed6beb27b0a324c26fab5deda377eb353b22..1169ac5c25b2d69a359d28e2b607602ba2f882b0 100644
--- a/third_party/WebKit/Source/modules/compositorworker/CompositorWorkerThread.cpp
+++ b/third_party/WebKit/Source/modules/compositorworker/CompositorWorkerThread.cpp
@@ -12,6 +12,7 @@
 #include "modules/compositorworker/CompositorWorkerGlobalScope.h"
 #include "platform/ThreadSafeFunctional.h"
 #include "platform/TraceEvent.h"
+#include "platform/WaitableEvent.h"
 #include "public/platform/Platform.h"
 
 namespace blink {
@@ -19,33 +20,87 @@ namespace blink {
 namespace {
 
 // 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;
+        MutexLocker locker(holderInstanceMutex());
+        return *s_instance;
     }
 
-    WorkerBackingThread* thread() { return m_thread.get(); }
-    void resetForTest()
+    static void ensureInstance()
+    {
+        MutexLocker locker(holderInstanceMutex());

[kinuko, 2016/05/23 09:53:55]

I'd like to remove this lock if it shouldn't be needed.  The threading
assertions we have seem to be enough, or are there still other possibilities
that we could be stumbled on?

[flackr, 2016/05/25 16:42:52]

Given s_instance is cleared on main after a synchronous shutdown, I think
removing this is fine. Done.

+        if (!s_instance)
+            s_instance = new BackingThreadHolder;
+    }
+
+    static void clear()
     {
-        ASSERT(!m_thread || (m_thread->workerScriptCount() == 0));
-        m_thread = nullptr;
-        m_thread = WorkerBackingThread::createForTest(Platform::current()->compositorThread());
+        MutexLocker locker(holderInstanceMutex());
+        if (s_instance) {
+            s_instance->thread()->isolate()->TerminateExecution();
+            s_instance->shutdownAndWait();
+            delete s_instance;
+            s_instance = nullptr;
+        }
     }
 
+    static void createForTest()
+    {
+        MutexLocker locker(holderInstanceMutex());
+        DCHECK_EQ(nullptr, s_instance);
+        s_instance = new BackingThreadHolder(WorkerBackingThread::createForTest(Platform::current()->compositorThread()));

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

I didn't like how resetForTest swapped out the m_thread (which could have then
had the wrong workerBackingCount if the posted task had already run) and so I'd
have to check that the task to attach was still pending, so I decided we should
just pass the test WorkerBackingThread in to the constructor.

+    }
+
+    WorkerBackingThread* thread() { return m_thread.get(); }
+
 private:
-    BackingThreadHolder() : m_thread(WorkerBackingThread::create(Platform::current()->compositorThread())) {}
+    BackingThreadHolder(PassOwnPtr<WorkerBackingThread> useBackingThread = nullptr)
+        : m_thread(useBackingThread ? useBackingThread.release() : WorkerBackingThread::create(Platform::current()->compositorThread()))
+    {
+        DCHECK(isMainThread());
+        Platform::current()->compositorThread()->getWebTaskRunner()->postTask(BLINK_FROM_HERE, threadSafeBind(&BackingThreadHolder::initializeOnThread, AllowCrossThreadAccess(this)));
+    }
+
+    static Mutex& holderInstanceMutex()
+    {
+        DEFINE_THREAD_SAFE_STATIC_LOCAL(Mutex, holderMutex, new Mutex);
+        return holderMutex;
+    }
+
+    void initializeOnThread()
+    {
+        DCHECK_EQ(0u, m_thread->workerScriptCount()) << "BackingThreadHolder should be the first to attach to WorkerBackingThread";
+        m_thread->attach();
+    }
+
+    void shutdownAndWait()
+    {
+        DCHECK(isMainThread());
+        WaitableEvent doneEvent;
+        Platform::current()->compositorThread()->getWebTaskRunner()->postTask(BLINK_FROM_HERE, threadSafeBind(&BackingThreadHolder::shutdownOnThread, AllowCrossThreadAccess(this), AllowCrossThreadAccess(&doneEvent)));
+        doneEvent.wait();
+    }
+
+    void shutdownOnThread(WaitableEvent* doneEvent)
+    {
+        DCHECK_EQ(1u, m_thread->workerScriptCount()) << "BackingThreadHolder should be the last to detach from WorkerBackingThread";
+        m_thread->detach();
+        doneEvent->signal();
+    }
 
     OwnPtr<WorkerBackingThread> m_thread;
+
+    static BackingThreadHolder* s_instance;
 };
 
+BackingThreadHolder* BackingThreadHolder::s_instance = nullptr;
+
 } // namespace
 
 PassOwnPtr<CompositorWorkerThread> CompositorWorkerThread::create(PassRefPtr<WorkerLoaderProxy> workerLoaderProxy, InProcessWorkerObjectProxy& workerObjectProxy, double timeOrigin)
@@ -77,9 +132,21 @@ WorkerGlobalScope*CompositorWorkerThread::createWorkerGlobalScope(PassOwnPtr<Wor
     return CompositorWorkerGlobalScope::create(this, std::move(startupData), m_timeOrigin);
 }
 
-void CompositorWorkerThread::resetSharedBackingThreadForTest()
+void CompositorWorkerThread::ensureSharedBackingThread()
+{
+    DCHECK(isMainThread());
+    BackingThreadHolder::ensureInstance();
+}
+
+void CompositorWorkerThread::clearSharedBackingThread()
+{
+    DCHECK(isMainThread());
+    BackingThreadHolder::clear();
+}
+
+void CompositorWorkerThread::createSharedBackingThreadForTest()
 {
-    BackingThreadHolder::instance().resetForTest();
+    BackingThreadHolder::createForTest();
 }
 
 } // namespace blink