Refactoring WorkletThreadBackingHolder with template pattern

third_party/WebKit/Source/modules/webaudio/AudioWorkletThread.cpp

 // Copyright 2016 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/webaudio/AudioWorkletThread.h"
 
 #include "core/workers/WorkerBackingThread.h"
 #include "core/workers/WorkerThreadStartupData.h"
 #include "core/workers/WorkletBackingThreadHolder.h"
 #include "modules/webaudio/AudioWorkletGlobalScope.h"
 #include "platform/CrossThreadFunctional.h"
 #include "platform/WaitableEvent.h"
 #include "platform/WebThreadSupportingGC.h"
 #include "platform/tracing/TraceEvent.h"
 #include "platform/weborigin/SecurityOrigin.h"
 #include "public/platform/Platform.h"
 #include "wtf/Assertions.h"
 #include "wtf/PtrUtil.h"
 #include <memory>
 
 namespace blink {
 
 namespace {
 
-// TODO(hongchan): consider refactoring static methods in this class into
-// a template class.
-class AudioWorkletThreadHolder final : public WorkletBackingThreadHolder {
+// AudioWorkletThreadHolder is a singletone container/mananger of a dedicated
+// thread for processing the AudioWorklet.
+class AudioWorkletThreadHolder final
+    : public WorkletThreadHolder<AudioWorkletThreadHolder> {

[nhiroki, 2016/10/19 01:36:49]

I'd prefer to avoid class inheritance as much as possible because WorkerThread
classes already have deep inheritance hierarchy. Instead of inheritance, how
about like this?

1) Make WorkletThreadHolder receive client's WorkerThread name
(AudioWorkletThread) as a template parameter instead of a derived class name
(AudioWorkletThreadHolder).

  WorkletThreadHolder<AudioWorkletThreadHolder>
    -> WorkletThreadHolder<AudioWorkletThread>

2) Define createWorkletThreadHolder() on AudioWorkletThread like this:

  static WorkletThreadHolder<AudioWorkletThread>*
AudioWorkletThread::createWorkletThreadHolder(std::unique_ptr<WorkerBackingThread>
backingThread = nullptr) { ... }

3) Make
WorkletThreadHolder<AudioWorkletThread>::ensureInstance()/createForTest() call
AudioWorkletThread::createWorkletThreadHolder() instead of the derived class's
ctor.

WDYT?

+  friend class WorkletThreadHolder<AudioWorkletThreadHolder>;
+
  public:
-  static AudioWorkletThreadHolder* instance() {
-    MutexLocker locker(holderInstanceMutex());
-    return s_instance;
-  }
-
-  static void ensureInstance() {
-    if (!s_instance)
-      s_instance = new AudioWorkletThreadHolder;
-  }
-
-  static void clear() {
-    MutexLocker locker(holderInstanceMutex());
-    if (s_instance) {
-      s_instance->shutdownAndWait();
-      delete s_instance;
-      s_instance = nullptr;
-    }
-  }
-
   static void createForTest() {
-    MutexLocker locker(holderInstanceMutex());
-    DCHECK_EQ(nullptr, s_instance);
-    s_instance =
-        new AudioWorkletThreadHolder(WorkerBackingThread::createForTest(
-            "AudioWorkletThread", BlinkGC::PerThreadHeapMode));
+    WorkletThreadHolder<AudioWorkletThreadHolder>::createForTest(
+        WorkerBackingThread::createForTest("AudioWorkletThread",
+                                           BlinkGC::PerThreadHeapMode));
   }
 
  private:
   AudioWorkletThreadHolder(
       std::unique_ptr<WorkerBackingThread> backingThread = nullptr)
-      : WorkletBackingThreadHolder(
-            backingThread
-                ? std::move(backingThread)
-                : WorkerBackingThread::create("AudioWorkletThread",
-                                              BlinkGC::PerThreadHeapMode)) {}
-
-  static Mutex& holderInstanceMutex() {
-    DEFINE_THREAD_SAFE_STATIC_LOCAL(Mutex, holderMutex, new Mutex);
-    return holderMutex;
-  }
-
-  void initializeOnThread() {
-    MutexLocker locker(holderInstanceMutex());
-    DCHECK(!m_initialized);
-    m_thread->initialize();
-    m_initialized = true;
-  }
-
-  static AudioWorkletThreadHolder* s_instance;
+      : WorkletThreadHolder(backingThread ? std::move(backingThread)
+                                          : WorkerBackingThread::create(
+                                                "AudioWorkletThread",
+                                                BlinkGC::PerThreadHeapMode)) {}
 };
 
-AudioWorkletThreadHolder* AudioWorkletThreadHolder::s_instance = nullptr;
-
 }  // namespace
 
 std::unique_ptr<AudioWorkletThread> AudioWorkletThread::create(
     PassRefPtr<WorkerLoaderProxy> workerLoaderProxy,
     WorkerReportingProxy& workerReportingProxy) {
   TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("audio-worklet"),
                "AudioWorkletThread::create");
   DCHECK(isMainThread());
   return wrapUnique(new AudioWorkletThread(std::move(workerLoaderProxy),
                                            workerReportingProxy));
@@ skipping 52 matching lines @@
     securityOrigin->transferPrivilegesFrom(
         std::move(startupData->m_starterOriginPrivilegeData));
   }
 
   return AudioWorkletGlobalScope::create(
       startupData->m_scriptURL, startupData->m_userAgent,
       securityOrigin.release(), this->isolate(), this);
 }
 
 }  // namespace blink