Implement PlatformSensorProviderWinrt

As the second step for implementing usage of the
Windows.Devices.Sensors WinRT API in Chromium, this CL:
  - Finishes the implementation of PlatformSensorProviderWinrt.
  - Adds more unit tests to PlatformSensorProviderWinrt.
  - Abstracts out the PlatformSensorReaderWin interface so it can be
    generically reused by both the ISensor and Windows.Devices.Sensors
    implementation.

Verified by running unit tests.

Bug: 958266
Change-Id: I71783954d4db37f6e921d9c4eaac150763382722
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1693732
Commit-Queue: Wenjie Shi <wensh@microsoft.com>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#678414}

services/device/generic_sensor/BUILD.gn

@@ -54,6 +54,7 @@ source_set("generic_sensor") {
     "platform_sensor_reader_linux.h",
     "platform_sensor_reader_win.cc",
     "platform_sensor_reader_win.h",
+    "platform_sensor_reader_win_base.h",
     "platform_sensor_win.cc",
     "platform_sensor_win.h",
     "relative_orientation_euler_angles_fusion_algorithm_using_accelerometer.cc",
@@ -107,6 +108,8 @@ source_set("generic_sensor") {
     sources += [
       "platform_sensor_provider_winrt.cc",
       "platform_sensor_provider_winrt.h",
+      "platform_sensor_reader_winrt.cc",
+      "platform_sensor_reader_winrt.h",
     ]
 
     libs = [

services/device/generic_sensor/platform_sensor_provider_win.cc

@@ -108,7 +108,7 @@ void PlatformSensorProviderWin::SensorReaderCreated(
     mojom::SensorType type,
     SensorReadingSharedBuffer* reading_buffer,
     const CreateSensorCallback& callback,
-    std::unique_ptr<PlatformSensorReaderWin> sensor_reader) {
+    std::unique_ptr<PlatformSensorReaderWinBase> sensor_reader) {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
   if (!sensor_reader) {
     // Fallback options for sensors that can be implemented using sensor
@@ -135,12 +135,12 @@ void PlatformSensorProviderWin::SensorReaderCreated(
   callback.Run(sensor);
 }
 
-std::unique_ptr<PlatformSensorReaderWin>
+std::unique_ptr<PlatformSensorReaderWinBase>
 PlatformSensorProviderWin::CreateSensorReader(mojom::SensorType type) {
   DCHECK(com_sta_task_runner_->RunsTasksInCurrentSequence());
   if (!sensor_manager_)
     return nullptr;
-  return PlatformSensorReaderWin::Create(type, sensor_manager_);
+  return PlatformSensorReaderWin32::Create(type, sensor_manager_);
 }
 
 }  // namespace device

services/device/generic_sensor/platform_sensor_provider_win.h

@@ -12,7 +12,7 @@
 
 namespace device {
 
-class PlatformSensorReaderWin;
+class PlatformSensorReaderWinBase;
 
 // Implementation of PlatformSensorProvider for Windows platform.
 // PlatformSensorProviderWin is responsible for following tasks:
@@ -40,13 +40,13 @@ class PlatformSensorProviderWin final : public PlatformSensorProvider {
   void OnInitSensorManager(mojom::SensorType type,
                            SensorReadingSharedBuffer* reading_buffer,
                            const CreateSensorCallback& callback);
-  std::unique_ptr<PlatformSensorReaderWin> CreateSensorReader(
+  std::unique_ptr<PlatformSensorReaderWinBase> CreateSensorReader(
       mojom::SensorType type);
   void SensorReaderCreated(
       mojom::SensorType type,
       SensorReadingSharedBuffer* reading_buffer,
       const CreateSensorCallback& callback,
-      std::unique_ptr<PlatformSensorReaderWin> sensor_reader);
+      std::unique_ptr<PlatformSensorReaderWinBase> sensor_reader);
 
   scoped_refptr<base::SingleThreadTaskRunner> com_sta_task_runner_;
   Microsoft::WRL::ComPtr<ISensorManager> sensor_manager_;

services/device/generic_sensor/platform_sensor_provider_winrt.cc

@@ -4,17 +4,102 @@
 
 #include "services/device/generic_sensor/platform_sensor_provider_winrt.h"
 
+#include <comdef.h>
+
+#include "base/task/post_task.h"
+#include "base/task_runner_util.h"
+#include "services/device/generic_sensor/linear_acceleration_fusion_algorithm_using_accelerometer.h"
+#include "services/device/generic_sensor/orientation_euler_angles_fusion_algorithm_using_quaternion.h"
+#include "services/device/generic_sensor/platform_sensor_fusion.h"
+#include "services/device/generic_sensor/platform_sensor_reader_winrt.h"
+#include "services/device/generic_sensor/platform_sensor_win.h"
+
 namespace device {
 
-PlatformSensorProviderWinrt::PlatformSensorProviderWinrt() = default;
+std::unique_ptr<PlatformSensorReaderWinBase>
+SensorReaderFactory::CreateSensorReader(mojom::SensorType type) {
+  return PlatformSensorReaderWinrtFactory::Create(type);
+}
+
+PlatformSensorProviderWinrt::PlatformSensorProviderWinrt()
+    : com_sta_task_runner_(base::CreateCOMSTATaskRunnerWithTraits(
+          base::TaskPriority::USER_VISIBLE)),
+      sensor_reader_factory_(std::make_unique<SensorReaderFactory>()) {}
 
 PlatformSensorProviderWinrt::~PlatformSensorProviderWinrt() = default;
 
+void PlatformSensorProviderWinrt::SetSensorReaderFactoryForTesting(
+    std::unique_ptr<SensorReaderFactory> sensor_reader_factory) {
+  sensor_reader_factory_ = std::move(sensor_reader_factory);
+}
+
 void PlatformSensorProviderWinrt::CreateSensorInternal(
     mojom::SensorType type,
     SensorReadingSharedBuffer* reading_buffer,
     const CreateSensorCallback& callback) {
-  callback.Run(nullptr);
+  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+
+  switch (type) {
+    // Fusion sensor.
+    case mojom::SensorType::LINEAR_ACCELERATION: {
+      auto linear_acceleration_fusion_algorithm = std::make_unique<
+          LinearAccelerationFusionAlgorithmUsingAccelerometer>();
+      // If this PlatformSensorFusion object is successfully initialized,
+      // |callback| will be run with a reference to this object.
+      PlatformSensorFusion::Create(
+          reading_buffer, this, std::move(linear_acceleration_fusion_algorithm),
+          callback);
+      break;
+    }
+
+    // Try to create low-level sensors by default.
+    default: {
+      base::PostTaskAndReplyWithResult(
+          com_sta_task_runner_.get(), FROM_HERE,
+          base::Bind(&PlatformSensorProviderWinrt::CreateSensorReader,
+                     base::Unretained(this), type),
+          base::Bind(&PlatformSensorProviderWinrt::SensorReaderCreated,
+                     base::Unretained(this), type, reading_buffer, callback));
+      break;
+    }
+  }
+}
+
+void PlatformSensorProviderWinrt::SensorReaderCreated(
+    mojom::SensorType type,
+    SensorReadingSharedBuffer* reading_buffer,
+    const CreateSensorCallback& callback,
+    std::unique_ptr<PlatformSensorReaderWinBase> sensor_reader) {
+  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+  if (!sensor_reader) {
+    // Fallback options for sensors that can be implemented using sensor
+    // fusion. Note that it is important not to generate a cycle by adding a
+    // fallback here that depends on one of the other fallbacks provided.
+    switch (type) {
+      case mojom::SensorType::ABSOLUTE_ORIENTATION_EULER_ANGLES: {
+        auto algorithm = std::make_unique<
+            OrientationEulerAnglesFusionAlgorithmUsingQuaternion>(
+            /*absolute=*/true);
+        PlatformSensorFusion::Create(reading_buffer, this, std::move(algorithm),
+                                     std::move(callback));
+        return;
+      }
+      default:
+        callback.Run(nullptr);
+        return;
+    }
+  }
+  scoped_refptr<PlatformSensor> sensor =
+      base::MakeRefCounted<PlatformSensorWin>(type, reading_buffer, this,
+                                              com_sta_task_runner_,
+                                              std::move(sensor_reader));
+  callback.Run(std::move(sensor));
+}
+
+std::unique_ptr<PlatformSensorReaderWinBase>
+PlatformSensorProviderWinrt::CreateSensorReader(mojom::SensorType type) {
+  DCHECK(com_sta_task_runner_->RunsTasksInCurrentSequence());
+  return sensor_reader_factory_->CreateSensorReader(type);
 }
 
-}  // namespace device
+}  // namespace device
\ No newline at end of file

services/device/generic_sensor/platform_sensor_provider_winrt.h

@@ -9,7 +9,15 @@
 
 namespace device {
 
-class PlatformSensorReaderWin;
+class PlatformSensorReaderWinBase;
+
+// Helper class used to instantiate new PlatformSensorReaderWinBase instances.
+class SensorReaderFactory {
+ public:
+  virtual ~SensorReaderFactory() = default;
+  virtual std::unique_ptr<PlatformSensorReaderWinBase> CreateSensorReader(
+      mojom::SensorType type);
+};
 
 // Implementation of PlatformSensorProvider for Windows platform using the
 // Windows.Devices.Sensors WinRT API. PlatformSensorProviderWinrt is
@@ -22,6 +30,9 @@ class PlatformSensorProviderWinrt final : public PlatformSensorProvider {
   PlatformSensorProviderWinrt();
   ~PlatformSensorProviderWinrt() override;
 
+  void SetSensorReaderFactoryForTesting(
+      std::unique_ptr<SensorReaderFactory> sensor_reader_factory);
+
  protected:
   // PlatformSensorProvider interface implementation.
   void CreateSensorInternal(mojom::SensorType type,
@@ -29,11 +40,20 @@ class PlatformSensorProviderWinrt final : public PlatformSensorProvider {
                             const CreateSensorCallback& callback) override;
 
  private:
+  std::unique_ptr<PlatformSensorReaderWinBase> CreateSensorReader(
+      mojom::SensorType type);
+
   void SensorReaderCreated(
       mojom::SensorType type,
       SensorReadingSharedBuffer* reading_buffer,
       const CreateSensorCallback& callback,
-      std::unique_ptr<PlatformSensorReaderWin> sensor_reader);
+      std::unique_ptr<PlatformSensorReaderWinBase> sensor_reader);
+
+  // The Windows.Devices.Sensors WinRT API supports both STA and MTA
+  // threads. STA was chosen as PlatformSensorWin can only handle STA.
+  scoped_refptr<base::SingleThreadTaskRunner> com_sta_task_runner_;
+
+  std::unique_ptr<SensorReaderFactory> sensor_reader_factory_;
 
   PlatformSensorProviderWinrt(const PlatformSensorProviderWinrt&) = delete;
   PlatformSensorProviderWinrt& operator=(const PlatformSensorProviderWinrt&) =
@@ -42,4 +62,4 @@ class PlatformSensorProviderWinrt final : public PlatformSensorProvider {
 
 }  // namespace device
 
-#endif  // SERVICES_DEVICE_GENERIC_SENSOR_PLATFORM_SENSOR_PROVIDER_WINRT_H_
+#endif  // SERVICES_DEVICE_GENERIC_SENSOR_PLATFORM_SENSOR_PROVIDER_WINRT_H_
\ No newline at end of file

services/device/generic_sensor/platform_sensor_provider_winrt_unittest.cc

@@ -7,32 +7,100 @@
 #include "base/run_loop.h"
 #include "base/test/bind_test_util.h"
 #include "base/test/scoped_task_environment.h"
+#include "services/device/generic_sensor/platform_sensor_reader_win_base.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace device {
 
+// Mock for PlatformSensorReaderWinBase, used to ensure the actual
+// Windows.Devices.Sensor API isn't called.
+class MockSensorReader : public PlatformSensorReaderWinBase {
+ public:
+  MockSensorReader() = default;
+  ~MockSensorReader() override = default;
+
+  MOCK_METHOD1(SetClient, void(Client* client));
+  MOCK_CONST_METHOD0(GetMinimalReportingInterval, base::TimeDelta());
+  MOCK_METHOD1(StartSensor,
+               bool(const PlatformSensorConfiguration& configuration));
+  MOCK_METHOD0(StopSensor, void());
+};
+
+// Mock for SensorReaderFactory, injected into
+// PlatformSensorProviderWinrt so it will create MockSensorReaders instead
+// of the real PlatformSensorReaderWinBase which calls into the WinRT APIs.
+class MockSensorReaderFactory : public SensorReaderFactory {
+ public:
+  MockSensorReaderFactory() = default;
+  ~MockSensorReaderFactory() override = default;
+
+  MOCK_METHOD1(
+      CreateSensorReader,
+      std::unique_ptr<PlatformSensorReaderWinBase>(mojom::SensorType type));
+};
+
 // Tests that PlatformSensorProviderWinrt can successfully be instantiated
 // and passes the correct result to the CreateSensor callback.
 TEST(PlatformSensorProviderTestWinrt, SensorCreationReturnCheck) {
   base::test::ScopedTaskEnvironment scoped_task_environment;
 
+  auto mock_sensor_reader_factory =
+      std::make_unique<testing::NiceMock<MockSensorReaderFactory>>();
+
+  // Return valid PlatformSensorReaderWinBase instances for gyroscope and
+  // accelerometer to represent them as supported/present. Return nullptr
+  // for ambient light to represent it as not present/supported.
+  EXPECT_CALL(*mock_sensor_reader_factory.get(),
+              CreateSensorReader(mojom::SensorType::AMBIENT_LIGHT))
+      .WillOnce(testing::Invoke([](mojom::SensorType) { return nullptr; }));
+  EXPECT_CALL(*mock_sensor_reader_factory.get(),
+              CreateSensorReader(mojom::SensorType::GYROSCOPE))
+      .WillOnce(testing::Invoke([](mojom::SensorType) {
+        return std::make_unique<testing::NiceMock<MockSensorReader>>();
+      }));
+  EXPECT_CALL(*mock_sensor_reader_factory.get(),
+              CreateSensorReader(mojom::SensorType::ACCELEROMETER))
+      .WillOnce(testing::Invoke([](mojom::SensorType) {
+        return std::make_unique<testing::NiceMock<MockSensorReader>>();
+      }));
+
   auto provider = std::make_unique<PlatformSensorProviderWinrt>();
 
-  // CreateSensor is async so create a RunLoop to wait for completion.
-  base::RunLoop run_loop;
+  provider->SetSensorReaderFactoryForTesting(
+      std::move(mock_sensor_reader_factory));
+
+  // CreateSensor is async so use a RunLoop to wait for completion.
+  base::Optional<base::RunLoop> run_loop;
+  bool expect_sensor_valid = false;
 
   base::Callback<void(scoped_refptr<PlatformSensor> sensor)>
-      CreateSensorCallback =
+      create_sensor_callback =
           base::BindLambdaForTesting([&](scoped_refptr<PlatformSensor> sensor) {
-            EXPECT_FALSE(sensor);
-            run_loop.Quit();
+            if (expect_sensor_valid)
+              EXPECT_TRUE(sensor);
+            else
+              EXPECT_FALSE(sensor);
+            run_loop->Quit();
           });
 
-  // PlatformSensorProviderWinrt is not implemented so it should always
-  // return nullptr.
+  run_loop.emplace();
   provider->CreateSensor(mojom::SensorType::AMBIENT_LIGHT,
-                         CreateSensorCallback);
-  run_loop.Run();
+                         create_sensor_callback);
+  run_loop->Run();
+
+  expect_sensor_valid = true;
+  run_loop.emplace();
+  provider->CreateSensor(mojom::SensorType::GYROSCOPE, create_sensor_callback);
+  run_loop->Run();
+
+  // Linear acceleration is a fusion sensor built on top of accelerometer,
+  // this should trigger the CreateSensorReader(ACCELEROMETER) call.
+  expect_sensor_valid = true;
+  run_loop.emplace();
+  provider->CreateSensor(mojom::SensorType::LINEAR_ACCELERATION,
+                         create_sensor_callback);
+  run_loop->Run();
 }
 
 }  // namespace device
\ No newline at end of file

services/device/generic_sensor/platform_sensor_reader_win.cc

@@ -24,7 +24,7 @@
 
 namespace device {
 
-// Init params for the PlatformSensorReaderWin.
+// Init params for the PlatformSensorReaderWin32.
 struct ReaderInitParams {
   // ISensorDataReport::GetSensorValue is not const, therefore, report
   // cannot be passed as const ref.
@@ -35,7 +35,7 @@ struct ReaderInitParams {
                                     SensorReading* reading);
   SENSOR_TYPE_ID sensor_type_id;
   ReaderFunctor reader_func;
-  unsigned long min_reporting_interval_ms = 0;
+  base::TimeDelta min_reporting_interval;
 };
 
 namespace {
@@ -252,7 +252,7 @@ std::unique_ptr<ReaderInitParams> CreateReaderInitParamsForSensor(
 // by ISensor interface to dispatch state and data change events.
 class EventListener : public ISensorEvents, public base::win::IUnknownImpl {
  public:
-  explicit EventListener(PlatformSensorReaderWin* platform_sensor_reader)
+  explicit EventListener(PlatformSensorReaderWin32* platform_sensor_reader)
       : platform_sensor_reader_(platform_sensor_reader) {
     DCHECK(platform_sensor_reader_);
   }
@@ -345,14 +345,14 @@ class EventListener : public ISensorEvents, public base::win::IUnknownImpl {
   }
 
  private:
-  PlatformSensorReaderWin* const platform_sensor_reader_;
+  PlatformSensorReaderWin32* const platform_sensor_reader_;
   SensorReading last_sensor_reading_;
 
   DISALLOW_COPY_AND_ASSIGN(EventListener);
 };
 
 // static
-std::unique_ptr<PlatformSensorReaderWin> PlatformSensorReaderWin::Create(
+std::unique_ptr<PlatformSensorReaderWinBase> PlatformSensorReaderWin32::Create(
     mojom::SensorType type,
     Microsoft::WRL::ComPtr<ISensorManager> sensor_manager) {
   DCHECK(sensor_manager);
@@ -368,8 +368,10 @@ std::unique_ptr<PlatformSensorReaderWin> PlatformSensorReaderWin::Create(
   base::win::ScopedPropVariant min_interval;
   HRESULT hr = sensor->GetProperty(SENSOR_PROPERTY_MIN_REPORT_INTERVAL,
                                    min_interval.Receive());
-  if (SUCCEEDED(hr) && min_interval.get().vt == VT_UI4)
-    params->min_reporting_interval_ms = min_interval.get().ulVal;
+  if (SUCCEEDED(hr) && min_interval.get().vt == VT_UI4) {
+    params->min_reporting_interval =
+        base::TimeDelta::FromMilliseconds(min_interval.get().ulVal);
+  }
 
   GUID interests[] = {SENSOR_EVENT_STATE_CHANGED, SENSOR_EVENT_DATA_UPDATED};
   hr = sensor->SetEventInterest(interests, base::size(interests));
@@ -377,11 +379,11 @@ std::unique_ptr<PlatformSensorReaderWin> PlatformSensorReaderWin::Create(
     return nullptr;
 
   return base::WrapUnique(
-      new PlatformSensorReaderWin(sensor, std::move(params)));
+      new PlatformSensorReaderWin32(sensor, std::move(params)));
 }
 
 // static
-Microsoft::WRL::ComPtr<ISensor> PlatformSensorReaderWin::GetSensorForType(
+Microsoft::WRL::ComPtr<ISensor> PlatformSensorReaderWin32::GetSensorForType(
     REFSENSOR_TYPE_ID sensor_type,
     Microsoft::WRL::ComPtr<ISensorManager> sensor_manager) {
   Microsoft::WRL::ComPtr<ISensor> sensor;
@@ -398,7 +400,7 @@ Microsoft::WRL::ComPtr<ISensor> PlatformSensorReaderWin::GetSensorForType(
   return sensor;
 }
 
-PlatformSensorReaderWin::PlatformSensorReaderWin(
+PlatformSensorReaderWin32::PlatformSensorReaderWin32(
     Microsoft::WRL::ComPtr<ISensor> sensor,
     std::unique_ptr<ReaderInitParams> params)
     : init_params_(std::move(params)),
@@ -413,13 +415,13 @@ PlatformSensorReaderWin::PlatformSensorReaderWin(
   DCHECK(sensor_);
 }
 
-void PlatformSensorReaderWin::SetClient(Client* client) {
+void PlatformSensorReaderWin32::SetClient(Client* client) {
   base::AutoLock autolock(lock_);
   // Can be null.
   client_ = client;
 }
 
-void PlatformSensorReaderWin::StopSensor() {
+void PlatformSensorReaderWin32::StopSensor() {
   base::AutoLock autolock(lock_);
   if (sensor_active_) {
     sensor_->SetEventSink(nullptr);
@@ -427,11 +429,11 @@ void PlatformSensorReaderWin::StopSensor() {
   }
 }
 
-PlatformSensorReaderWin::~PlatformSensorReaderWin() {
+PlatformSensorReaderWin32::~PlatformSensorReaderWin32() {
   DCHECK(task_runner_->BelongsToCurrentThread());
 }
 
-bool PlatformSensorReaderWin::StartSensor(
+bool PlatformSensorReaderWin32::StartSensor(
     const PlatformSensorConfiguration& configuration) {
   base::AutoLock autolock(lock_);
 
@@ -440,7 +442,7 @@ bool PlatformSensorReaderWin::StartSensor(
 
   if (!sensor_active_) {
     task_runner_->PostTask(
-        FROM_HERE, base::BindOnce(&PlatformSensorReaderWin::ListenSensorEvent,
+        FROM_HERE, base::BindOnce(&PlatformSensorReaderWin32::ListenSensorEvent,
                                   weak_factory_.GetWeakPtr()));
     sensor_active_ = true;
   }
@@ -448,7 +450,7 @@ bool PlatformSensorReaderWin::StartSensor(
   return true;
 }
 
-void PlatformSensorReaderWin::ListenSensorEvent() {
+void PlatformSensorReaderWin32::ListenSensorEvent() {
   // Set event listener.
   if (FAILED(sensor_->SetEventSink(event_listener_.get()))) {
     SensorError();
@@ -456,7 +458,7 @@ void PlatformSensorReaderWin::ListenSensorEvent() {
   }
 }
 
-bool PlatformSensorReaderWin::SetReportingInterval(
+bool PlatformSensorReaderWin32::SetReportingInterval(
     const PlatformSensorConfiguration& configuration) {
   Microsoft::WRL::ComPtr<IPortableDeviceValues> props;
   HRESULT hr = ::CoCreateInstance(CLSID_PortableDeviceValues, nullptr,
@@ -485,7 +487,7 @@ bool PlatformSensorReaderWin::SetReportingInterval(
   return SUCCEEDED(hr);
 }
 
-HRESULT PlatformSensorReaderWin::SensorReadingChanged(
+HRESULT PlatformSensorReaderWin32::SensorReadingChanged(
     ISensorDataReport* report,
     SensorReading* reading) const {
   if (!client_)
@@ -497,13 +499,13 @@ HRESULT PlatformSensorReaderWin::SensorReadingChanged(
   return hr;
 }
 
-void PlatformSensorReaderWin::SensorError() {
+void PlatformSensorReaderWin32::SensorError() {
   if (client_)
     client_->OnSensorError();
 }
 
-unsigned long PlatformSensorReaderWin::GetMinimalReportingIntervalMs() const {
-  return init_params_->min_reporting_interval_ms;
+base::TimeDelta PlatformSensorReaderWin32::GetMinimalReportingInterval() const {
+  return init_params_->min_reporting_interval;
 }
 
 }  // namespace device

services/device/generic_sensor/platform_sensor_reader_win.h

@@ -8,8 +8,13 @@
 #include <SensorsApi.h>
 #include <wrl/client.h>
 
+#include "services/device/generic_sensor/platform_sensor_reader_win_base.h"
 #include "services/device/public/mojom/sensor.mojom.h"
 
+namespace base {
+class TimeDelta;
+}
+
 namespace device {
 
 class PlatformSensorConfiguration;
@@ -19,44 +24,35 @@ union SensorReading;
 // Generic class that uses ISensor interface to fetch sensor data. Used
 // by PlatformSensorWin and delivers notifications via Client interface.
 // Instances of this class must be created and destructed on the same thread.
-class PlatformSensorReaderWin {
+class PlatformSensorReaderWin32 final : public PlatformSensorReaderWinBase {
  public:
-  // Client interface that can be used to receive notifications about sensor
-  // error or data change events.
-  class Client {
-   public:
-    virtual void OnReadingUpdated(const SensorReading& reading) = 0;
-    virtual void OnSensorError() = 0;
-
-   protected:
-    virtual ~Client() {}
-  };
-
-  static std::unique_ptr<PlatformSensorReaderWin> Create(
+  static std::unique_ptr<PlatformSensorReaderWinBase> Create(
       mojom::SensorType type,
       Microsoft::WRL::ComPtr<ISensorManager> sensor_manager);
 
   // Following methods are thread safe.
-  void SetClient(Client* client);
-  unsigned long GetMinimalReportingIntervalMs() const;
-  bool StartSensor(const PlatformSensorConfiguration& configuration);
-  void StopSensor();
+  void SetClient(Client* client) override;
+  base::TimeDelta GetMinimalReportingInterval() const override;
+  bool StartSensor(const PlatformSensorConfiguration& configuration) override
+      WARN_UNUSED_RESULT;
+  void StopSensor() override;
 
   // Must be destructed on the same thread that was used during construction.
-  ~PlatformSensorReaderWin();
+  ~PlatformSensorReaderWin32() override;
 
  private:
-  PlatformSensorReaderWin(Microsoft::WRL::ComPtr<ISensor> sensor,
-                          std::unique_ptr<ReaderInitParams> params);
+  PlatformSensorReaderWin32(Microsoft::WRL::ComPtr<ISensor> sensor,
+                            std::unique_ptr<ReaderInitParams> params);
 
   static Microsoft::WRL::ComPtr<ISensor> GetSensorForType(
       REFSENSOR_TYPE_ID sensor_type,
       Microsoft::WRL::ComPtr<ISensorManager> sensor_manager);
 
-  bool SetReportingInterval(const PlatformSensorConfiguration& configuration);
+  bool SetReportingInterval(const PlatformSensorConfiguration& configuration)
+      WARN_UNUSED_RESULT;
   void ListenSensorEvent();
   HRESULT SensorReadingChanged(ISensorDataReport* report,
-                               SensorReading* reading) const;
+                               SensorReading* reading) const WARN_UNUSED_RESULT;
   void SensorError();
 
  private:
@@ -72,9 +68,9 @@ class PlatformSensorReaderWin {
   Client* client_;
   Microsoft::WRL::ComPtr<ISensor> sensor_;
   scoped_refptr<EventListener> event_listener_;
-  base::WeakPtrFactory<PlatformSensorReaderWin> weak_factory_;
+  base::WeakPtrFactory<PlatformSensorReaderWin32> weak_factory_;
 
-  DISALLOW_COPY_AND_ASSIGN(PlatformSensorReaderWin);
+  DISALLOW_COPY_AND_ASSIGN(PlatformSensorReaderWin32);
 };
 
 }  // namespace device

services/device/generic_sensor/platform_sensor_reader_win_base.h

+// Copyright 2019 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.
+
+#ifndef SERVICES_DEVICE_GENERIC_SENSOR_PLATFORM_SENSOR_READER_WIN_BASE_H_
+#define SERVICES_DEVICE_GENERIC_SENSOR_PLATFORM_SENSOR_READER_WIN_BASE_H_
+
+namespace base {
+class TimeDelta;
+}
+
+namespace device {
+
+class PlatformSensorConfiguration;
+union SensorReading;
+
+class PlatformSensorReaderWinBase {
+ public:
+  // Client interface that can be used to receive notifications about sensor
+  // error or data change events.
+  class Client {
+   public:
+    virtual void OnReadingUpdated(const SensorReading& reading) = 0;
+    virtual void OnSensorError() = 0;
+
+   protected:
+    virtual ~Client() = default;
+  };
+
+  // Following methods must be thread safe.
+  // Sets the client PlatformSensorReaderWinBase will use to notify
+  // about errors or data change events. Only one client can be registered
+  // at a time (last client to register wins) and can be removed by
+  // setting the client to nullptr.
+  virtual void SetClient(Client* client) = 0;
+  virtual base::TimeDelta GetMinimalReportingInterval() const = 0;
+  virtual bool StartSensor(
+      const PlatformSensorConfiguration& configuration) = 0;
+  virtual void StopSensor() = 0;
+
+  virtual ~PlatformSensorReaderWinBase() = default;
+};
+
+}  // namespace device
+
+#endif  // SERVICES_DEVICE_GENERIC_SENSOR_PLATFORM_SENSOR_READER_WIN_BASE_H_
\ No newline at end of file

services/device/generic_sensor/platform_sensor_reader_winrt.cc

+// Copyright 2019 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 "services/device/generic_sensor/platform_sensor_reader_winrt.h"
+
+#include "services/device/public/mojom/sensor.mojom.h"
+
+namespace device {
+
+// static
+std::unique_ptr<PlatformSensorReaderWinBase>
+PlatformSensorReaderWinrtFactory::Create(mojom::SensorType) {
+  return nullptr;
+}
+
+}  // namespace device
\ No newline at end of file

services/device/generic_sensor/platform_sensor_reader_winrt.h

+// Copyright 2019 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.
+
+#ifndef SERVICES_DEVICE_GENERIC_SENSOR_PLATFORM_SENSOR_READER_WINRT_H_
+#define SERVICES_DEVICE_GENERIC_SENSOR_PLATFORM_SENSOR_READER_WINRT_H_
+
+#include <memory>
+
+#include "services/device/generic_sensor/platform_sensor_reader_win_base.h"
+
+namespace device {
+
+namespace mojom {
+enum class SensorType;
+}
+
+// Helper class used to create PlatformSensorReaderWinBasert instances
+class PlatformSensorReaderWinrtFactory {
+ public:
+  static std::unique_ptr<PlatformSensorReaderWinBase> Create(
+      mojom::SensorType type);
+};
+
+}  // namespace device
+
+#endif  // SERVICES_DEVICE_GENERIC_SENSOR_PLATFORM_SENSOR_READER_WINRT_H_
\ No newline at end of file

services/device/generic_sensor/platform_sensor_win.cc

@@ -18,7 +18,7 @@ PlatformSensorWin::PlatformSensorWin(
     SensorReadingSharedBuffer* reading_buffer,
     PlatformSensorProvider* provider,
     scoped_refptr<base::SingleThreadTaskRunner> sensor_thread_runner,
-    std::unique_ptr<PlatformSensorReaderWin> sensor_reader)
+    std::unique_ptr<PlatformSensorReaderWinBase> sensor_reader)
     : PlatformSensor(type, reading_buffer, provider),
       sensor_thread_runner_(sensor_thread_runner),
       sensor_reader_(sensor_reader.release()),
@@ -38,11 +38,11 @@ mojom::ReportingMode PlatformSensorWin::GetReportingMode() {
 }
 
 double PlatformSensorWin::GetMaximumSupportedFrequency() {
-  double minimal_reporting_interval_ms =
-      sensor_reader_->GetMinimalReportingIntervalMs();
-  if (!minimal_reporting_interval_ms)
+  base::TimeDelta minimal_reporting_interval_ms =
+      sensor_reader_->GetMinimalReportingInterval();
+  if (minimal_reporting_interval_ms.is_zero())
     return kDefaultSensorReportingFrequency;
-  return base::Time::kMillisecondsPerSecond / minimal_reporting_interval_ms;
+  return 1.0 / minimal_reporting_interval_ms.InSecondsF();
 }
 
 void PlatformSensorWin::OnReadingUpdated(const SensorReading& reading) {
@@ -69,12 +69,11 @@ void PlatformSensorWin::StopSensor() {
 bool PlatformSensorWin::CheckSensorConfiguration(
     const PlatformSensorConfiguration& configuration) {
   DCHECK(task_runner_->BelongsToCurrentThread());
-  double minimal_reporting_interval_ms =
-      sensor_reader_->GetMinimalReportingIntervalMs();
-  if (minimal_reporting_interval_ms == 0)
+  base::TimeDelta minimal_reporting_interval_ms =
+      sensor_reader_->GetMinimalReportingInterval();
+  if (minimal_reporting_interval_ms.is_zero())
     return true;
-  double max_frequency =
-      base::Time::kMillisecondsPerSecond / minimal_reporting_interval_ms;
+  double max_frequency = 1.0 / minimal_reporting_interval_ms.InSecondsF();
   return configuration.frequency() <= max_frequency;
 }
 

services/device/generic_sensor/platform_sensor_win.h

@@ -18,25 +18,25 @@ namespace device {
 // Implementation of PlatformSensor interface for Windows platform. Instance
 // of PlatformSensorWin is bound to IPC thread where PlatformSensorProvider is
 // running and communication with Windows platform sensor is done through
-// PlatformSensorReaderWin |sensor_reader_| interface which is bound to sensor
-// thread and communicates with PlatformSensorWin using
-// PlatformSensorReaderWin::Client interface. The error and data change events
-// are forwarded to IPC task runner.
+// PlatformSensorReaderWinBase |sensor_reader_| interface which is bound to
+// sensor thread and communicates with PlatformSensorWin using
+// PlatformSensorReaderWinBase::Client interface. The error and data change
+// events are forwarded to IPC task runner.
 class PlatformSensorWin final : public PlatformSensor,
-                                public PlatformSensorReaderWin::Client {
+                                public PlatformSensorReaderWinBase::Client {
  public:
   PlatformSensorWin(
       mojom::SensorType type,
       SensorReadingSharedBuffer* reading_buffer,
       PlatformSensorProvider* provider,
       scoped_refptr<base::SingleThreadTaskRunner> sensor_thread_runner,
-      std::unique_ptr<PlatformSensorReaderWin> sensor_reader);
+      std::unique_ptr<PlatformSensorReaderWinBase> sensor_reader);
 
   PlatformSensorConfiguration GetDefaultConfiguration() override;
   mojom::ReportingMode GetReportingMode() override;
   double GetMaximumSupportedFrequency() override;
 
-  // PlatformSensorReaderWin::Client interface implementation.
+  // PlatformSensorReaderWinBase::Client interface implementation.
   void OnReadingUpdated(const SensorReading& reading) override;
   void OnSensorError() override;
 
@@ -51,7 +51,7 @@ class PlatformSensorWin final : public PlatformSensor,
 
  private:
   scoped_refptr<base::SingleThreadTaskRunner> sensor_thread_runner_;
-  PlatformSensorReaderWin* const sensor_reader_;
+  PlatformSensorReaderWinBase* const sensor_reader_;
   base::WeakPtrFactory<PlatformSensorWin> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(PlatformSensorWin);

services/device/generic_sensor/windows/README.md

@@ -140,64 +140,7 @@ API will be reimplemented with these new classes:
   - Wrapper class around the actual Windows.Devices.Sensors APIs, functional
     equivalent of PlatformSensorReaderWin.
 
-#### 4.2.1 PlatformSensorProviderWinrt
-
-Similar to PlatformSensorProviderWin, but with the ISensorManager
-references removed. This is the header for the class:
-
-```cpp
-class PlatformSensorProviderWinrt final : public PlatformSensorProvider {
- public:
-  static PlatformSensorProviderWinrt* GetInstance();
-
-  void SetSensorReaderCreatorForTesting(
-      std::unique_ptr<SensorReaderCreator> sensor_reader_creator);
-
- protected:
-  ~PlatformSensorProviderWinrt() override;
-
-  // Base class calls this function to create sensor instances.
-  void CreateSensorInternal(mojom::SensorType type,
-                            SensorReadingSharedBuffer* reading_buffer,
-                            const CreateSensorCallback& callback) override;
-
- private:
-  PlatformSensorProviderWinrt();
-
-  std::unique_ptr<PlatformSensorReaderWin> CreateSensorReader(
-      mojom::SensorType type);
-  void SensorReaderCreated(
-      mojom::SensorType type,
-      SensorReadingSharedBuffer* reading_buffer,
-      const CreateSensorCallback& callback,
-      std::unique_ptr<PlatformSensorReaderWin> sensor_reader);
-
-  scoped_refptr<base::SingleThreadTaskRunner> com_sta_task_runner_;
-
-  std::unique_ptr<SensorReaderCreator> sensor_reader_creator_;
-
-  DISALLOW_COPY_AND_ASSIGN(PlatformSensorProviderWinrt);
-};
-```
-
-The `SensorReaderCreator` interface is simply a wrapper around the
-`PlatformSensorReaderWinrt::Create(type)` call and allows unit tests
-to mock it out.
-
-For brevity, the implementation has been deferred to section 8.1 in
-the appendix.
-
-Required mocks for unit testing:
-
-- Create a mock PlatformSensorReaderWinrt class.
-- Mock `PlatformSensorReaderWinrt::Create()` by injecting a
-  `MockSensorReaderCreator` so when PlatformSensorProviderWinrt
-  attempts to create a sensor reader, it returns a mocked instance
-  instead of one that calls into the Windows.Devices.Sensors APIs.
-
-Unit test cases for this class are detailed in section 7.
-
-#### 4.2.2 PlatformSensorReaderWinrt
+#### 4.2.1 PlatformSensorReaderWinrt
 
 The existing PlatformSensorReaderWin class will be pulled out into
 its own interface:
@@ -326,8 +269,8 @@ class PlatformSensorReaderWinrtLightSensor final
 ```
 
 The implementation for PlatformSensorReaderWinrtBase and
-PlatformSensorReaderWinrtLightSensor have been deferred to section 8.2
-and 8.3 of the appendix for brevity.
+PlatformSensorReaderWinrtLightSensor have been deferred to section 8.1
+and 8.2 of the appendix for brevity.
 
 Lastly, PlatformSensorReaderWinrt::Create will choose which derived
 PlatformSensorReaderWin class to instantiate based on the requested
@@ -413,24 +356,6 @@ thresholding.
 The modernization changes will be broken down into several incremental
 changes to keep change lists to a reviewable size:
 
-#### Change list 2: Implement PlatformSensorProviderWinrt
-
-- Feature Work:
-  - Fill in the implementation for PlatformSensorProviderWinrt as
-    defined in section 8.1.
-- Testing:
-  - Add unit tests for PlatformSensorProviderWinrt:
-    - Validate `CreateSensorCallback()` is given null if the sensor
-      type does not exist on system. This involves mocking
-      `PlatformSensorReaderWinrt::Create()` so it returns null.
-    - Validate `CreateSensorCallback()` is given non-null if the
-      sensor does exist on system. This involves mocking
-      `PlatformSensorReaderWinrt::Create()` so it returns a non-null
-      PlatformSensorReaderWinrt.
-    - Validate `CreateSensorCallback()` is given null if the sensor
-      type is not supported (e.g. SensorType::PRESSURE). This involves
-      mocking `PlatformSensorReaderWinrt::Create()` so it returns null.
-
 #### Change list 3: Define the interface for PlatformSensorReaderWinBase and implement PlatformSensorReaderWinrtBase
 
 - Feature Work:
@@ -500,101 +425,7 @@ section actually represents four separate CLs.
 
 ## 8. Appendix
 
-### 8.1 PlatformSensorProviderWinrt Implementation
-
-`platform_sensor_provider_winrt.h`
-
-Located in section 4.2.1 above.
-
-`platform_sensor_provider_winrt.cc`
-
-```cpp
-std::unique_ptr<PlatformSensorReaderWin>
-SensorReaderCreator::CreateSensorReader(mojom::SensorType type) {
-  return PlatformSensorReaderWinrt::Create(type);
-}
-
-PlatformSensorProviderWinrt::PlatformSensorProviderWinrt() = default;
-
-PlatformSensorProviderWinrt::~PlatformSensorProviderWinrt() = default;
-
-void PlatformSensorProviderWinrt::SetSensorReaderCreatorForTesting(
-    std::unique_ptr<SensorReaderCreator> sensor_reader_creator) {
-  sensor_reader_creator_ = std::move(sensor_reader_creator);
-}
-
-void PlatformSensorProviderWinrt::CreateSensorInternal(
-    mojom::SensorType type,
-    SensorReadingSharedBuffer* reading_buffer,
-    const CreateSensorCallback& callback) {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
-
-  switch (type) {
-    // Fusion sensor.
-    case mojom::SensorType::LINEAR_ACCELERATION: {
-      auto linear_acceleration_fusion_algorithm = std::make_unique<
-          LinearAccelerationFusionAlgorithmUsingAccelerometer>();
-      // If this PlatformSensorFusion object is successfully initialized,
-      // |callback| will be run with a reference to this object.
-      PlatformSensorFusion::Create(
-          reading_buffer, this, std::move(linear_acceleration_fusion_algorithm),
-          callback);
-      break;
-    }
-
-    // Try to create low-level sensors by default.
-    default: {
-      base::PostTaskAndReplyWithResult(
-          com_sta_task_runner_.get(), FROM_HERE,
-          base::Bind(&PlatformSensorProviderWinrt::CreateSensorReader,
-                     base::Unretained(this), type),
-          base::Bind(&PlatformSensorProviderWinrt::SensorReaderCreated,
-                     base::Unretained(this), type, reading_buffer, callback));
-      break;
-    }
-  }
-}
-
-void PlatformSensorProviderWinrt::SensorReaderCreated(
-    mojom::SensorType type,
-    SensorReadingSharedBuffer* reading_buffer,
-    const CreateSensorCallback& callback,
-    std::unique_ptr<PlatformSensorReaderWin> sensor_reader) {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
-  if (!sensor_reader) {
-    // Fallback options for sensors that can be implemented using sensor
-    // fusion. Note that it is important not to generate a cycle by adding a
-    // fallback here that depends on one of the other fallbacks provided.
-    switch (type) {
-      case mojom::SensorType::ABSOLUTE_ORIENTATION_EULER_ANGLES: {
-        auto algorithm = std::make_unique<
-            OrientationEulerAnglesFusionAlgorithmUsingQuaternion>(
-            true /* absolute */);
-        PlatformSensorFusion::Create(reading_buffer, this, std::move(algorithm),
-                                     std::move(callback));
-        return;
-      }
-      default:
-        callback.Run(nullptr);
-        return;
-    }
-  }
-
-  scoped_refptr<PlatformSensor> sensor =
-      new PlatformSensorWin(type, reading_buffer, this, com_sta_task_runner_,
-                            std::move(sensor_reader));
-  callback.Run(sensor);
-}
-
-std::unique_ptr<PlatformSensorReaderWin>
-PlatformSensorProviderWinrt::CreateSensorReader(mojom::SensorType type) {
-  DCHECK(com_sta_task_runner_->RunsTasksInCurrentSequence());
-
-  return sensor_reader_creator_->CreateSensorReader(type);
-}
-```
-
-### 8.2 PlatformSensorReaderWinrtBase Implementation
+### 8.1 PlatformSensorReaderWinrtBase Implementation
 
 `platform_sensor_reader_winrt.h`:
 
@@ -799,7 +630,7 @@ void PlatformSensorReaderWinrtBase<
 }
 ```
 
-### 8.3 PlatformSensorReaderWinrtLightSensor Implementation
+### 8.2 PlatformSensorReaderWinrtLightSensor Implementation
 
 `platform_sensor_reader_winrt.h`: