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

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

Index: third_party/WebKit/Source/core/workers/WorkerThread.cpp
diff --git a/third_party/WebKit/Source/core/workers/WorkerThread.cpp b/third_party/WebKit/Source/core/workers/WorkerThread.cpp
index f03dfb3508ee3a50d82574df321b0017c5fc9281..3b4554bace4b3c9ab95ed8be1ea38e5816b43036 100644
--- a/third_party/WebKit/Source/core/workers/WorkerThread.cpp
+++ b/third_party/WebKit/Source/core/workers/WorkerThread.cpp
@@ -320,6 +320,18 @@ unsigned WorkerThread::workerThreadCount() {
   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;