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

content/renderer/device_sensors/device_motion_event_pump.h

Index: content/renderer/device_sensors/device_motion_event_pump.h
diff --git a/content/renderer/device_sensors/device_motion_event_pump.h b/content/renderer/device_sensors/device_motion_event_pump.h
index 5846db199bb8a2e123874350752b4ee685c91cbc..61481be19cd54fad806a6c095b732d400f989cfa 100644
--- a/content/renderer/device_sensors/device_motion_event_pump.h
+++ b/content/renderer/device_sensors/device_motion_event_pump.h
@@ -6,38 +6,102 @@
 #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:
+  // 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 };

[timvolodine, 2017/06/07 20:37:36]

This is functionality from the device_sensor_event_pump base class originally
(with Start and Stop methods already implemented there). Can
DeviceSensorEventPump be re-used here?

[juncai, 2017/06/07 23:05:07]

Yes, it will be re-used eventually when the device orientation is refactored.
Similar to the previously closed bigger CL:
https://codereview.chromium.org/2885203004/diff/80001/content/renderer/device...

For now, I think it makes sense to have some duplicate code here. It makes this
CL cleaner by not modifying any orientation related code since this CL makes
DeviceMotionEventPump and DeviceOrientationEventPump not share
DeviceSensorEventPump code.

[timvolodine, 2017/06/08 20:26:51]

I though this CL is only about motion? I was wondering if it's possible to reuse
without touching base class or orientation..

[juncai, 2017/06/08 23:29:43]

This CL is only about motion. Based on what I see at the current
device_sensor_event_pump.h:
https://cs.chromium.org/chromium/src/content/renderer/device_sensors/device_s...
These functions can not be re-used.
The duplication here will eventually be removed when the device orientation is
refactored, so I think maybe it is not worth to reuse the current
device_sensor_event_pump.h since it will make device_motion_event_pump.h more
complicated.

[timvolodine, 2017/06/09 18:54:51]

I guess you could simply keep inheriting from DeviceSensorEventPump and
overriding DidStart (making it virtual).. but if you prefer to refactor later
e.g. when DeviceOrientation comes along fine with me. Please add a TODO though..

[juncai, 2017/06/13 23:02:18]

Added TODO.

Done.

+
+  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);
 };