Refactor DeviceMotionEventPump to use //device/generic_sensor instead of //device/sensors

//device/generic_sensors already has all the sensors that can be used
to implement the DeviceOrientation event:
https://w3c.github.io/deviceorientation/spec-source-orientation.html

Currently, //content/renderer/device_sensors uses sensors from
//device/sensors as its backend, and this is one of the CLs that refactor
//content/renderer/device_sensors to use sensors from
//device/generic_sensor as the backend and removes //device/sensors.

This CL refactors DeviceMotionEvent to use sensors from //device/generic_sensor.

The issue page contains the design doc link for this change.

BUG=721427

Review-Url: https://codereview.chromium.org/2896583005
Cr-Commit-Position: refs/heads/master@{#480934}

content/renderer/BUILD.gn

@@ -458,6 +458,7 @@ target(link_target_type, "renderer") {
     "//device/base/synchronization",
     "//device/gamepad/public/cpp:shared_with_blink",
     "//device/gamepad/public/interfaces",
+    "//device/generic_sensor/public/cpp",
     "//device/screen_orientation/public/interfaces",
     "//device/sensors/public/cpp:full",
     "//device/sensors/public/interfaces",

content/renderer/DEPS

@@ -18,6 +18,8 @@ include_rules = [
   "+device/base/synchronization",
   "+device/gamepad/public/cpp",
   "+device/gamepad/public/interfaces",
+  "+device/generic_sensor/public/cpp",
+  "+device/generic_sensor/public/interfaces",
   "+device/screen_orientation/public/interfaces",
   "+device/sensors/public",
   "+device/usb/public",

content/renderer/device_sensors/device_motion_event_pump.h

@@ -6,38 +6,105 @@
 #define CONTENT_RENDERER_DEVICE_SENSORS_DEVICE_MOTION_EVENT_PUMP_H_
 
 #include <memory>
+#include <utility>
+#include <vector>
 
+#include "base/bind.h"
+#include "base/bind_helpers.h"
 #include "base/macros.h"
-#include "content/renderer/device_sensors/device_sensor_event_pump.h"
-#include "content/renderer/shared_memory_seqlock_reader.h"
+#include "base/time/time.h"
+#include "base/timer/timer.h"
+#include "content/public/renderer/platform_event_observer.h"
+#include "content/renderer/render_thread_impl.h"
+#include "device/generic_sensor/public/cpp/sensor_reading.h"
+#include "device/generic_sensor/public/interfaces/sensor.mojom.h"
+#include "device/generic_sensor/public/interfaces/sensor_provider.mojom.h"
 #include "device/sensors/public/cpp/motion_data.h"
-#include "device/sensors/public/interfaces/motion.mojom.h"
-
-namespace blink {
-class WebDeviceMotionListener;
-}
+#include "mojo/public/cpp/bindings/binding.h"
+#include "third_party/WebKit/public/platform/modules/device_orientation/WebDeviceMotionListener.h"
 
 namespace content {
 
-typedef SharedMemorySeqLockReader<device::MotionData>
-    DeviceMotionSharedMemoryReader;
-
 class CONTENT_EXPORT DeviceMotionEventPump
-    : public DeviceSensorMojoClientMixin<
-          DeviceSensorEventPump<blink::WebDeviceMotionListener>,
-          device::mojom::MotionSensor> {
+    : NON_EXPORTED_BASE(
+          public PlatformEventObserver<blink::WebDeviceMotionListener>) {
  public:
   explicit DeviceMotionEventPump(RenderThread* thread);
   ~DeviceMotionEventPump() override;
 
-  // PlatformEventObserver.
+  // PlatformEventObserver:
+  void Start(blink::WebPlatformEventListener* listener) override;
+  void Stop() override;
+  void SendStartMessage() override;
+  void SendStopMessage() override;
   void SendFakeDataForTesting(void* fake_data) override;
 
  protected:
-  void FireEvent() override;
-  bool InitializeReader(base::SharedMemoryHandle handle) override;
+  // Default rate for firing events.
+  static constexpr int kDefaultPumpFrequencyHz = 60;
+  static constexpr int kDefaultPumpDelayMicroseconds =
+      base::Time::kMicrosecondsPerSecond / kDefaultPumpFrequencyHz;
+
+  struct CONTENT_EXPORT SensorEntry : public device::mojom::SensorClient {
+    SensorEntry(DeviceMotionEventPump* pump,
+                device::mojom::SensorType sensor_type);
+    ~SensorEntry() override;
+
+    // device::mojom::SensorClient:
+    void RaiseError() override;
+    void SensorReadingChanged() override;
+
+    // Mojo callback for SensorProvider::GetSensor().
+    void OnSensorCreated(device::mojom::SensorInitParamsPtr params,
+                         device::mojom::SensorClientRequest client_request);
+
+    // Mojo callback for Sensor::AddConfiguration().
+    void OnSensorAddConfiguration(bool success);
+
+    void HandleSensorError();
+
+    bool SensorReadingCouldBeRead();
+
+    DeviceMotionEventPump* event_pump;
+    device::mojom::SensorPtr sensor;
+    device::mojom::SensorType type;
+    device::mojom::ReportingMode mode;
+    device::PlatformSensorConfiguration default_config;
+    mojo::ScopedSharedBufferHandle shared_buffer_handle;
+    mojo::ScopedSharedBufferMapping shared_buffer;
+    device::SensorReading reading;
+    mojo::Binding<device::mojom::SensorClient> client_binding;
+  };
+
+  friend struct SensorEntry;
+
+  virtual void FireEvent();
+
+  void DidStart();
+
+  SensorEntry accelerometer_;
+  SensorEntry linear_acceleration_sensor_;
+  SensorEntry gyroscope_;
+
+ private:
+  // TODO(juncai): refactor DeviceMotionEventPump to use DeviceSensorEventPump
+  // when refactoring DeviceOrientation.
+  //
+  // The pump is a tri-state automaton with allowed transitions as follows:
+  // STOPPED -> PENDING_START
+  // PENDING_START -> RUNNING
+  // PENDING_START -> STOPPED
+  // RUNNING -> STOPPED
+  enum class PumpState { STOPPED, RUNNING, PENDING_START };
+
+  bool CanStart() const;
+  void GetDataFromSharedMemory(device::MotionData* data);
+  void GetSensor(SensorEntry* sensor_entry);
+  void HandleSensorProviderError();
 
-  std::unique_ptr<DeviceMotionSharedMemoryReader> reader_;
+  mojo::InterfacePtr<device::mojom::SensorProvider> sensor_provider_;
+  PumpState state_;
+  base::RepeatingTimer timer_;
 
   DISALLOW_COPY_AND_ASSIGN(DeviceMotionEventPump);
 };

content/test/BUILD.gn

@@ -1529,6 +1529,7 @@ test("content_unittests") {
     "//device/gamepad",
     "//device/gamepad:test_helpers",
     "//device/gamepad/public/cpp:shared_with_blink",
+    "//device/generic_sensor/public/cpp",
     "//device/sensors/public/cpp:full",
     "//device/sensors/public/interfaces",
     "//gin",

content/test/DEPS

@@ -12,6 +12,7 @@ include_rules = [
   "+content",
   "+device/bluetooth", # For WebBluetooth tests
   "+device/gamepad/public/cpp",
+  "+device/generic_sensor/public/cpp",
   "+device/sensors/public/cpp",
   # For loading V8's initial snapshot from external files.
   "+gin/v8_initializer.h",

content/test/data/device_sensors/device_motion_only_some_sensors_are_available_test.html

+<html>
+  <head>
+    <title>DeviceMotion only some sensors are available test</title>
+    <script type="text/javascript">
+      let expectedInterval = Math.floor(1000 / 60);
+      function checkMotionEvent(event) {
+        return event.acceleration.x == 1 &&
+               event.acceleration.y == 2 &&
+               event.acceleration.z == 3 &&
+               event.accelerationIncludingGravity.x == null &&
+               event.accelerationIncludingGravity.y == null &&
+               event.accelerationIncludingGravity.z == null &&
+               event.rotationRate.alpha == 7 &&
+               event.rotationRate.beta == 8 &&
+               event.rotationRate.gamma == 9 &&
+               event.interval == expectedInterval;
+      }
+
+      function onMotion(event) {
+        if (checkMotionEvent(event)) {
+          window.removeEventListener('devicemotion', onMotion);
+          pass();
+        } else {
+          fail();
+        }
+      }
+
+      function pass() {
+        document.getElementById('status').innerHTML = 'PASS';
+        document.location = '#pass';
+      }
+
+      function fail() {
+        document.location = '#fail';
+      }
+    </script>
+  </head>
+  <body onLoad="window.addEventListener('devicemotion', onMotion)">
+    <div id="status">FAIL</div>
+  </body>
+</html>

content/test/data/device_sensors/device_motion_test.html

@@ -2,6 +2,7 @@
   <head>
     <title>DeviceMotion test</title>
     <script type="text/javascript">
+      let expectedInterval = Math.floor(1000 / 60);
       function checkMotionEvent(event) {
         return event.acceleration.x == 1 &&
                event.acceleration.y == 2 &&
@@ -12,7 +13,7 @@
                event.rotationRate.alpha == 7 &&
                event.rotationRate.beta == 8 &&
                event.rotationRate.gamma == 9 &&
-               event.interval == 100;
+               event.interval == expectedInterval;
       }
 
       function onMotion(event) {

device/generic_sensor/BUILD.gn

@@ -52,6 +52,7 @@ component("generic_sensor") {
   deps = [
     "//base",
     "//device/base/synchronization",
+    "//services/device/public/cpp:device_features",
   ]
 
   public_deps = [

device/generic_sensor/DEPS

 include_rules = [
   "+device/base/synchronization",
   "+jni",
+  "+services/device/public/cpp/device_features.h",
   "+third_party/sudden_motion_sensor",
 ]

device/generic_sensor/sensor_provider_impl.cc

@@ -6,11 +6,13 @@
 
 #include <utility>
 
+#include "base/feature_list.h"
 #include "base/memory/ptr_util.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "device/generic_sensor/platform_sensor_provider.h"
 #include "device/generic_sensor/sensor_impl.h"
 #include "mojo/public/cpp/bindings/strong_binding.h"
+#include "services/device/public/cpp/device_features.h"
 
 namespace device {
 
@@ -54,6 +56,17 @@ SensorProviderImpl::~SensorProviderImpl() {}
 void SensorProviderImpl::GetSensor(mojom::SensorType type,
                                    mojom::SensorRequest sensor_request,
                                    GetSensorCallback callback) {
+  // TODO(juncai): remove when the GenericSensor feature goes stable.
+  // For sensors that are used by DeviceMotionEvent, don't check the
+  // features::kGenericSensor flag.
+  if (!base::FeatureList::IsEnabled(features::kGenericSensor) &&
+      !(type == mojom::SensorType::ACCELEROMETER ||
+        type == mojom::SensorType::LINEAR_ACCELERATION ||
+        type == mojom::SensorType::GYROSCOPE)) {
+    NotifySensorCreated(nullptr, nullptr, std::move(callback));
+    return;
+  }
+
   auto cloned_handle = provider_->CloneSharedBufferHandle();
   if (!cloned_handle.is_valid()) {
     NotifySensorCreated(nullptr, nullptr, std::move(callback));

services/device/device_service.cc

@@ -7,7 +7,6 @@
 #include <utility>
 
 #include "base/bind.h"
-#include "base/feature_list.h"
 #include "base/memory/ptr_util.h"
 #include "base/memory/weak_ptr.h"
 #include "base/single_thread_task_runner.h"
@@ -18,7 +17,6 @@
 #include "mojo/public/cpp/system/message_pipe.h"
 #include "services/device/fingerprint/fingerprint.h"
 #include "services/device/power_monitor/power_monitor_message_broadcaster.h"
-#include "services/device/public/cpp/device_features.h"
 #include "services/device/public/interfaces/battery_monitor.mojom.h"
 #include "services/device/time_zone_monitor/time_zone_monitor.h"
 #include "services/service_manager/public/cpp/bind_source_info.h"
@@ -90,8 +88,6 @@ DeviceService::~DeviceService() {
 void DeviceService::OnStart() {
   registry_.AddInterface<mojom::Fingerprint>(base::Bind(
       &DeviceService::BindFingerprintRequest, base::Unretained(this)));
-  registry_.AddInterface<mojom::MotionSensor>(base::Bind(
-      &DeviceService::BindMotionSensorRequest, base::Unretained(this)));
   registry_.AddInterface<mojom::OrientationSensor>(base::Bind(
       &DeviceService::BindOrientationSensorRequest, base::Unretained(this)));
   registry_.AddInterface<mojom::OrientationAbsoluteSensor>(
@@ -102,10 +98,8 @@ void DeviceService::OnStart() {
   registry_.AddInterface<mojom::ScreenOrientationListener>(
       base::Bind(&DeviceService::BindScreenOrientationListenerRequest,
                  base::Unretained(this)));
-  if (base::FeatureList::IsEnabled(features::kGenericSensor)) {
-    registry_.AddInterface<mojom::SensorProvider>(base::Bind(
-        &DeviceService::BindSensorProviderRequest, base::Unretained(this)));
-  }
+  registry_.AddInterface<mojom::SensorProvider>(base::Bind(
+      &DeviceService::BindSensorProviderRequest, base::Unretained(this)));
   registry_.AddInterface<mojom::TimeZoneMonitor>(base::Bind(
       &DeviceService::BindTimeZoneMonitorRequest, base::Unretained(this)));
   registry_.AddInterface<mojom::WakeLockProvider>(base::Bind(
@@ -164,23 +158,6 @@ void DeviceService::BindFingerprintRequest(
   Fingerprint::Create(std::move(request));
 }
 
-void DeviceService::BindMotionSensorRequest(
-    const service_manager::BindSourceInfo& source_info,
-    mojom::MotionSensorRequest request) {
-#if defined(OS_ANDROID)
-  // On Android the device sensors implementations need to run on the UI thread
-  // to communicate to Java.
-  DeviceMotionHost::Create(std::move(request));
-#else
-  // On platforms other than Android the device sensors implementations run on
-  // the IO thread.
-  if (io_task_runner_) {
-    io_task_runner_->PostTask(FROM_HERE, base::Bind(&DeviceMotionHost::Create,
-                                                    base::Passed(&request)));
-  }
-#endif  // defined(OS_ANDROID)
-}
-
 void DeviceService::BindOrientationSensorRequest(
     const service_manager::BindSourceInfo& source_info,
     mojom::OrientationSensorRequest request) {

services/device/device_service.h

@@ -8,7 +8,6 @@
 #include "base/memory/ref_counted.h"
 #include "device/generic_sensor/public/interfaces/sensor_provider.mojom.h"
 #include "device/screen_orientation/public/interfaces/screen_orientation.mojom.h"
-#include "device/sensors/public/interfaces/motion.mojom.h"
 #include "device/sensors/public/interfaces/orientation.mojom.h"
 #include "device/wake_lock/public/interfaces/wake_lock_provider.mojom.h"
 #include "device/wake_lock/wake_lock_context.h"
@@ -75,10 +74,6 @@ class DeviceService : public service_manager::Service {
       const service_manager::BindSourceInfo& source_info,
       mojom::FingerprintRequest request);
 
-  void BindMotionSensorRequest(
-      const service_manager::BindSourceInfo& source_info,
-      mojom::MotionSensorRequest request);
-
   void BindOrientationSensorRequest(
       const service_manager::BindSourceInfo& source_info,
       mojom::OrientationSensorRequest request);