[TimeZoneMonitor] Decouple renderer side impl from content to blink.

third_party/WebKit/Source/core/workers/WorkerThread.cpp

 /*
  * Copyright (C) 2008 Apple Inc. All Rights Reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 302 matching lines @@
 WorkerInspectorController* WorkerThread::workerInspectorController() {
   DCHECK(isCurrentThread());
   return m_workerInspectorController.get();
 }
 
 unsigned WorkerThread::workerThreadCount() {
   MutexLocker lock(threadSetMutex());
   return workerThreads().size();
 }
 
+void WorkerThread::PostTaskToAllWorkerThreads(
+    const WebTraceLocation& location,
+    std::unique_ptr<ExecutionContextTask> task) {
+  MutexLocker lock(threadSetMutex());
+  HashSet<WorkerThread*> threads = workerThreads();
+
+  for (WorkerThread* thread : threads) {
+    if (thread->isOwningBackingThread())

[nhiroki, 2016/10/13 10:24:12]

isOwningBackingThread() returns true when this WorkerThread exclusively owns
WorkerBackingThread. In other words, if WorkerThread shares a
WorkerBackingThread with other WorkerThreads, this returns false. Therefore, a
task is never posted to WorkerBackingThread that is shared among multiple
WorkerThreads. I guess this behavior is different from your expectation.

Instead of isOwningBackingThread(), how about recording a pointer to
WorkerBackingThread and check it on each iteration like this?

WTF::HashSet<WorkerBackingThread*> posted;
for (WorkerThread* thread : threads) {
  if (posted.contains(&thread->workerBackingThread()))
    continue;
  thread->postTask(...);
  posted.add(&thread->workerBackingThread());
}

(NOTE: I have't checked this code snippet with a compiler, so this may need some
modification)

In addition, this filter seems to be specific to your usecase. It'd be more
common that a caller of postTask() doesn't take care of whether target
WorkerThreads share a WorkerBackingThread. So, how about making workerThreads()
public and filtering in TimeZoneMonitorClient::OnTimeZoneChange() instead? This
also enables to duplicate a task (see my other comment)

[leonhsl(Using Gerrit), 2016/10/14 03:12:00]

Oh,, I misunderstood that isOwningBackingThread() means the WorkerThread is the
*main* worker thread for a WorkerBackingThread. Thanks for the new filter
suggestion.
A question:
If we'll make workerThreads() public, then we must also make the mutext
threadSetMutex() public and enforce workerThreads()'s callers always lock the
mutex until they finished using the returned threads. This looks like somehow
dangerous..

[nhiroki, 2016/10/17 03:49:11]

Yeah..., this is so confusing.

To be more precise, "BackingThread" means a platform thread (e.g. Pthreads) here
and isOwningBackingThread() is used for checking whether it's necessary to
initialize/finalize the platform thread when WorkerBackingThread is
constructed/destructed. As my other comment[1], WorkerBackingThread can borrows
an existing platform thread (e.g. compositor thread, audio thread) instead of
creating a dedicated one. In the case, we don't have to initialize/finalize a
platform thread on construction/destruction, so the function returns false.

[1]
https://codereview.chromium.org/2402983002/diff/40001/third_party/WebKit/Sour...
Good point. I think the caller can use the returned threads without a lock if
they are synchronously used on the main thread, because the thread set is
updated only on the main thread.

To clarify that, we may want to have a comment on workerThreads() like this:

// Returns a set of all worker threads. This must be called only on the main
thread and the returned set must not be stored for future use.
HashSet<WorkerThread*>& WorkerThread::workerThreads();

[leonhsl(Using Gerrit), 2016/10/17 07:30:01]

Done. Thank you very much for sharing the details and the solution suggestion! 

+      thread->postTask(location, std::move(task));

[nhiroki, 2016/10/13 10:24:12]

|task| is moved here, so you can't use it on following iterations.

[leonhsl(Using Gerrit), 2016/10/14 03:12:00]

My bad.. Thanks!

+  }
+}
+
 PlatformThreadId WorkerThread::platformThreadId() {
   if (!m_requestedToStart)
     return 0;
   return workerBackingThread().backingThread().platformThread().threadId();
 }
 
 bool WorkerThread::isForciblyTerminated() {
   MutexLocker lock(m_threadStateMutex);
   switch (m_exitCode) {
     case ExitCode::NotTerminated:
@@ skipping 352 matching lines @@
   MutexLocker lock(m_threadStateMutex);
   return m_requestedToTerminate;
 }
 
 ExitCode WorkerThread::getExitCodeForTesting() {
   MutexLocker lock(m_threadStateMutex);
   return m_exitCode;
 }
 
 }  // namespace blink