[PowerMonitor] Fix threads racing in PowerMonitorTest

Ensure PowerMonitorTest:: data members all be accessed only on the UI
thread.

Bug: 1127374, 1126315
Change-Id: I9c41bb7c4cd451f84732b128f80e14db40d6bcaf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2465190Reviewed-by: Ken Rockot <rockot@google.com>
Reviewed-by: Charlie Reis <creis@chromium.org>
Commit-Queue: Leon Han <leon.han@intel.com>
Cr-Commit-Position: refs/heads/master@{#816464}

content/browser/power_monitor_browsertest.cc

@@ -70,10 +70,7 @@ class MockPowerMonitorMessageBroadcaster : public device::mojom::PowerMonitor {
   ~MockPowerMonitorMessageBroadcaster() override = default;
 
   void Bind(mojo::PendingReceiver<device::mojom::PowerMonitor> receiver) {
-    GetUIThreadTaskRunner({})->PostTask(
-        FROM_HERE,
-        base::BindOnce(&MockPowerMonitorMessageBroadcaster::BindOnMainThread,
-                       base::Unretained(this), std::move(receiver)));
+    receivers_.Add(this, std::move(receiver));
   }
 
   // device::mojom::PowerMonitor:
@@ -92,11 +89,6 @@ class MockPowerMonitorMessageBroadcaster : public device::mojom::PowerMonitor {
   }
 
  private:
-  void BindOnMainThread(
-      mojo::PendingReceiver<device::mojom::PowerMonitor> receiver) {
-    receivers_.Add(this, std::move(receiver));
-  }
-
   bool on_battery_power_ = false;
 
   mojo::ReceiverSet<device::mojom::PowerMonitor> receivers_;
@@ -130,16 +122,10 @@ class PowerMonitorTest : public ContentBrowserTest {
     if (!r)
       return;
 
-    // We can receiver binding requests for the spare RenderProcessHost -- this
-    // might happen before the test has provided the |renderer_bound_closure_|.
-    if (renderer_bound_closure_) {
-      ++request_count_from_renderer_;
-      std::move(renderer_bound_closure_).Run();
-    } else {
-      DCHECK(RenderProcessHostImpl::GetSpareRenderProcessHostForTesting());
-    }
-
-    power_monitor_message_broadcaster_.Bind(std::move(r));
+    GetUIThreadTaskRunner({})->PostTask(
+        FROM_HERE,
+        base::BindOnce(&PowerMonitorTest::BindForRendererOnMainThread,
+                       base::Unretained(this), std::move(r)));
   }
 
   void BindForNonRenderer(BrowserChildProcessHost* process_host,
@@ -148,28 +134,11 @@ class PowerMonitorTest : public ContentBrowserTest {
     if (!r)
       return;
 
-    const int type = process_host->GetData().process_type;
-    if (type == PROCESS_TYPE_UTILITY) {
-      if (utility_bound_closure_) {
-        ++request_count_from_utility_;
-        std::move(utility_bound_closure_).Run();
-      }
-    } else if (type == PROCESS_TYPE_GPU) {
-      ++request_count_from_gpu_;
-
-      // We ignore null gpu_bound_closure_ here for two possible scenarios:
-      //  - TestRendererProcess and TestUtilityProcess also result in spinning
-      //    up GPU processes as a side effect, but they do not set valid
-      //    gpu_bound_closure_.
-      //  - As GPU process is started during setup of browser test suite, so
-      //    it's possible that TestGpuProcess execution may have not started
-      //    yet when the PowerMonitor bind request comes here, in such case
-      //    gpu_bound_closure_ will also be null.
-      if (gpu_bound_closure_)
-        std::move(gpu_bound_closure_).Run();
-    }
-
-    power_monitor_message_broadcaster_.Bind(std::move(r));
+    GetUIThreadTaskRunner({})->PostTask(
+        FROM_HERE,
+        base::BindOnce(&PowerMonitorTest::BindForNonRendererOnMainThread,
+                       base::Unretained(this),
+                       process_host->GetData().process_type, std::move(r)));
   }
 
  protected:
@@ -200,6 +169,46 @@ class PowerMonitorTest : public ContentBrowserTest {
   }
 
  private:
+  void BindForRendererOnMainThread(
+      mojo::PendingReceiver<device::mojom::PowerMonitor> receiver) {
+    // We can receiver binding requests for the spare RenderProcessHost -- this
+    // might happen before the test has provided the |renderer_bound_closure_|.
+    if (renderer_bound_closure_) {
+      ++request_count_from_renderer_;
+      std::move(renderer_bound_closure_).Run();
+    } else {
+      DCHECK(RenderProcessHostImpl::GetSpareRenderProcessHostForTesting());
+    }
+
+    power_monitor_message_broadcaster_.Bind(std::move(receiver));
+  }
+
+  void BindForNonRendererOnMainThread(
+      int process_type,
+      mojo::PendingReceiver<device::mojom::PowerMonitor> receiver) {
+    if (process_type == PROCESS_TYPE_UTILITY) {
+      if (utility_bound_closure_) {
+        ++request_count_from_utility_;
+        std::move(utility_bound_closure_).Run();
+      }
+    } else if (process_type == PROCESS_TYPE_GPU) {
+      ++request_count_from_gpu_;
+
+      // We ignore null gpu_bound_closure_ here for two possible scenarios:
+      //  - TestRendererProcess and TestUtilityProcess also result in spinning
+      //    up GPU processes as a side effect, but they do not set valid
+      //    gpu_bound_closure_.
+      //  - As GPU process is started during setup of browser test suite, so
+      //    it's possible that TestGpuProcess execution may have not started
+      //    yet when the PowerMonitor bind request comes here, in such case
+      //    gpu_bound_closure_ will also be null.
+      if (gpu_bound_closure_)
+        std::move(gpu_bound_closure_).Run();
+    }
+
+    power_monitor_message_broadcaster_.Bind(std::move(receiver));
+  }
+
   int request_count_from_renderer_ = 0;
   int request_count_from_utility_ = 0;
   int request_count_from_gpu_ = 0;
@@ -261,13 +270,8 @@ IN_PROC_BROWSER_TEST_F(PowerMonitorTest, TestUtilityProcess) {
   // Verify utility process on_battery_power changed to false.
   VerifyPowerStateInChildProcess(power_monitor_utility.get(), false);
 }
-// This flakes on Linux TSan: http://crbug.com/1127374
-#if defined(THREAD_SANITIZER)
-#define MAYBE_TestGpuProcess DISABLED_TestGpuProcess
-#else
-#define MAYBE_TestGpuProcess TestGpuProcess
-#endif
-IN_PROC_BROWSER_TEST_F(PowerMonitorTest, MAYBE_TestGpuProcess) {
+
+IN_PROC_BROWSER_TEST_F(PowerMonitorTest, TestGpuProcess) {
   // As gpu process is started automatically during the setup period of browser
   // test suite, it may have already started and bound PowerMonitor interface to
   // Device Service before execution of this TestGpuProcess test. So here we