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

content/renderer/device_sensors/device_motion_event_pump.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "device_motion_event_pump.h"
-
+#include "content/renderer/device_sensors/device_motion_event_pump.h"
+
+#include "base/memory/ptr_util.h"
+#include "content/public/common/service_names.mojom.h"
 #include "content/public/renderer/render_thread.h"
+#include "services/device/public/interfaces/constants.mojom.h"
+#include "services/service_manager/public/cpp/connector.h"
 #include "third_party/WebKit/public/platform/modules/device_orientation/WebDeviceMotionListener.h"
 
 namespace content {
 
 DeviceMotionEventPump::DeviceMotionEventPump(RenderThread* thread)
-    : DeviceSensorMojoClientMixin<
-          DeviceSensorEventPump<blink::WebDeviceMotionListener>,
-          device::mojom::MotionSensor>(thread) {}
+    : PlatformEventObserver<blink::WebDeviceMotionListener>(thread),
+      pump_delay_microseconds_(kDefaultPumpDelayMicroseconds),
+      num_available_sensors_(0),
+      num_sensors_tried_(0),
+      state_(PumpState::STOPPED) {
+  auto request = mojo::MakeRequest(&sensor_provider_);
+
+  // When running layout tests, those observers should not listen to the
+  // actual hardware changes. In order to make that happen, don't connect
+  // the other end of the mojo pipe to anything.
+  if (RenderThreadImpl::current() &&
+      !RenderThreadImpl::current()->layout_test_mode()) {
+    RenderThread::Get()->GetConnector()->BindInterface(
+        device::mojom::kServiceName, std::move(request));

[Reilly Grant (use Gerrit), 2017/05/22 20:29:46]

Move this logic to SendStartMessage so that GetSensor's assumption that
sensor_provider_ is not null is correct (the error handler can reset it).

[juncai, 2017/05/23 02:30:23]

Done.

+    sensor_provider_.set_connection_error_handler(
+        base::Bind(&DeviceMotionEventPump::HandleSensorProviderError,
+                   base::Unretained(this)));
+  }
+}
 
 DeviceMotionEventPump::~DeviceMotionEventPump() {
+  PlatformEventObserver<blink::WebDeviceMotionListener>::StopIfObserving();
+}
+
+void DeviceMotionEventPump::Start(blink::WebPlatformEventListener* listener) {
+  DVLOG(2) << "requested start";
+
+  if (state_ != PumpState::STOPPED)
+    return;
+
+  DCHECK(!timer_.IsRunning());
+
+  PlatformEventObserver<blink::WebDeviceMotionListener>::Start(listener);
+  state_ = PumpState::PENDING_START;
+}
+
+void DeviceMotionEventPump::Stop() {
+  DVLOG(2) << "requested stop";
+
+  if (state_ == PumpState::STOPPED)
+    return;
+
+  DCHECK((state_ == PumpState::PENDING_START && !timer_.IsRunning()) ||
+         (state_ == PumpState::RUNNING && timer_.IsRunning()));
+
+  if (timer_.IsRunning())
+    timer_.Stop();
+
+  PlatformEventObserver<blink::WebDeviceMotionListener>::Stop();
+  state_ = PumpState::STOPPED;
+}
+
+void DeviceMotionEventPump::SendStartMessage() {
+  GetSensor(device::mojom::SensorType::ACCELEROMETER);
+  GetSensor(device::mojom::SensorType::LINEAR_ACCELERATION);
+  GetSensor(device::mojom::SensorType::GYROSCOPE);
+}
+
+void DeviceMotionEventPump::SendStopMessage() {
+  sensors_.clear();
+  num_sensors_tried_ = 0;
+  num_available_sensors_ = 0;
+}
+
+void DeviceMotionEventPump::SendFakeDataForTesting(void* fake_data) {
+  blink::WebDeviceMotionData data =
+      *static_cast<blink::WebDeviceMotionData*>(fake_data);
+  listener()->DidChangeDeviceMotion(data);
+}
+
+DeviceMotionEventPump::SensorEntry::SensorEntry(DeviceMotionEventPump* pump)
+    : event_pump(pump), active(false), client_binding(this) {}
+
+DeviceMotionEventPump::SensorEntry::~SensorEntry() {
+  client_binding.Close();
+}
+
+void DeviceMotionEventPump::SensorEntry::RaiseError() {
+  HandleSensorError();
+}
+
+void DeviceMotionEventPump::SensorEntry::SensorReadingChanged() {
+  // Since DeviceMotionEventPump::FireEvent is called in a certain
+  // frequency, the |shared_buffer| is read frequently, so this
+  // method doesn't need to be implemented.
+}
+
+void DeviceMotionEventPump::SensorEntry::OnSensorCreated(
+    device::mojom::SensorInitParamsPtr params,
+    device::mojom::SensorClientRequest client_request) {
+  ++event_pump->num_sensors_tried_;
+
+  if (!params) {
+    HandleSensorError();
+    if (event_pump->CanStart())
+      event_pump->DidStart();
+    return;
+  }
+
+  constexpr size_t kReadBufferSize = sizeof(device::SensorReadingSharedBuffer);
+
+  DCHECK_EQ(0u, params->buffer_offset % kReadBufferSize);
+
+  mode = params->mode;
+  default_config = params->default_configuration;
+
+  DCHECK(sensor.is_bound());
+  client_binding.Bind(std::move(client_request));
+
+  shared_buffer_handle = std::move(params->memory);
+  DCHECK(!shared_buffer);
+  shared_buffer =
+      shared_buffer_handle->MapAtOffset(kReadBufferSize, params->buffer_offset);
+
+  if (!shared_buffer) {
+    HandleSensorError();
+    if (event_pump->CanStart())
+      event_pump->DidStart();
+    return;
+  }
+
+  ++event_pump->num_available_sensors_;
+
+  frequency_limits.first = params->minimum_frequency;
+  frequency_limits.second = params->maximum_frequency;
+
+  DCHECK_GT(frequency_limits.first, 0.0);
+  DCHECK_GE(frequency_limits.second, frequency_limits.first);
+  constexpr double kMaxAllowedFrequency =
+      device::mojom::SensorConfiguration::kMaxAllowedFrequency;
+  DCHECK_GE(kMaxAllowedFrequency, frequency_limits.second);
+
+  sensor.set_connection_error_handler(
+      base::Bind(&SensorEntry::HandleSensorError, base::Unretained(this)));

[Reilly Grant (use Gerrit), 2017/05/22 20:29:46]

This should be set in GetSensor.

[juncai, 2017/05/23 02:30:23]

Done.

+  sensor->AddConfiguration(default_config,
+                           base::Bind(&SensorEntry::OnSensorAddConfiguration,
+                                      base::Unretained(this)));
+}
+
+void DeviceMotionEventPump::SensorEntry::OnSensorAddConfiguration(
+    bool success) {
+  active = success;
+  if (event_pump->CanStart())
+    event_pump->DidStart();
+}
+
+void DeviceMotionEventPump::SensorEntry::HandleSensorError() {
+  sensor.reset();
+  active = false;
+  shared_buffer_handle.reset();
+  shared_buffer.reset();
+  client_binding.Close();
+}
+
+bool DeviceMotionEventPump::SensorEntry::UpdateSensorReading() {
+  int read_attempts = 0;
+  device::SensorReading reading_data;
+  while (!TryReadFromBuffer(&reading_data)) {
+    if (++read_attempts == kMaxReadAttemptsCount) {
+      HandleSensorError();
+      return false;
+    }
+  }
+
+  reading = reading_data;
+  return true;
+}
+
+bool DeviceMotionEventPump::SensorEntry::TryReadFromBuffer(
+    device::SensorReading* result) {
+  const device::SensorReadingSharedBuffer* buffer =
+      static_cast<const device::SensorReadingSharedBuffer*>(
+          shared_buffer.get());
+  const device::OneWriterSeqLock& seqlock = buffer->seqlock.value();
+  auto version = seqlock.ReadBegin();
+  auto reading_data = buffer->reading;
+  if (seqlock.ReadRetry(version))
+    return false;
+  *result = reading_data;
+  return true;
+}
+
+bool DeviceMotionEventPump::SensorEntry::SensorReadingUpdated() {
+  return active && UpdateSensorReading();
 }
 
 void DeviceMotionEventPump::FireEvent() {
+  blink::WebDeviceMotionData data;
+  data.interval = kDefaultPumpDelayMicroseconds;
+
   DCHECK(listener());
-  device::MotionData data;
-  if (reader_->GetLatestData(&data) && data.all_available_sensors_are_active)
+
+  if (GetDataFromSharedMemory(&data))
     listener()->DidChangeDeviceMotion(data);
 }
 
-bool DeviceMotionEventPump::InitializeReader(base::SharedMemoryHandle handle) {
-  if (!reader_)
-    reader_.reset(new DeviceMotionSharedMemoryReader());
-  return reader_->Initialize(handle);
-}
-
-void DeviceMotionEventPump::SendFakeDataForTesting(void* fake_data) {
-  device::MotionData data = *static_cast<device::MotionData*>(fake_data);
-
-  listener()->DidChangeDeviceMotion(data);
+void DeviceMotionEventPump::DidStart() {
+  DVLOG(2) << "did start sensor event pump";
+
+  if (state_ != PumpState::PENDING_START)
+    return;
+
+  DCHECK(!timer_.IsRunning());
+
+  timer_.Start(FROM_HERE,
+               base::TimeDelta::FromMicroseconds(pump_delay_microseconds_),
+               this, &DeviceMotionEventPump::FireEvent);
+  state_ = PumpState::RUNNING;
+}
+
+bool DeviceMotionEventPump::CanStart() const {
+  DCHECK_EQ(3u, sensors_.size());
+
+  // device motion spec does not require all sensors to be available.

[Reilly Grant (use Gerrit), 2017/05/22 20:29:46]

This comment and the code seem to be at odds.

[juncai, 2017/05/23 02:30:23]

Updated code to not have a state that a sensor is available but not active. So
if a sensor completes the process of GetSensor() and AddConfiguration(), it is
active. Otherwise it is not active.

Done.

+  return (static_cast<size_t>(num_sensors_tried_) == sensors_.size()) &&
+         AllAvailableSensorsAreActive();
+}
+
+bool DeviceMotionEventPump::AllAvailableSensorsAreActive() const {
+  int num_active_sensors = 0;
+  for (const auto& sensor : sensors_) {
+    if (sensor->active)
+      ++num_active_sensors;
+  }
+
+  return num_available_sensors_ == num_active_sensors;
+}
+
+bool DeviceMotionEventPump::GetDataFromSharedMemory(
+    blink::WebDeviceMotionData* data) {
+  if (!AllAvailableSensorsAreActive())
+    return false;
+
+  bool has_data = false;
+  for (auto& sensor : sensors_) {
+    if (!sensor->SensorReadingUpdated())
+      continue;
+
+    double d0 = sensor->reading.values[0].value();
+    double d1 = sensor->reading.values[1].value();
+    double d2 = sensor->reading.values[2].value();
+    switch (sensor->type) {
+      case device::mojom::SensorType::ACCELEROMETER:
+        data->acceleration_including_gravity_x = d0;
+        data->acceleration_including_gravity_y = d1;
+        data->acceleration_including_gravity_z = d2;
+        data->has_acceleration_including_gravity_x = true;
+        data->has_acceleration_including_gravity_y = true;
+        data->has_acceleration_including_gravity_z = true;
+        has_data = true;
+        break;
+      case device::mojom::SensorType::LINEAR_ACCELERATION:
+        data->acceleration_x = d0;
+        data->acceleration_y = d1;
+        data->acceleration_z = d2;
+        data->has_acceleration_x = true;
+        data->has_acceleration_y = true;
+        data->has_acceleration_z = true;
+        has_data = true;
+        break;
+      case device::mojom::SensorType::GYROSCOPE:
+        data->rotation_rate_alpha = d0;
+        data->rotation_rate_beta = d1;
+        data->rotation_rate_gamma = d2;
+        data->has_rotation_rate_alpha = true;
+        data->has_rotation_rate_beta = true;
+        data->has_rotation_rate_gamma = true;
+        has_data = true;
+        break;
+      default:
+        NOTREACHED();
+        break;
+    }
+  }
+
+  return has_data;
+}
+
+void DeviceMotionEventPump::GetSensor(device::mojom::SensorType type) {
+  sensors_.push_back(base::MakeUnique<SensorEntry>(this));
+  SensorEntry* sensor_entry = sensors_.back().get();
+  sensor_entry->type = type;
+  auto request = mojo::MakeRequest(&sensor_entry->sensor);
+  sensor_provider_->GetSensor(type, std::move(request),
+                              base::Bind(&SensorEntry::OnSensorCreated,
+                                         base::Unretained(sensor_entry)));
+}
+
+void DeviceMotionEventPump::HandleSensorProviderError() {
+  sensor_provider_.reset();
+  for (auto& sensor : sensors_)
+    sensor->HandleSensorError();

[Reilly Grant (use Gerrit), 2017/05/22 20:29:46]

I don't think we need to close all the sensors in response to this error.

[juncai, 2017/05/23 02:30:23]

Done.

 }
 
 }  // namespace content