Decouple DeviceSensorEventPump from PlatformEventObserver

This changes prepares DeviceSensorEventPump and its subclasses for
the move to blink by removing the inheritance of DeviceSensorEventPump
from PlatformEventObserver and moving listener-related logic to
DeviceSensorEventPump. Since Device*EventPump instances can no longer be
stored in RendererBlinkPlatformImpl::platform_event_observers_, separate
variables have been added temporarily that will be moved to appropriate
Device*Dispatcher classes along with the logic when Device*EventPump
classes are moved to blink.

Bug: 850997
Change-Id: Iedf23e368adb9180091d094146306330987bcf9c
Reviewed-on: https://chromium-review.googlesource.com/1091275
Commit-Queue: Oksana Zhuravlova <oksamyt@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Tim Volodine <timvolodine@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#566706}

content/renderer/device_sensors/device_motion_event_pump.cc

@@ -8,7 +8,6 @@
 
 #include "base/logging.h"
 #include "content/public/renderer/render_frame.h"
-#include "content/public/renderer/render_thread.h"
 #include "services/device/public/cpp/generic_sensor/motion_data.h"
 #include "services/device/public/mojom/sensor.mojom.h"
 #include "services/service_manager/public/cpp/interface_provider.h"
@@ -25,7 +24,9 @@ DeviceMotionEventPump::DeviceMotionEventPump()
           device::mojom::SensorType::LINEAR_ACCELERATION),
       gyroscope_(this, device::mojom::SensorType::GYROSCOPE) {}
 
