BatteryMonitor: Use Frame BrowserInterfaceBroker

Bind the battery monitor to the RenderFrameHostImpl BIB instead of the
RenderProcessHostImpl BIB. This allows freezing/managing the interface
with more granularity.

Remove multiple-windows tests. These were necessary when multiple
windows could share the same mojo connection. Since each page has its
own connection these tests are no longer necessary.

Bug: 1126758

Change-Id: Iba5a928105d19a9e86f9100fa6fdcf356b847c29
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2500968Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Tim Volodine <timvolodine@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Commit-Queue: Johann Koenig <johannkoenig@google.com>
Cr-Commit-Position: refs/heads/master@{#828585}

content/browser/battery_monitor_browsertest.cc

@@ -7,6 +7,7 @@
 #include "base/bind.h"
 #include "base/callback_helpers.h"
 #include "base/macros.h"
+#include "content/browser/browser_interface_binders.h"
 #include "content/browser/renderer_host/render_process_host_impl.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/test/browser_test.h"
@@ -77,14 +78,13 @@ class BatteryMonitorTest : public ContentBrowserTest {
     // Because Device Service also runs in this process(browser process), here
     // we can directly set our binder to intercept interface requests against
     // it.
-    RenderProcessHostImpl::OverrideBatteryMonitorBinderForTesting(
+    OverrideBatteryMonitorBinderForTesting(
         base::BindRepeating(&MockBatteryMonitor::Bind,
                             base::Unretained(mock_battery_monitor_.get())));
   }
 
   ~BatteryMonitorTest() override {
-    RenderProcessHostImpl::OverrideBatteryMonitorBinderForTesting(
-        base::NullCallback());
+    OverrideBatteryMonitorBinderForTesting(base::NullCallback());
   }
 
  protected:

content/browser/browser_interface_binders.cc

@@ -64,6 +64,7 @@
 #include "media/mojo/mojom/remoting.mojom.h"
 #include "media/mojo/mojom/video_decode_perf_history.mojom.h"
 #include "media/mojo/services/video_decode_perf_history.h"
+#include "services/device/public/mojom/battery_monitor.mojom.h"
 #include "services/device/public/mojom/sensor_provider.mojom.h"
 #include "services/device/public/mojom/vibration_manager.mojom.h"
 #include "services/metrics/public/mojom/ukm_interface.mojom.h"
@@ -490,6 +491,20 @@ void EmptyBinderForFrame(RenderFrameHost* host,
               << " for the frame/document scope";
 }
 
+BatteryMonitorBinder& GetBatteryMonitorBinderOverride() {
+  static base::NoDestructor<BatteryMonitorBinder> binder;
+  return *binder;
+}
+
+void BindBatteryMonitor(
+    mojo::PendingReceiver<device::mojom::BatteryMonitor> receiver) {
+  const auto& binder = GetBatteryMonitorBinderOverride();
+  if (binder)
+    binder.Run(std::move(receiver));
+  else
+    GetDeviceService().BindBatteryMonitor(std::move(receiver));
+}
+
 VibrationManagerBinder& GetVibrationManagerBinderOverride() {
   static base::NoDestructor<VibrationManagerBinder> binder;
   return *binder;
@@ -516,6 +531,9 @@ void PopulateFrameBinders(RenderFrameHostImpl* host, mojo::BinderMap* map) {
   map->Add<blink::mojom::AudioContextManager>(base::BindRepeating(
       &RenderFrameHostImpl::GetAudioContextManager, base::Unretained(host)));
 
+  map->Add<device::mojom::BatteryMonitor>(
+      base::BindRepeating(&BindBatteryMonitor));
+
   map->Add<blink::mojom::CacheStorage>(base::BindRepeating(
       &RenderFrameHostImpl::BindCacheStorage, base::Unretained(host)));
 
@@ -1143,6 +1161,10 @@ void PopulateBinderMap(ServiceWorkerHost* host, mojo::BinderMap* map) {
 
 }  // namespace internal
 
+void OverrideBatteryMonitorBinderForTesting(BatteryMonitorBinder binder) {
+  internal::GetBatteryMonitorBinderOverride() = std::move(binder);
+}
+
 void OverrideVibrationManagerBinderForTesting(VibrationManagerBinder binder) {
   internal::GetVibrationManagerBinderOverride() = std::move(binder);
 }

content/browser/browser_interface_binders.h

@@ -8,6 +8,7 @@
 #include "content/browser/service_worker/service_worker_info.h"
 #include "content/common/content_export.h"
 #include "mojo/public/cpp/bindings/binder_map.h"
+#include "services/device/public/mojom/battery_monitor.mojom-forward.h"
 #include "services/device/public/mojom/vibration_manager.mojom-forward.h"
 #include "url/origin.h"
 
@@ -60,6 +61,12 @@ ServiceWorkerVersionInfo GetContextForHost(ServiceWorkerHost* host);
 
 }  // namespace internal
 
+// Allows tests to override how frame hosts bind BatteryMonitor receivers.
+using BatteryMonitorBinder = base::RepeatingCallback<void(
+    mojo::PendingReceiver<device::mojom::BatteryMonitor>)>;
+CONTENT_EXPORT void OverrideBatteryMonitorBinderForTesting(
+    BatteryMonitorBinder binder);
+
 // Allows tests to override how frame hosts bind VibrationManager receivers.
 using VibrationManagerBinder = base::RepeatingCallback<void(
     mojo::PendingReceiver<device::mojom::VibrationManager>)>;

content/browser/renderer_host/render_process_host_impl.cc

@@ -199,7 +199,6 @@
 #include "mojo/public/cpp/bindings/scoped_message_error_crash_key.h"
 #include "mojo/public/cpp/system/platform_handle.h"
 #include "sandbox/policy/switches.h"
-#include "services/device/public/mojom/battery_monitor.mojom.h"
 #include "services/device/public/mojom/power_monitor.mojom.h"
 #include "services/device/public/mojom/screen_orientation.mojom.h"
 #include "services/device/public/mojom/time_zone_monitor.mojom.h"
@@ -1241,20 +1240,6 @@ GetBindHostReceiverInterceptor() {
   return *interceptor;
 }
 
-RenderProcessHostImpl::BatteryMonitorBinder& GetBatteryMonitorBinderOverride() {
-  static base::NoDestructor<RenderProcessHostImpl::BatteryMonitorBinder> binder;
-  return *binder;
-}
-
-void BindBatteryMonitor(
-    mojo::PendingReceiver<device::mojom::BatteryMonitor> receiver) {
-  const auto& binder = GetBatteryMonitorBinderOverride();
-  if (binder)
-    binder.Run(std::move(receiver));
-  else
-    GetDeviceService().BindBatteryMonitor(std::move(receiver));
-}
-
 RenderProcessHostImpl::CreateNetworkFactoryCallback&
 GetCreateNetworkFactoryCallback() {
   static base::NoDestructor<RenderProcessHostImpl::CreateNetworkFactoryCallback>
@@ -2247,18 +2232,9 @@ RenderProcessHostImpl::GetPeerConnectionTrackerHost() {
   return peer_connection_tracker_host_.get();
 }
 
-// static
-void RenderProcessHostImpl::OverrideBatteryMonitorBinderForTesting(
-    BatteryMonitorBinder binder) {
-  GetBatteryMonitorBinderOverride() = std::move(binder);
-}
-
 void RenderProcessHostImpl::RegisterMojoInterfaces() {
   auto registry = std::make_unique<service_manager::BinderRegistry>();
 
-  AddUIThreadInterface(registry.get(),
-                       base::BindRepeating(&BindBatteryMonitor));
-
   AddUIThreadInterface(
       registry.get(),
       base::BindRepeating(

content/browser/renderer_host/render_process_host_impl.h

@@ -64,7 +64,6 @@
 #include "net/base/network_isolation_key.h"
 #include "net/net_buildflags.h"
 #include "ppapi/buildflags/buildflags.h"
-#include "services/device/public/mojom/battery_monitor.mojom-forward.h"
 #include "services/network/public/mojom/mdns_responder.mojom.h"
 #include "services/network/public/mojom/p2p.mojom-forward.h"
 #include "services/network/public/mojom/url_loader_factory.mojom.h"
@@ -684,13 +683,6 @@ class CONTENT_EXPORT RenderProcessHostImpl
 
   PeerConnectionTrackerHost* GetPeerConnectionTrackerHost();
 
-  // Allows tests to override how RenderProcessHosts bind incoming
-  // BatteryMonitor receivers.
-  using BatteryMonitorBinder = base::RepeatingCallback<void(
-      mojo::PendingReceiver<device::mojom::BatteryMonitor>)>;
-  static void OverrideBatteryMonitorBinderForTesting(
-      BatteryMonitorBinder binder);
-
   // Allows overriding the URLLoaderFactory creation via CreateURLLoaderFactory.
   // Passing a null callback will restore the default behavior.
   // This method must be called either on the UI thread or before threads start.

third_party/blink/renderer/modules/battery/battery_dispatcher.cc

@@ -5,7 +5,7 @@
 #include "third_party/blink/renderer/modules/battery/battery_dispatcher.h"
 
 #include "services/device/public/mojom/battery_status.mojom-blink.h"
-#include "third_party/blink/public/common/thread_safe_browser_interface_broker_proxy.h"
+#include "third_party/blink/public/common/browser_interface_broker_proxy.h"
 #include "third_party/blink/public/platform/platform.h"
 #include "third_party/blink/renderer/core/frame/local_dom_window.h"
 #include "third_party/blink/renderer/platform/mojo/mojo_helper.h"
@@ -13,14 +13,8 @@
 
 namespace blink {
 
-BatteryDispatcher& BatteryDispatcher::Instance() {
-  DEFINE_STATIC_LOCAL(Persistent<BatteryDispatcher>, battery_dispatcher,
-                      (MakeGarbageCollected<BatteryDispatcher>()));
-  return *battery_dispatcher;
-}
-
-BatteryDispatcher::BatteryDispatcher()
-    : monitor_(nullptr), has_latest_data_(false) {}
+BatteryDispatcher::BatteryDispatcher(ExecutionContext* context)
+    : monitor_(context), has_latest_data_(false) {}
 
 void BatteryDispatcher::Trace(Visitor* visitor) const {
   visitor->Trace(monitor_);
@@ -53,7 +47,7 @@ void BatteryDispatcher::UpdateBatteryStatus(
 void BatteryDispatcher::StartListening(LocalDOMWindow* window) {
   DCHECK(!monitor_.is_bound());
   // See https://bit.ly/2S0zRAS for task types.
-  Platform::Current()->GetBrowserInterfaceBroker()->GetInterface(
+  window->GetBrowserInterfaceBroker().GetInterface(
       monitor_.BindNewPipeAndPassReceiver(
           window->GetTaskRunner(TaskType::kMiscPlatformAPI)));
   QueryNextStatus();

third_party/blink/renderer/modules/battery/battery_dispatcher.h

@@ -7,8 +7,8 @@
 
 #include "base/macros.h"
 #include "services/device/public/mojom/battery_monitor.mojom-blink.h"
+#include "third_party/blink/renderer/core/execution_context/execution_context.h"
 #include "third_party/blink/renderer/core/frame/platform_event_dispatcher.h"
-#include "third_party/blink/renderer/modules/battery/battery_manager.h"
 #include "third_party/blink/renderer/modules/battery/battery_status.h"
 #include "third_party/blink/renderer/modules/modules_export.h"
 #include "third_party/blink/renderer/platform/mojo/heap_mojo_remote.h"
@@ -19,9 +19,7 @@ class MODULES_EXPORT BatteryDispatcher final
     : public GarbageCollected<BatteryDispatcher>,
       public PlatformEventDispatcher {
  public:
-  static BatteryDispatcher& Instance();
-
-  BatteryDispatcher();
+  explicit BatteryDispatcher(ExecutionContext*);
 
   const BatteryStatus* LatestData() const {
     return has_latest_data_ ? &battery_status_ : nullptr;

third_party/blink/renderer/modules/battery/battery_manager.cc

@@ -45,7 +45,9 @@ BatteryManager::~BatteryManager() = default;
 BatteryManager::BatteryManager(Navigator& navigator)
     : Supplement<Navigator>(navigator),
       ExecutionContextLifecycleStateObserver(navigator.DomWindow()),
-      PlatformEventController(*navigator.DomWindow()) {
+      PlatformEventController(*navigator.DomWindow()),
+      battery_dispatcher_(
+          MakeGarbageCollected<BatteryDispatcher>(navigator.DomWindow())) {
   UpdateStateIfNeeded();
 }
 
@@ -86,7 +88,7 @@ void BatteryManager::DidUpdateData() {
   DCHECK(battery_property_);
 
   BatteryStatus old_status = battery_status_;
-  battery_status_ = *BatteryDispatcher::Instance().LatestData();
+  battery_status_ = *battery_dispatcher_->LatestData();
 
   if (battery_property_->GetState() == BatteryProperty::kPending) {
     battery_property_->Resolve(this);
@@ -110,15 +112,15 @@ void BatteryManager::DidUpdateData() {
 }
 
 void BatteryManager::RegisterWithDispatcher() {
-  BatteryDispatcher::Instance().AddController(this, DomWindow());
+  battery_dispatcher_->AddController(this, DomWindow());
 }
 
 void BatteryManager::UnregisterWithDispatcher() {
-  BatteryDispatcher::Instance().RemoveController(this);
+  battery_dispatcher_->RemoveController(this);
 }
 
 bool BatteryManager::HasLastData() {
-  return BatteryDispatcher::Instance().LatestData();
+  return battery_dispatcher_->LatestData();
 }
 
 void BatteryManager::ContextLifecycleStateChanged(
@@ -148,6 +150,7 @@ bool BatteryManager::HasPendingActivity() const {
 
 void BatteryManager::Trace(Visitor* visitor) const {
   visitor->Trace(battery_property_);
+  visitor->Trace(battery_dispatcher_);
   Supplement<Navigator>::Trace(visitor);
   PlatformEventController::Trace(visitor);
   EventTargetWithInlineData::Trace(visitor);

third_party/blink/renderer/modules/battery/battery_manager.h

@@ -10,6 +10,7 @@
 #include "third_party/blink/renderer/bindings/core/v8/script_promise_property.h"
 #include "third_party/blink/renderer/core/execution_context/execution_context_lifecycle_state_observer.h"
 #include "third_party/blink/renderer/core/frame/platform_event_controller.h"
+#include "third_party/blink/renderer/modules/battery/battery_dispatcher.h"
 #include "third_party/blink/renderer/modules/battery/battery_status.h"
 #include "third_party/blink/renderer/modules/event_target_modules.h"
 #include "third_party/blink/renderer/platform/heap/handle.h"
@@ -73,6 +74,7 @@ class BatteryManager final : public EventTargetWithInlineData,
       ScriptPromiseProperty<Member<BatteryManager>, Member<DOMException>>;
   Member<BatteryProperty> battery_property_;
   BatteryStatus battery_status_;
+  Member<BatteryDispatcher> battery_dispatcher_;
 };
 
 }  // namespace blink

third_party/blink/web_tests/battery-status/multiple-windows-expected.txt

-Test battery status API with multiple windows.
-
-On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
-
-resolution in window
-PASS batteryInfo is defined.
-PASS lastSetMockBatteryInfo is defined.
-PASS batteryInfo.charging is lastSetMockBatteryInfo.charging
-PASS batteryInfo.chargingTime is lastSetMockBatteryInfo.chargingTime
-PASS batteryInfo.dischargingTime is lastSetMockBatteryInfo.dischargingTime
-PASS batteryInfo.level is lastSetMockBatteryInfo.level
-resolution in secondWindow
-PASS batteryInfo is defined.
-PASS lastSetMockBatteryInfo is defined.
-PASS batteryInfo.charging is lastSetMockBatteryInfo.charging
-PASS batteryInfo.chargingTime is lastSetMockBatteryInfo.chargingTime
-PASS batteryInfo.dischargingTime is lastSetMockBatteryInfo.dischargingTime
-PASS batteryInfo.level is lastSetMockBatteryInfo.level
-PASS successfullyParsed is true
-
-TEST COMPLETE
-

third_party/blink/web_tests/battery-status/multiple-windows-page-visibility-expected.txt

-Test multiple windows with page visibility.
-
-On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
-
-first window: page is visible
-resolution in window
-PASS batteryInfo is defined.
-PASS lastSetMockBatteryInfo is defined.
-PASS batteryInfo.charging is lastSetMockBatteryInfo.charging
-PASS batteryInfo.chargingTime is lastSetMockBatteryInfo.chargingTime
-PASS batteryInfo.dischargingTime is lastSetMockBatteryInfo.dischargingTime
-PASS batteryInfo.level is lastSetMockBatteryInfo.level
-first window: page is hidden
-resolution in secondWindow
-PASS batteryInfo is defined.
-PASS lastSetMockBatteryInfo is defined.
-PASS batteryInfo.charging is lastSetMockBatteryInfo.charging
-PASS batteryInfo.chargingTime is lastSetMockBatteryInfo.chargingTime
-PASS batteryInfo.dischargingTime is lastSetMockBatteryInfo.dischargingTime
-PASS batteryInfo.level is lastSetMockBatteryInfo.level
-first window: page is visible
-PASS batteryInfo is defined.
-PASS lastSetMockBatteryInfo is defined.
-PASS batteryInfo.charging is lastSetMockBatteryInfo.charging
-PASS batteryInfo.chargingTime is lastSetMockBatteryInfo.chargingTime
-PASS batteryInfo.dischargingTime is lastSetMockBatteryInfo.dischargingTime
-PASS batteryInfo.level is lastSetMockBatteryInfo.level
-PASS successfullyParsed is true
-
-TEST COMPLETE
-

third_party/blink/web_tests/battery-status/multiple-windows-page-visibility.html

-<!DOCTYPE html>
-<html>
-<body>
-<script src="../resources/js-test.js"></script>
-<script src="../resources/visibility.js"></script>
-<script src="file:///gen/layout_test_data/mojo/public/js/mojo_bindings.js"></script>
-<script src="file:///gen/services/device/public/mojom/battery_monitor.mojom.js"></script>
-<script src="resources/mock-battery-monitor.js"></script>
-<script>
-description("Test multiple windows with page visibility.");
-
-if (!window.testRunner)
-    debug('This test cannot be run without the TestRunner');
-
-// Clean-up any unused battery manager objects from previous tests.
-gc();
-jsTestIsAsync = true;
-testRunner.waitUntilDone();
-testRunner.setCanOpenWindows();
-
-document.addEventListener("visibilitychange", function() {
-    debug("first window: page is " + document.visibilityState);
-});
-
-var battery;
-var battery2;
-function batteryStatusSuccess(batteryManager) {
-    battery = batteryManager;
-    debug('resolution in window');
-    testIfBatteryStatusIsUpToDate(battery);
-    battery.addEventListener('levelchange', failAndFinish);
-
-    firstWindow = window;
-    secondWindow = window.open("");
-    secondWindow.navigator.getBattery().then(
-        (b) => { battery2 = b; },
-        batteryStatusFailure);
-
-    setMainWindowHidden(true).then(checkBatteryInSecondWindow);
-}
-
-function checkBatteryInSecondWindow() {
-    debug('resolution in secondWindow');
-    testIfBatteryStatusIsUpToDate(battery2);
-    setTimeout(fireNewMockLevel, 0);
-}
-
-function fireNewMockLevel() {
-    setAndFireMockBatteryInfo(false, 10, 20, 0.6);
-    proceedToVisible();
-}
-
-function proceedToVisible() {
-    battery.removeEventListener('levelchange', failAndFinish);
-    battery.addEventListener('levelchange', onLevelChange);
-
-    setMainWindowHidden(false);
-}
-
-function onLevelChange() {
-    testIfBatteryStatusIsUpToDate(battery);
-    battery.removeEventListener('levelchange', onLevelChange);
-    setTimeout(finishJSTest, 0);
-}
-
-function failAndFinish() {
-    testFailed('received event while the page was hidden');
-    setTimeout(finishJSTest, 0);
-}
-
-debug("first window: page is visible");
-navigator.getBattery().then(batteryStatusSuccess, batteryStatusFailure);
-setAndFireMockBatteryInfo(false, 10, 20, 0.5);
-</script>
-</body>
-</html>

third_party/blink/web_tests/battery-status/multiple-windows.html

-<!DOCTYPE html>
-<html>
-<body>
-<script src="../resources/js-test.js"></script>
-<script src="file:///gen/layout_test_data/mojo/public/js/mojo_bindings.js"></script>
-<script src="file:///gen/services/device/public/mojom/battery_monitor.mojom.js"></script>
-<script src="resources/mock-battery-monitor.js"></script>
-<script>
-description("Test battery status API with multiple windows.");
-
-if (!window.testRunner)
-    debug('This test cannot be run without the TestRunner');
-
-// Clean-up any unused battery manager objects from previous tests.
-gc();
-jsTestIsAsync = true;
-testRunner.waitUntilDone();
-testRunner.setCanOpenWindows();
-
-function batteryStatusSuccess(battery) {
-    debug('resolution in window');
-    testIfBatteryStatusIsUpToDate(battery);
-
-    secondWindow = window.open("");
-    secondWindow.navigator.getBattery().then(
-        function(battery2) {
-            debug('resolution in secondWindow');
-            testIfBatteryStatusIsUpToDate(battery2);
-            setTimeout(finishJSTest, 0);
-        }, batteryStatusFailure);
-}
-
-navigator.getBattery().then(batteryStatusSuccess, batteryStatusFailure);
-setAndFireMockBatteryInfo(false, 10, 20, 0.5);
-</script>
-</body>
-</html>

third_party/blink/web_tests/battery-status/resources/mock-battery-monitor.js

@@ -17,7 +17,7 @@ class MockBatteryMonitor {
     this.bindingSet_ = new mojo.BindingSet(device.mojom.BatteryMonitor);
 
     this.interceptor_ = new MojoInterfaceInterceptor(
-        device.mojom.BatteryMonitor.name, "process");
+        device.mojom.BatteryMonitor.name);
     this.interceptor_.oninterfacerequest = e => this.bindRequest(e.handle);
     this.interceptor_.start();
   }