Sensor API: Remove SensorThread from platform_sensor_provider_win.cc

This CL removes SensorThread from platform_sensor_provider_win.cc and
uses the //base/task/post_task.h APIs instead.

Bug: 883877
Change-Id: Icd3632f33e58b2577c2f2d6925d9ce5c3469e4d0
Reviewed-on: https://chromium-review.googlesource.com/c/1246063
Commit-Queue: Jun Cai <juncai@chromium.org>
Reviewed-by: Robert Liao <robliao@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595980}

services/device/generic_sensor/platform_sensor_and_provider_unittest_win.cc

@@ -9,6 +9,7 @@
 #include "base/bind.h"
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
+#include "base/test/scoped_task_environment.h"
 #include "base/win/iunknown_impl.h"
 #include "base/win/propvarutil.h"
 #include "base/win/scoped_propvariant.h"
@@ -189,7 +190,12 @@ class MockISensorDataReport : public MockCOMInterface<ISensorDataReport> {
 //                         data in OnDataUpdated event.
 class PlatformSensorAndProviderTestWin : public ::testing::Test {
  public:
+  PlatformSensorAndProviderTestWin()
+      : scoped_task_environment_(
+            base::test::ScopedTaskEnvironment::MainThreadType::IO) {}
+
   void SetUp() override {
+    EXPECT_EQ(S_OK, CoInitialize(nullptr));
     sensor_ = new NiceMock<MockISensor>();
     sensor_collection_ = new NiceMock<MockISensorCollection>();
     sensor_manager_ = new NiceMock<MockISensorManager>();
@@ -244,7 +250,7 @@ class PlatformSensorAndProviderTestWin : public ::testing::Test {
 
   void SetUnsupportedSensor(REFSENSOR_TYPE_ID sensor) {
     EXPECT_CALL(*sensor_manager_, GetSensorsByType(sensor, _))
-        .WillOnce(Invoke(
+        .WillRepeatedly(Invoke(
             [this](REFSENSOR_TYPE_ID type, ISensorCollection** collection) {
               return HRESULT_FROM_WIN32(ERROR_NOT_FOUND);
             }));
@@ -288,7 +294,7 @@ class PlatformSensorAndProviderTestWin : public ::testing::Test {
           events->AddRef();
           sensor_events_.Attach(events);
           if (this->run_loop_) {
-            message_loop_.task_runner()->PostTask(
+            scoped_task_environment_.GetMainThreadTaskRunner()->PostTask(
                 FROM_HERE,
                 base::Bind(&PlatformSensorAndProviderTestWin::QuitInnerLoop,
                            base::Unretained(this)));
@@ -302,7 +308,7 @@ class PlatformSensorAndProviderTestWin : public ::testing::Test {
         .WillByDefault(Invoke([this](ISensorEvents* events) {
           sensor_events_.Reset();
           if (this->run_loop_) {
-            message_loop_.task_runner()->PostTask(
+            scoped_task_environment_.GetMainThreadTaskRunner()->PostTask(
                 FROM_HERE,
                 base::Bind(&PlatformSensorAndProviderTestWin::QuitInnerLoop,
                            base::Unretained(this)));
@@ -382,11 +388,11 @@ class PlatformSensorAndProviderTestWin : public ::testing::Test {
     sensor_events_->OnDataUpdated(sensor_.get(), data_report.Get());
   }
 
+  base::test::ScopedTaskEnvironment scoped_task_environment_;
   scoped_refptr<MockISensorManager> sensor_manager_;
   scoped_refptr<MockISensorCollection> sensor_collection_;
   scoped_refptr<MockISensor> sensor_;
   Microsoft::WRL::ComPtr<ISensorEvents> sensor_events_;
-  base::MessageLoop message_loop_;
   scoped_refptr<PlatformSensor> platform_sensor_;
   // Inner run loop used to wait for async sensor creation callback.
   std::unique_ptr<base::RunLoop> run_loop_;

services/device/generic_sensor/platform_sensor_provider_win.cc

@@ -10,6 +10,8 @@
 #include <iomanip>
 
 #include "base/memory/singleton.h"
+#include "base/task/post_task.h"
+#include "base/task/task_traits.h"
 #include "base/task_runner_util.h"
 #include "base/threading/thread.h"
 #include "services/device/generic_sensor/linear_acceleration_fusion_algorithm_using_accelerometer.h"
@@ -19,44 +21,6 @@
 
 namespace device {
 
-class PlatformSensorProviderWin::SensorThread final : public base::Thread {
- public:
-  SensorThread() : base::Thread("Sensor thread") { init_com_with_mta(true); }
-
-  void SetSensorManagerForTesting(
-      Microsoft::WRL::ComPtr<ISensorManager> sensor_manager) {
-    sensor_manager_ = sensor_manager;
-  }
-
-  const Microsoft::WRL::ComPtr<ISensorManager>& sensor_manager() const {
-    return sensor_manager_;
-  }
-
- protected:
-  void Init() override {
-    if (sensor_manager_)
-      return;
-    HRESULT hr = ::CoCreateInstance(CLSID_SensorManager, nullptr, CLSCTX_ALL,
-                                    IID_PPV_ARGS(&sensor_manager_));
-    if (FAILED(hr)) {
-      // Only log this error the first time.
-      static bool logged_failure = false;
-      if (!logged_failure) {
-        LOG(ERROR) << "Unable to create instance of SensorManager: "
-                   << _com_error(hr).ErrorMessage() << " (0x" << std::hex
-                   << std::uppercase << std::setfill('0') << std::setw(8) << hr
-                   << ")";
-        logged_failure = true;
-      }
-    }
-  }
-
-  void CleanUp() override { sensor_manager_.Reset(); }
-
- private:
-  Microsoft::WRL::ComPtr<ISensorManager> sensor_manager_;
-};
-
 // static
 PlatformSensorProviderWin* PlatformSensorProviderWin::GetInstance() {
   return base::Singleton<
@@ -66,11 +30,13 @@ PlatformSensorProviderWin* PlatformSensorProviderWin::GetInstance() {
 
 void PlatformSensorProviderWin::SetSensorManagerForTesting(
     Microsoft::WRL::ComPtr<ISensorManager> sensor_manager) {
-  CreateSensorThread();
-  sensor_thread_->SetSensorManagerForTesting(sensor_manager);
+  sensor_manager_ = sensor_manager;
 }
 
-PlatformSensorProviderWin::PlatformSensorProviderWin() = default;
+PlatformSensorProviderWin::PlatformSensorProviderWin()
+    : com_sta_task_runner_(base::CreateCOMSTATaskRunnerWithTraits(
+          base::TaskPriority::USER_VISIBLE)) {}
+
 PlatformSensorProviderWin::~PlatformSensorProviderWin() = default;
 
 void PlatformSensorProviderWin::CreateSensorInternal(
@@ -78,7 +44,43 @@ void PlatformSensorProviderWin::CreateSensorInternal(
     SensorReadingSharedBuffer* reading_buffer,
     const CreateSensorCallback& callback) {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
-  if (!StartSensorThread()) {
+  if (sensor_manager_) {
+    OnInitSensorManager(type, reading_buffer, callback);
+  } else {
+    com_sta_task_runner_->PostTaskAndReply(
+        FROM_HERE,
+        base::Bind(&PlatformSensorProviderWin::InitSensorManager,
+                   base::Unretained(this)),
+        base::Bind(&PlatformSensorProviderWin::OnInitSensorManager,
+                   base::Unretained(this), type, reading_buffer, callback));
+  }
+}
+
+void PlatformSensorProviderWin::InitSensorManager() {
+  DCHECK(com_sta_task_runner_->RunsTasksInCurrentSequence());
+
+  HRESULT hr = ::CoCreateInstance(CLSID_SensorManager, nullptr, CLSCTX_ALL,
+                                  IID_PPV_ARGS(&sensor_manager_));
+  if (FAILED(hr)) {
+    // Only log this error the first time.
+    static bool logged_failure = false;
+    if (!logged_failure) {
+      LOG(ERROR) << "Unable to create instance of SensorManager: "
+                 << _com_error(hr).ErrorMessage() << " (0x" << std::hex
+                 << std::uppercase << std::setfill('0') << std::setw(8) << hr
+                 << ")";
+      logged_failure = true;
+    }
+  }
+}
+
+void PlatformSensorProviderWin::OnInitSensorManager(
+    mojom::SensorType type,
+    SensorReadingSharedBuffer* reading_buffer,
+    const CreateSensorCallback& callback) {
+  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+
+  if (!sensor_manager_) {
     callback.Run(nullptr);
     return;
   }
@@ -99,7 +101,7 @@ void PlatformSensorProviderWin::CreateSensorInternal(
     // Try to create low-level sensors by default.
     default: {
       base::PostTaskAndReplyWithResult(
-          sensor_thread_->task_runner().get(), FROM_HERE,
+          com_sta_task_runner_.get(), FROM_HERE,
           base::Bind(&PlatformSensorProviderWin::CreateSensorReader,
                      base::Unretained(this), type),
           base::Bind(&PlatformSensorProviderWin::SensorReaderCreated,
@@ -109,27 +111,6 @@ void PlatformSensorProviderWin::CreateSensorInternal(
   }
 }
 
-void PlatformSensorProviderWin::FreeResources() {
-  StopSensorThread();
-}
-
-void PlatformSensorProviderWin::CreateSensorThread() {
-  if (!sensor_thread_)
-    sensor_thread_ = std::make_unique<SensorThread>();
-}
-
-bool PlatformSensorProviderWin::StartSensorThread() {
-  CreateSensorThread();
-  if (!sensor_thread_->IsRunning())
-    return sensor_thread_->Start();
-  return true;
-}
-
-void PlatformSensorProviderWin::StopSensorThread() {
-  if (sensor_thread_ && sensor_thread_->IsRunning())
-    sensor_thread_->Stop();
-}
-
 void PlatformSensorProviderWin::SensorReaderCreated(
     mojom::SensorType type,
     SensorReadingSharedBuffer* reading_buffer,
@@ -155,19 +136,18 @@ void PlatformSensorProviderWin::SensorReaderCreated(
     }
   }
 
-  scoped_refptr<PlatformSensor> sensor = new PlatformSensorWin(
-      type, reading_buffer, this, sensor_thread_->task_runner(),
-      std::move(sensor_reader));
+  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>
 PlatformSensorProviderWin::CreateSensorReader(mojom::SensorType type) {
-  DCHECK(sensor_thread_->task_runner()->BelongsToCurrentThread());
-  if (!sensor_thread_->sensor_manager())
+  DCHECK(com_sta_task_runner_->RunsTasksInCurrentSequence());
+  if (!sensor_manager_)
     return nullptr;
-  return PlatformSensorReaderWin::Create(type,
-                                         sensor_thread_->sensor_manager());
+  return PlatformSensorReaderWin::Create(type, sensor_manager_);
 }
 
 }  // namespace device

services/device/generic_sensor/platform_sensor_provider_win.h

@@ -37,7 +37,6 @@ class PlatformSensorProviderWin final : public PlatformSensorProvider {
   ~PlatformSensorProviderWin() override;
 
   // PlatformSensorProvider interface implementation.
-  void FreeResources() override;
   void CreateSensorInternal(mojom::SensorType type,
                             SensorReadingSharedBuffer* reading_buffer,
                             const CreateSensorCallback& callback) override;
@@ -45,13 +44,12 @@ class PlatformSensorProviderWin final : public PlatformSensorProvider {
  private:
   friend struct base::DefaultSingletonTraits<PlatformSensorProviderWin>;
 
-  class SensorThread;
-
   PlatformSensorProviderWin();
 
-  void CreateSensorThread();
-  bool StartSensorThread();
-  void StopSensorThread();
+  void InitSensorManager();
+  void OnInitSensorManager(mojom::SensorType type,
+                           SensorReadingSharedBuffer* reading_buffer,
+                           const CreateSensorCallback& callback);
   std::unique_ptr<PlatformSensorReaderWin> CreateSensorReader(
       mojom::SensorType type);
   void SensorReaderCreated(
@@ -60,7 +58,8 @@ class PlatformSensorProviderWin final : public PlatformSensorProvider {
       const CreateSensorCallback& callback,
       std::unique_ptr<PlatformSensorReaderWin> sensor_reader);
 
-  std::unique_ptr<SensorThread> sensor_thread_;
+  scoped_refptr<base::SingleThreadTaskRunner> com_sta_task_runner_;
+  Microsoft::WRL::ComPtr<ISensorManager> sensor_manager_;
 
   DISALLOW_COPY_AND_ASSIGN(PlatformSensorProviderWin);
 };