-DeviceMotionEventPump::~DeviceMotionEventPump() {}
+DeviceMotionEventPump::~DeviceMotionEventPump() {
+  StopIfObserving();
+}
 
 void DeviceMotionEventPump::SendStartMessage() {
   if (!sensor_provider_) {

content/renderer/device_sensors/device_orientation_event_pump.cc

@@ -8,7 +8,6 @@
 
 #include "base/logging.h"
 #include "content/public/renderer/render_frame.h"
-#include "content/public/renderer/render_thread.h"
 #include "services/device/public/mojom/sensor.mojom.h"
 #include "services/service_manager/public/cpp/interface_provider.h"
 
@@ -54,7 +53,9 @@ DeviceOrientationEventPump::DeviceOrientationEventPump(bool absolute)
       absolute_(absolute),
       fall_back_to_absolute_orientation_sensor_(!absolute) {}
 
-DeviceOrientationEventPump::~DeviceOrientationEventPump() {}
+DeviceOrientationEventPump::~DeviceOrientationEventPump() {
+  StopIfObserving();
+}
 
 void DeviceOrientationEventPump::SendStartMessage() {
   if (!sensor_provider_) {

content/renderer/device_sensors/device_sensor_event_pump.h

@@ -15,10 +15,9 @@
 #include "base/macros.h"
 #include "base/time/time.h"
 #include "base/timer/timer.h"
+#include "content/common/content_export.h"
 #include "content/public/common/service_names.mojom.h"
-#include "content/public/renderer/platform_event_observer.h"
 #include "content/public/renderer/render_frame.h"
-#include "content/public/renderer/render_thread.h"
 #include "mojo/public/cpp/bindings/binding.h"
 #include "mojo/public/cpp/bindings/interface_request.h"
 #include "services/device/public/cpp/generic_sensor/sensor_reading.h"
@@ -33,8 +32,7 @@
 namespace content {
 
 template <typename ListenerType>
-class CONTENT_EXPORT DeviceSensorEventPump
-    : public PlatformEventObserver<ListenerType> {
+class CONTENT_EXPORT DeviceSensorEventPump {
  public:
   // Default rate for firing events.
   static constexpr int kDefaultPumpFrequencyHz = 60;
@@ -65,8 +63,7 @@ class CONTENT_EXPORT DeviceSensorEventPump
     SUSPENDED
   };
 
-  // PlatformEventObserver:
-  void Start(blink::WebPlatformEventListener* listener) override {
+  virtual void Start(blink::WebPlatformEventListener* listener) {
     DVLOG(2) << "requested start";
 
     if (state_ != PumpState::STOPPED)
@@ -75,11 +72,15 @@ class CONTENT_EXPORT DeviceSensorEventPump
     DCHECK(!timer_.IsRunning());
 
     state_ = PumpState::PENDING_START;
-    PlatformEventObserver<ListenerType>::Start(listener);
+
+    DCHECK(!is_observing_);
+    listener_ = static_cast<ListenerType*>(listener);
+    is_observing_ = true;
+
+    SendStartMessage();
   }
 
-  // PlatformEventObserver:
-  void Stop() override {
+  virtual void Stop() {
     DVLOG(2) << "requested stop";
 
     if (state_ == PumpState::STOPPED)
@@ -91,7 +92,12 @@ class CONTENT_EXPORT DeviceSensorEventPump
     if (timer_.IsRunning())
       timer_.Stop();
 
-    PlatformEventObserver<ListenerType>::Stop();
+    DCHECK(is_observing_);
+    listener_ = nullptr;
+    is_observing_ = false;
+
+    SendStopMessage();
+
     state_ = PumpState::STOPPED;
   }
 
@@ -105,14 +111,34 @@ class CONTENT_EXPORT DeviceSensorEventPump
   PumpState GetPumpStateForTesting() { return state_; }
 
  protected:
-  DeviceSensorEventPump() : state_(PumpState::STOPPED) {}
-
-  ~DeviceSensorEventPump() override {
-    PlatformEventObserver<ListenerType>::StopIfObserving();
+  DeviceSensorEventPump()
+      : state_(PumpState::STOPPED), is_observing_(false), listener_(nullptr) {}
+
+  virtual ~DeviceSensorEventPump() { DCHECK(!is_observing_); }
+
+  // This method is expected to send an IPC to the browser process to let it
+  // know that it should start observing.
+  // It is expected for subclasses to override it.
+  virtual void SendStartMessage() = 0;
+
+  // This method is expected to send an IPC to the browser process to let it
+  // know that it should start observing.
+  // It is expected for subclasses to override it.
+  virtual void SendStopMessage() = 0;
+
+  // Implementations of DeviceSensorEventPump must call StopIfObserving()
+  // from their destructor to shutdown in an orderly manner.
+  // (As Stop() calls a virtual method, it cannot be handled by
+  // ~DeviceSensorEventPump.)
+  void StopIfObserving() {
+    if (is_observing_)
+      Stop();
   }
 
   virtual void FireEvent() = 0;
 
+  ListenerType* listener() { return listener_; }
+
   struct SensorEntry : public device::mojom::SensorClient {
     SensorEntry(DeviceSensorEventPump* pump,
                 device::mojom::SensorType sensor_type)
@@ -319,6 +345,8 @@ class CONTENT_EXPORT DeviceSensorEventPump
 
   PumpState state_;
   base::RepeatingTimer timer_;
+  bool is_observing_;
+  ListenerType* listener_;
 
   DISALLOW_COPY_AND_ASSIGN(DeviceSensorEventPump);
 };

content/renderer/renderer_blink_platform_impl.cc

@@ -42,8 +42,6 @@
 #include "content/public/renderer/media_stream_utils.h"
 #include "content/public/renderer/render_frame.h"
 #include "content/renderer/blob_storage/webblobregistry_impl.h"
-#include "content/renderer/device_sensors/device_motion_event_pump.h"
-#include "content/renderer/device_sensors/device_orientation_event_pump.h"
 #include "content/renderer/dom_storage/local_storage_cached_areas.h"
 #include "content/renderer/dom_storage/local_storage_namespace.h"
 #include "content/renderer/dom_storage/session_web_storage_namespace_impl.h"
@@ -1120,12 +1118,6 @@ std::unique_ptr<PlatformEventObserverBase>
 RendererBlinkPlatformImpl::CreatePlatformEventObserverFromType(
     blink::WebPlatformEventType type) {
   switch (type) {
-    case blink::kWebPlatformEventTypeDeviceMotion:
-      return std::make_unique<DeviceMotionEventPump>();
-    case blink::kWebPlatformEventTypeDeviceOrientation:
-      return std::make_unique<DeviceOrientationEventPump>(false /* absolute */);
-    case blink::kWebPlatformEventTypeDeviceOrientationAbsolute:
-      return std::make_unique<DeviceOrientationEventPump>(true /* absolute */);
     case blink::kWebPlatformEventTypeGamepad:
       return std::make_unique<GamepadSharedMemoryReader>();
     default:
@@ -1154,28 +1146,96 @@ blink::InterfaceProvider* RendererBlinkPlatformImpl::GetInterfaceProvider() {
   return blink_interface_provider_.get();
 }
 
+void RendererBlinkPlatformImpl::InitDeviceSensorEventPump(
+    blink::WebPlatformEventType type,
+    blink::WebPlatformEventListener* listener) {
+  switch (type) {
+    case blink::kWebPlatformEventTypeDeviceMotion:
+      if (!motion_event_pump_)
+        motion_event_pump_ = std::make_unique<DeviceMotionEventPump>();
+      motion_event_pump_->Start(listener);
+      break;
+    case blink::kWebPlatformEventTypeDeviceOrientation:
+      if (!orientation_event_pump_) {
+        orientation_event_pump_ =
+            std::make_unique<DeviceOrientationEventPump>(false /* absolute */);
+      }
+      orientation_event_pump_->Start(listener);
+      break;
+    case blink::kWebPlatformEventTypeDeviceOrientationAbsolute:
+      if (!absolute_orientation_event_pump_) {
+        absolute_orientation_event_pump_ =
+            std::make_unique<DeviceOrientationEventPump>(true /* absolute */);
+      }
+      absolute_orientation_event_pump_->Start(listener);
+      break;
+    default:
+      DVLOG(1) << "RendererBlinkPlatformImpl::InitDeviceSensorEventPump() "
+                  "with unknown type.";
+  }
+}
+
+void RendererBlinkPlatformImpl::StopDeviceSensorEventPump(
+    blink::WebPlatformEventType type) {
+  switch (type) {
+    case blink::kWebPlatformEventTypeDeviceMotion: {
+      if (motion_event_pump_)
+        motion_event_pump_->Stop();
+      break;
+    }
+    case blink::kWebPlatformEventTypeDeviceOrientation: {
+      if (orientation_event_pump_)
+        orientation_event_pump_->Stop();
+      break;
+    }
+    case blink::kWebPlatformEventTypeDeviceOrientationAbsolute: {
+      if (absolute_orientation_event_pump_)
+        absolute_orientation_event_pump_->Stop();
+      break;
+    }
+    default:
+      DVLOG(1) << "RendererBlinkPlatformImpl::StopDeviceSensorEventPump() "
+                  "with unknown type.";
+  }
+}
 void RendererBlinkPlatformImpl::StartListening(
     blink::WebPlatformEventType type,
     blink::WebPlatformEventListener* listener) {
-  PlatformEventObserverBase* observer = platform_event_observers_.Lookup(type);
-  if (!observer) {
-    std::unique_ptr<PlatformEventObserverBase> new_observer =
-        CreatePlatformEventObserverFromType(type);
-    if (!new_observer)
-      return;
-    observer = new_observer.get();
-    platform_event_observers_.AddWithID(std::move(new_observer),
-                                        static_cast<int32_t>(type));
+  if (type == blink::kWebPlatformEventTypeDeviceMotion ||
+      type == blink::kWebPlatformEventTypeDeviceOrientation ||
+      type == blink::kWebPlatformEventTypeDeviceOrientationAbsolute) {
+    // this method creates DeviceSensorEventPump instances if necessary and
+    // calls Start() on them
+    InitDeviceSensorEventPump(type, listener);
+  } else {
+    PlatformEventObserverBase* observer =
+        platform_event_observers_.Lookup(type);
+    if (!observer) {
+      std::unique_ptr<PlatformEventObserverBase> new_observer =
+          CreatePlatformEventObserverFromType(type);
+      if (!new_observer)
+        return;
+      observer = new_observer.get();
+      platform_event_observers_.AddWithID(std::move(new_observer),
+                                          static_cast<int32_t>(type));
+    }
+    observer->Start(listener);
   }
-  observer->Start(listener);
 }
 
 void RendererBlinkPlatformImpl::StopListening(
     blink::WebPlatformEventType type) {
-  PlatformEventObserverBase* observer = platform_event_observers_.Lookup(type);
-  if (!observer)
-    return;
-  observer->Stop();
+  if (type == blink::kWebPlatformEventTypeDeviceMotion ||
+      type == blink::kWebPlatformEventTypeDeviceOrientation ||
+      type == blink::kWebPlatformEventTypeDeviceOrientationAbsolute) {
+    StopDeviceSensorEventPump(type);
+  } else {
+    PlatformEventObserverBase* observer =
+        platform_event_observers_.Lookup(type);
+    if (!observer)
+      return;
+    observer->Stop();
+  }
 }
 
 //------------------------------------------------------------------------------

content/renderer/renderer_blink_platform_impl.h

@@ -21,6 +21,8 @@
 #include "content/child/blink_platform_impl.h"
 #include "content/common/content_export.h"
 #include "content/common/possibly_associated_interface_ptr.h"
+#include "content/renderer/device_sensors/device_motion_event_pump.h"
+#include "content/renderer/device_sensors/device_orientation_event_pump.h"
 #include "content/renderer/top_level_blame_context.h"
 #include "content/renderer/webpublicsuffixlist_impl.h"
 #include "services/network/public/mojom/url_loader_factory.mojom.h"
@@ -266,6 +268,15 @@ class CONTENT_EXPORT RendererBlinkPlatformImpl : public BlinkPlatformImpl {
   static std::unique_ptr<PlatformEventObserverBase>
   CreatePlatformEventObserverFromType(blink::WebPlatformEventType type);
 
+  // TODO(crbug.com/850997): Remove when Device*EventPump classes are
+  // moved to blink
+  void InitDeviceSensorEventPump(blink::WebPlatformEventType type,
+                                 blink::WebPlatformEventListener* listener);
+
+  // TODO(crbug.com/850997): Remove when Device*EventPump classes are
+  // moved to blink
+  void StopDeviceSensorEventPump(blink::WebPlatformEventType type);
+
   // Ensure that the WebDatabaseHost has been initialized.
   void InitializeWebDatabaseHostIfNeeded();
 
@@ -308,6 +319,12 @@ class CONTENT_EXPORT RendererBlinkPlatformImpl : public BlinkPlatformImpl {
   base::IDMap<std::unique_ptr<PlatformEventObserverBase>>
       platform_event_observers_;
 
+  // TODO(crbug.com/850997): Remove when Device*EventPump classes are
+  // moved to blink
+  std::unique_ptr<DeviceMotionEventPump> motion_event_pump_;
+  std::unique_ptr<DeviceOrientationEventPump> orientation_event_pump_;
+  std::unique_ptr<DeviceOrientationEventPump> absolute_orientation_event_pump_;
+
   // NOT OWNED
   blink::scheduler::WebThreadScheduler* main_thread_scheduler_;