sensors: Make SensorProviderProxy supplement Document, not LocalFrame

Supplementing LocalFrame causes the same SensorProviderProxy to be used
across navigations, and so is the connection to the actual sensor provider
via mojo.

Making SensorProviderProxy supplement Document also helps with testing: as
described in the related bug, running a manual test (which does not use
sensor mocks) and then a regular test causes the latter to fail because we
never establish a connection to the mock we need, but rather try to use an
actual sensor that is not present.

Bug: 861675
Change-Id: Ic3d2ea42f5b4654ba5ef42b6ad757a49d3e5ef08
Reviewed-on: https://chromium-review.googlesource.com/1160161Reviewed-by: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Raphael Kubo da Costa (CET) <raphael.kubo.da.costa@intel.com>
Cr-Commit-Position: refs/heads/master@{#589514}

third_party/WebKit/LayoutTests/http/tests/devtools/device-orientation-success-expected.txt

@@ -17,3 +17,6 @@ device-orientation-success.js:32 alpha: 90 beta: 0 gamma: 0
 device-orientation-success.js:39 quaternion: 0.000000,0.000000,0.707107,0.707107
 device-orientation-success.js:32 alpha: 1.1 beta: 2.2 gamma: 3.3
 
+Running: reloadPageAndOverride
+Page reloaded.
+

third_party/WebKit/LayoutTests/http/tests/devtools/device-orientation-success.js

@@ -78,5 +78,24 @@
       ConsoleTestRunner.dumpConsoleMessages();
       next();
     },
+
+    // This test ensures we do not repeatedly show console warning from
+    // SensorInspectorAgent::SetOrientationSensorOverride() when Document
+    // changes.
+    async function reloadPageAndOverride(next) {
+      // First, enable DeviceOrientationAgent again.
+      TestRunner.DeviceOrientationAgent.setDeviceOrientationOverride(1, 2, 3);
+
+      // Reload the page. DeviceOrientationInspectorAgent::Restore() will call
+      // SensorInspectorAgent::SetOrientationSensorOverride() again, and we do
+      // not want any new console messages.
+      // The console message from the call to setDeviceOrientationOverride()
+      // above is lost with the reload, but we do not care.
+      await TestRunner.reloadPagePromise();
+      ConsoleTestRunner.dumpConsoleMessages();
+
+      await TestRunner.DeviceOrientationAgent.clearDeviceOrientationOverride();
+      next();
+    },
   ]);
 })();

third_party/blink/renderer/modules/device_orientation/device_orientation_inspector_agent.cc

@@ -20,7 +20,8 @@ DeviceOrientationInspectorAgent::~DeviceOrientationInspectorAgent() = default;
 DeviceOrientationInspectorAgent::DeviceOrientationInspectorAgent(
     InspectedFrames* inspected_frames)
     : inspected_frames_(inspected_frames),
-      sensor_agent_(new SensorInspectorAgent(inspected_frames->Root())),
+      sensor_agent_(
+          new SensorInspectorAgent(inspected_frames->Root()->GetDocument())),
       enabled_(&agent_state_, /*default_value=*/false),
       alpha_(&agent_state_, /*default_value=*/0.0),
       beta_(&agent_state_, /*default_value=*/0.0),
@@ -79,6 +80,7 @@ void DeviceOrientationInspectorAgent::DidCommitLoadForLocalFrame(
   if (frame == inspected_frames_->Root()) {
     // New document in main frame - apply override there.
     // No need to cleanup previous one, as it's already gone.
+    sensor_agent_->DidCommitLoadForLocalFrame(frame);
     Restore();
   }
 }

third_party/blink/renderer/modules/sensor/sensor.cc

