Reland "ThreadPool: PlatformNativeWorkerPoolWin should use CallbackMayRunLong for blocking tasks"

This is a reland of e9dec932f40e8d257f2ef3bf57ea8fa6d38d944f. The change
was initially reverted due to a crash encountered when two successive
ScopedBlockingCalls were observed. Successive ScopedBlockingCalls led to
CallbackMayRunLong being called twice on the same callback, which is not
expected by the API. To fix this issue, the observer is cleared after a
call to CallbackMayRunLong, ensuring that CallbackMayRunLong is called at
most once per callback.

Original change's description:
> ThreadPool: PlatformNativeWorkerPoolWin should use CallbackMayRunLong for blocking tasks
>
> The Windows Thread Pool API provides a CallbackMayRunLong method to
> indicate that a callback may not return quickly. The thread pool can use
> this information to better determine when a new thread should be
> created.
>
> This CL teaches PlatformNativeWorkerPoolWin about ScopedBlockingCall in
> order to determine whether a callback will return quickly. A
> BlockingObserver is registered before running a task in the work
> callback. If the task contains a ScopedBlockingCall, the observer is
> notified, and will call CallbackMayRunLong on the appropriate
> PTP_CALLBACK_INSTANCE.
>
> Bug: 756547
> Change-Id: If04d2c5e01b00960dc510e94ff89525036c2ae67
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1574229
> Commit-Queue: François Doray <fdoray@chromium.org>
> Reviewed-by: François Doray <fdoray@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#653252}

Bug: 756547
Change-Id: Ic7c1a93b1429b333fdd4d9a574e232d5c9bdcb2e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1582219
Reviewed-by: François Doray <fdoray@chromium.org>
Commit-Queue: Aditya Keerthi <adityakeerthi@google.com>
Cr-Commit-Position: refs/heads/master@{#654507}


CrOS-Libchrome-Original-Commit: 2f548f8df68d7626499a3f84fd1c0c3648910637

4 files changed, 28 insertions, 3 deletions

diff --git a/base/task/thread_pool/scheduler_worker_pool_unittest.cc b/base/task/thread_pool/scheduler_worker_pool_unittest.cc
index 5f5e7052e5..1f615b3876 100644
--- a/base/task/thread_pool/scheduler_worker_pool_unittest.cc
+++ b/base/task/thread_pool/scheduler_worker_pool_unittest.cc
@@ -437,6 +437,31 @@ TEST_P(ThreadPoolWorkerPoolTest, UpdatePriorityBestEffortToUserBlocking) {
   task_tracker_.FlushForTesting();
 }
 
+// Regression test for crbug.com/955953.
+TEST_P(ThreadPoolWorkerPoolTest, ScopedBlockingCallTwice) {
+  StartWorkerPool();
+  auto task_runner = test::CreateTaskRunnerWithExecutionMode(
+      GetParam().execution_mode, &mock_scheduler_task_runner_delegate_,
+      {MayBlock()});
+
+  WaitableEvent task_ran;
+  task_runner->PostTask(FROM_HERE,
+                        BindOnce(
+                            [](WaitableEvent* task_ran) {
+                              {
+                                ScopedBlockingCall scoped_blocking_call(
+                                    FROM_HERE, BlockingType::MAY_BLOCK);
+                              }
+                              {
+                                ScopedBlockingCall scoped_blocking_call(
+                                    FROM_HERE, BlockingType::MAY_BLOCK);
+                              }
+                              task_ran->Signal();
+                            },
+                            Unretained(&task_ran)));
+  task_ran.Wait();
+}
+
 #if defined(OS_WIN)
 TEST_P(ThreadPoolWorkerPoolTest, COMMTAWorkerEnvironment) {
   StartWorkerPool(SchedulerWorkerPool::WorkerEnvironment::COM_MTA);
diff --git a/base/threading/scoped_blocking_call.cc b/base/threading/scoped_blocking_call.cc
index f5fcad0aa9..a548e1224e 100644
--- a/base/threading/scoped_blocking_call.cc
+++ b/base/threading/scoped_blocking_call.cc
@@ -121,7 +121,7 @@ void SetBlockingObserverForCurrentThread(BlockingObserver* blocking_observer) {
   tls_blocking_observer.Get().Set(blocking_observer);
 }
 
-void ClearBlockingObserverForTesting() {
+void ClearBlockingObserverForCurrentThread() {
   tls_blocking_observer.Get().Set(nullptr);
 }
 
diff --git a/base/threading/scoped_blocking_call.h b/base/threading/scoped_blocking_call.h
index 9161f20fec..4b3666b85f 100644
--- a/base/threading/scoped_blocking_call.h
+++ b/base/threading/scoped_blocking_call.h
@@ -160,7 +160,7 @@ class BASE_EXPORT BlockingObserver {
 BASE_EXPORT void SetBlockingObserverForCurrentThread(
     BlockingObserver* blocking_observer);
 
-BASE_EXPORT void ClearBlockingObserverForTesting();
+BASE_EXPORT void ClearBlockingObserverForCurrentThread();
 
 // Unregisters the |blocking_observer| on the current thread within its scope.
 // Used in ThreadPool tests to prevent calls to //base sync primitives from
diff --git a/base/threading/scoped_blocking_call_unittest.cc b/base/threading/scoped_blocking_call_unittest.cc
index 3ddf07bb7f..4c703b5007 100644
--- a/base/threading/scoped_blocking_call_unittest.cc
+++ b/base/threading/scoped_blocking_call_unittest.cc
@@ -34,7 +34,7 @@ class ScopedBlockingCallTest : public testing::Test {
   }
 
   ~ScopedBlockingCallTest() override {
-    internal::ClearBlockingObserverForTesting();
+    internal::ClearBlockingObserverForCurrentThread();
   }
 
   testing::StrictMock<MockBlockingObserver> observer_;