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 "public/platform/Platform.h"
 
 namespace blink {
 
 namespace {
 
+// Tracks whether we have instantiated a BackingThreadHolder.
+static bool s_hasBackingThreadHolder = false;
+
 // 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.
+// renderrer process. BackingThreadHolst::m_thread is cleared by

[haraken, 2016/05/09 02:00:53]

renderer

[flackr, 2016/05/10 18:07:39]

Done.

+// 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);
+        s_hasBackingThreadHolder = true;
         return holder;
     }
 
     WorkerBackingThread* thread() { return m_thread.get(); }
+    static void clear()
+    {
+        // Calling instance() if it has not already been called would create
+        // a backing thread so check if we have an instance first.
+        if (s_hasBackingThreadHolder)

[haraken, 2016/05/09 02:00:53]

Just to confirm: The access to s_hasBackingThreadHolder doesn't cause threading
race because it's guaranteed that the BackingThreadHolder is instantiated before
any compositor thread starts running and the BackingThreadHolder is destructed
after all compositor threads finished, right?

If my understanding is correct, it would be better to make
s_hasBackingThreadHolder a member variable of BackingThreadHolder. Then the call
site can just call instance().clear().

If my understanding is wrong, you need to address the threading race.

[flackr, 2016/05/10 18:07:39]

Right.
The problem is that calling instance() creates the BackingThreadHolder (and
WorkerBackingThread) if we haven't yet. This means that during shutdown we would
unnecessarily create the WorkerBackingThread just to clear it. This is why
yhirano removed the call to clearSharedBackingThread() in
https://codereview.chromium.org/1897143002. The problem is that we can't call
ThreadState::detach until we are no longer referencing oilpan objects from the
compositor thread (CompositorMutatorImpl).

+            instance().m_thread = nullptr;
+    }
     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())) {}
 
@@ skipping 24 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