@@ -177,7 +177,7 @@ void Sensor::InitSensorProxyIfNeeded() {
   if (!document || !document->GetFrame())
     return;
 
-  auto* provider = SensorProviderProxy::From(document->GetFrame());
+  auto* provider = SensorProviderProxy::From(document);
   sensor_proxy_ = provider->GetSensorProxy(type_);
 
   if (!sensor_proxy_)

third_party/blink/renderer/modules/sensor/sensor_inspector_agent.cc

@@ -13,8 +13,8 @@
 
 namespace blink {
 
-SensorInspectorAgent::SensorInspectorAgent(LocalFrame* frame)
-    : provider_(SensorProviderProxy::From(frame)) {}
+SensorInspectorAgent::SensorInspectorAgent(Document* document)
+    : provider_(SensorProviderProxy::From(document)) {}
 
 void SensorInspectorAgent::Trace(blink::Visitor* visitor) {
   visitor->Trace(provider_);
@@ -59,11 +59,24 @@ const char kInspectorConsoleMessage[] =
 
 }  // namespace
 
+void SensorInspectorAgent::DidCommitLoadForLocalFrame(LocalFrame* frame) {
+  Document* current_document = provider_->GetSupplementable();
+  Document* new_document = frame->GetDocument();
+  if (current_document != new_document) {
+    // We need to manually reset |provider_| to drop the strong reference it
+    // has to an old document that would otherwise be prevented from being
+    // deleted.
+    bool inspector_mode = provider_->inspector_mode();
+    provider_ = SensorProviderProxy::From(new_document);
+    provider_->set_inspector_mode(inspector_mode);
+  }
+}
+
 void SensorInspectorAgent::SetOrientationSensorOverride(double alpha,
                                                         double beta,
                                                         double gamma) {
   if (!provider_->inspector_mode()) {
-    Document* document = provider_->GetSupplementable()->GetDocument();
+    Document* document = provider_->GetSupplementable();
     if (document) {
       ConsoleMessage* console_message = ConsoleMessage::Create(
           kJSMessageSource, kInfoMessageLevel, kInspectorConsoleMessage);

third_party/blink/renderer/modules/sensor/sensor_inspector_agent.h

@@ -9,14 +9,17 @@
 
 namespace blink {
 
+class Document;
 class LocalFrame;
 class SensorProviderProxy;
 
 class SensorInspectorAgent : public GarbageCollected<SensorInspectorAgent> {
  public:
-  explicit SensorInspectorAgent(LocalFrame* frame);
+  explicit SensorInspectorAgent(Document* document);
   virtual void Trace(blink::Visitor*);
 
+  void DidCommitLoadForLocalFrame(LocalFrame* frame);
+
   void SetOrientationSensorOverride(double alpha, double beta, double gamma);
 
   void Disable();

third_party/blink/renderer/modules/sensor/sensor_provider_proxy.cc

@@ -13,14 +13,14 @@
 namespace blink {
 
 // SensorProviderProxy
-SensorProviderProxy::SensorProviderProxy(LocalFrame& frame)
-    : Supplement<LocalFrame>(frame), inspector_mode_(false) {}
+SensorProviderProxy::SensorProviderProxy(Document& document)
+    : Supplement<Document>(document), inspector_mode_(false) {}
 
 void SensorProviderProxy::InitializeIfNeeded() {
   if (IsInitialized())
     return;
 
-  GetSupplementable()->GetInterfaceProvider().GetInterface(
+  GetSupplementable()->GetInterfaceProvider()->GetInterface(
       mojo::MakeRequest(&sensor_provider_));
   sensor_provider_.set_connection_error_handler(
       WTF::Bind(&SensorProviderProxy::OnSensorProviderConnectionError,
@@ -31,13 +31,13 @@ void SensorProviderProxy::InitializeIfNeeded() {
 const char SensorProviderProxy::kSupplementName[] = "SensorProvider";
 
 // static
-SensorProviderProxy* SensorProviderProxy::From(LocalFrame* frame) {
-  DCHECK(frame);
+SensorProviderProxy* SensorProviderProxy::From(Document* document) {
+  DCHECK(document);
   SensorProviderProxy* provider_proxy =
-      Supplement<LocalFrame>::From<SensorProviderProxy>(*frame);
+      Supplement<Document>::From<SensorProviderProxy>(*document);
   if (!provider_proxy) {
-    provider_proxy = new SensorProviderProxy(*frame);
-    Supplement<LocalFrame>::ProvideTo(*frame, provider_proxy);
+    provider_proxy = new SensorProviderProxy(*document);
+    Supplement<Document>::ProvideTo(*document, provider_proxy);
   }
   provider_proxy->InitializeIfNeeded();
   return provider_proxy;
@@ -47,7 +47,7 @@ SensorProviderProxy::~SensorProviderProxy() = default;
 
 void SensorProviderProxy::Trace(blink::Visitor* visitor) {
   visitor->Trace(sensor_proxies_);
-  Supplement<LocalFrame>::Trace(visitor);
+  Supplement<Document>::Trace(visitor);
 }
 
 SensorProxy* SensorProviderProxy::CreateSensorProxy(

third_party/blink/renderer/modules/sensor/sensor_provider_proxy.h

@@ -7,7 +7,7 @@
 
 #include "services/device/public/mojom/sensor.mojom-blink.h"
 #include "services/device/public/mojom/sensor_provider.mojom-blink.h"
-#include "third_party/blink/renderer/core/frame/local_frame.h"
+#include "third_party/blink/renderer/core/dom/document.h"
 #include "third_party/blink/renderer/platform/heap/handle.h"
 #include "third_party/blink/renderer/platform/supplementable.h"
 
@@ -19,14 +19,14 @@ class SensorProxy;
 // 'SensorProxy' instances.
 class SensorProviderProxy final
     : public GarbageCollectedFinalized<SensorProviderProxy>,
-      public Supplement<LocalFrame> {
+      public Supplement<Document> {
   USING_GARBAGE_COLLECTED_MIXIN(SensorProviderProxy);
   WTF_MAKE_NONCOPYABLE(SensorProviderProxy);
 
  public:
   static const char kSupplementName[];
 
-  static SensorProviderProxy* From(LocalFrame*);
+  static SensorProviderProxy* From(Document*);
 
   ~SensorProviderProxy();
 
@@ -50,7 +50,7 @@ class SensorProviderProxy final
   const SensorsSet& sensor_proxies() const { return sensor_proxies_; }
 
   // For SensorProviderProxy personal use.
-  explicit SensorProviderProxy(LocalFrame&);
+  explicit SensorProviderProxy(Document&);
   void InitializeIfNeeded();
   bool IsInitialized() const { return sensor_provider_.is_bound(); }
   void OnSensorProviderConnectionError();

third_party/blink/renderer/modules/sensor/sensor_proxy.cc

@@ -117,7 +117,7 @@ bool SensorProxy::ShouldSuspendUpdates() const {
   if (!focused_frame)
     return true;
 
-  LocalFrame* this_frame = provider_->GetSupplementable();
+  LocalFrame* this_frame = provider_->GetSupplementable()->GetFrame();
   if (focused_frame == this_frame)
     return false;