From 72b94a3b5d80a0f63f31f931d6e94d2a3f130700 Mon Sep 17 00:00:00 2001
From: Dan Murphy <dmurph@chromium.org>
Date: Fri, 18 Sep 2026 15:25:51 -0700
Subject: [PATCH] [generic-sensor] Serialize Win32 sensor calls on COM STA runner

ISensor and ISensorManager COM interfaces on Windows require access
within a Single-Threaded Apartment (STA).

Previously, PlatformSensorReaderWin32 performed COM calls such as
SetProperties and SetEventSink directly from StartSensor and StopSensor
on the calling thread rather than dispatching them to the COM STA task
runner. Additionally, EventListener callbacks were not synchronized with
reader destruction, and PlatformSensorProviderWin initialized and
released ISensorManager across different sequences.

This change:
1. Marshals StartSensor and StopSensor operations onto the COM STA task
   runner via StartSensorInternal and StopSensorInternal.
2. Guards COM state in PlatformSensorReaderWin32 with a SEQUENCE_CHECKER
   on the STA task runner, including the EventListener callbacks.
3. Adds EventListener::Detach() so that active COM event sinks safely
   ignore callbacks once the reader is destroyed.
4. Encapsulates ISensorManager within base::SequenceBound<ComStaHelper>
   in PlatformSensorProviderWin to guarantee lifetime management stays
   on the STA thread.
5. Introduces ScopedPlatformSensorReaderWinBase, a unique_ptr whose
   base::OnTaskRunnerDeleter is bound to the reader's own STA runner, so
   a reader is always destroyed on the sequence its posted tasks run on.
   This also fixes the same latent use-after-free in
   PlatformSensorProviderWinrt.
6. Suppresses start failures that a concurrent StopSensor() has already
   superseded, so stopping a sensor mid-start no longer raises a
   spurious error.
7. Adds a shared IsValidSensorFrequency() helper in sensor_traits, used
   by both PlatformSensorWin::CheckSensorConfiguration() and
   PlatformSensorReaderWin32::SetReportingInterval().
8. Uses base::test::TestFuture to deterministically coordinate
   asynchronous operations across sequences in unit tests without
   RunUntilIdle.

Two behavior changes are worth calling out.

The Win32 reader's StartSensor() now always returns true. The COM work
it used to perform inline happens on the STA runner, and producing a
meaningful return value would require blocking on that runner, which is
what this change sets out to avoid. Win32 COM setup failures are
therefore reported asynchronously through Client::OnSensorError() rather
than by failing AddConfiguration(). The web facing effect is that
Sensor.start() resolves and is then followed by an error event, instead
of the configuration being rejected outright. This matches how the
Android and Linux readers already behave. The WinRT reader is unaffected
and still fails synchronously.

Dropping the InitSensorManager hop also drops the ISensorManager
availability check that used to gate the LINEAR_ACCELERATION and GRAVITY
fusion sensors. Those types now go straight to
PlatformSensorFusion::Create, matching PlatformSensorProviderWinrt and
the Linux and Android providers. The observable result is unchanged,
since both fuse from ACCELEROMETER, whose creation still fails when
ISensorManager is unavailable.

TAG=agy
CONV=02db779e-9e3f-45ee-8717-780ab8af2587

Bug: b:513620385
Test: services_unittests --gtest_filter="PlatformSensor*"
Change-Id: I2da870379db95b67a139612fca4228328d17be2c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8412274
Reviewed-by: Matt Reynolds <mattreynolds@chromium.org>
Commit-Queue: Daniel Murphy <dmurph@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1701140}
---
diff --git a/services/device/generic_sensor/platform_sensor_provider_win.cc b/services/device/generic_sensor/platform_sensor_provider_win.cc
index 8247ac9..21b036d 100644
--- a/services/device/generic_sensor/platform_sensor_provider_win.cc
+++ b/services/device/generic_sensor/platform_sensor_provider_win.cc
@@ -25,9 +25,52 @@
 
 namespace device {
 
+// Owns the ISensorManager COM object and every operation that touches it.
+// Instances live on PlatformSensorProviderWin::com_sta_task_runner_ (enforced
+// by base::SequenceBound), which guarantees that the manager is created, used
+// and released inside the same COM STA.
+class PlatformSensorProviderWin::ComStaHelper {
+ public:
+  ComStaHelper() = default;
+  ComStaHelper(const ComStaHelper&) = delete;
+  ComStaHelper& operator=(const ComStaHelper&) = delete;
+  ~ComStaHelper() = default;
+
+  void SetSensorManagerForTesting(
+      Microsoft::WRL::ComPtr<ISensorManager> sensor_manager) {
+    sensor_manager_ = std::move(sensor_manager);
+  }
+
+  ScopedPlatformSensorReaderWinBase CreateSensorReader(mojom::SensorType type) {
+    if (!sensor_manager_) {
+      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;
+        }
+        return ScopedPlatformSensorReaderWinBase(
+            nullptr, base::OnTaskRunnerDeleter(
+                         base::SingleThreadTaskRunner::GetCurrentDefault()));
+      }
+    }
+    return PlatformSensorReaderWin32::Create(type, sensor_manager_);
+  }
+
+ private:
+  Microsoft::WRL::ComPtr<ISensorManager> sensor_manager_;
+};
+
 PlatformSensorProviderWin::PlatformSensorProviderWin()
     : com_sta_task_runner_(base::ThreadPool::CreateCOMSTATaskRunner(
-          {base::TaskPriority::USER_VISIBLE})) {}
+          {base::TaskPriority::USER_VISIBLE})),
+      com_sta_helper_(com_sta_task_runner_) {}
 
 PlatformSensorProviderWin::~PlatformSensorProviderWin() = default;
 
@@ -37,7 +80,8 @@
 
 void PlatformSensorProviderWin::SetSensorManagerForTesting(
     Microsoft::WRL::ComPtr<ISensorManager> sensor_manager) {
-  sensor_manager_ = sensor_manager;
+  com_sta_helper_.AsyncCall(&ComStaHelper::SetSensorManagerForTesting)
+      .WithArgs(std::move(sensor_manager));
 }
 
 scoped_refptr<base::SingleThreadTaskRunner>
@@ -49,45 +93,6 @@
     mojom::SensorType type,
     CreateSensorCallback callback) {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
-  if (sensor_manager_) {
-    OnInitSensorManager(type, std::move(callback));
-  } else {
-    com_sta_task_runner_->PostTaskAndReply(
-        FROM_HERE,
-        base::BindOnce(&PlatformSensorProviderWin::InitSensorManager,
-                       base::Unretained(this)),
-        base::BindOnce(&PlatformSensorProviderWin::OnInitSensorManager,
-                       base::Unretained(this), type, std::move(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,
-    CreateSensorCallback callback) {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
-
-  if (!sensor_manager_) {
-    std::move(callback).Run(nullptr);
-    return;
-  }
 
   switch (type) {
     // Fusion sensors.
@@ -114,12 +119,11 @@
 
     // Try to create low-level sensors by default.
     default: {
-      com_sta_task_runner_->PostTaskAndReplyWithResult(
-          FROM_HERE,
-          base::BindOnce(&PlatformSensorProviderWin::CreateSensorReader,
-                         base::Unretained(this), type),
-          base::BindOnce(&PlatformSensorProviderWin::SensorReaderCreated,
-                         base::Unretained(this), type, std::move(callback)));
+      com_sta_helper_.AsyncCall(&ComStaHelper::CreateSensorReader)
+          .WithArgs(type)
+          .Then(base::BindOnce(&PlatformSensorProviderWin::SensorReaderCreated,
+                               weak_factory_.GetWeakPtr(), type,
+                               std::move(callback)));
       break;
     }
   }
@@ -128,7 +132,7 @@
 void PlatformSensorProviderWin::SensorReaderCreated(
     mojom::SensorType type,
     CreateSensorCallback callback,
-    std::unique_ptr<PlatformSensorReaderWinBase> sensor_reader) {
+    ScopedPlatformSensorReaderWinBase sensor_reader) {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
 
   if (!sensor_reader) {
@@ -139,7 +143,7 @@
       case mojom::SensorType::ABSOLUTE_ORIENTATION_EULER_ANGLES: {
         auto algorithm = std::make_unique<
             OrientationEulerAnglesFusionAlgorithmUsingQuaternion>(
-            true /* absolute */);
+            /*absolute=*/true);
         PlatformSensorFusion::Create(AsWeakPtr(), std::move(algorithm),
                                      std::move(callback));
         return;
@@ -150,18 +154,11 @@
     }
   }
 
-  scoped_refptr<PlatformSensor> sensor = new PlatformSensorWin(
-      type, GetSensorReadingSharedBufferForType(type), AsWeakPtr(),
-      com_sta_task_runner_, std::move(sensor_reader));
-  std::move(callback).Run(sensor);
-}
-
-std::unique_ptr<PlatformSensorReaderWinBase>
-PlatformSensorProviderWin::CreateSensorReader(mojom::SensorType type) {
-  DCHECK(com_sta_task_runner_->RunsTasksInCurrentSequence());
-  if (!sensor_manager_)
-    return nullptr;
-  return PlatformSensorReaderWin32::Create(type, sensor_manager_);
+  scoped_refptr<PlatformSensor> sensor =
+      base::MakeRefCounted<PlatformSensorWin>(
+          type, GetSensorReadingSharedBufferForType(type), AsWeakPtr(),
+          std::move(sensor_reader));
+  std::move(callback).Run(std::move(sensor));
 }
 
 }  // namespace device
diff --git a/services/device/generic_sensor/platform_sensor_provider_win.h b/services/device/generic_sensor/platform_sensor_provider_win.h
index dcb001f..ef91535 100644
--- a/services/device/generic_sensor/platform_sensor_provider_win.h
+++ b/services/device/generic_sensor/platform_sensor_provider_win.h
@@ -10,16 +10,17 @@
 
 #include "base/memory/weak_ptr.h"
 #include "base/task/single_thread_task_runner.h"
+#include "base/threading/sequence_bound.h"
 #include "services/device/generic_sensor/platform_sensor_provider.h"
+#include "services/device/generic_sensor/platform_sensor_reader_win_base.h"
 
 namespace device {
 
-class PlatformSensorReaderWinBase;
-
 // Implementation of PlatformSensorProvider for Windows platform.
 // PlatformSensorProviderWin is responsible for following tasks:
-// - Starts sensor thread and stops it when there are no active sensors.
-// - Initialises ISensorManager and creates sensor reader on sensor thread.
+// - Owns the COM STA task runner that all ISensorManager and ISensor access
+//   happens on, and keeps ISensorManager confined to it via ComStaHelper.
+// - Creates sensor readers on that COM STA task runner.
 // - Constructs PlatformSensorWin on IPC thread and returns it to requester.
 class PlatformSensorProviderWin final : public PlatformSensorProvider {
  public:
@@ -34,7 +35,9 @@
   base::WeakPtr<PlatformSensorProvider> AsWeakPtr() override;
 
   // Overrides ISensorManager COM interface provided by the system, used
-  // only for testing purposes.
+  // only for testing purposes. The override is applied asynchronously on
+  // |com_sta_task_runner_|, but it is ordered ahead of any subsequent
+  // CreateSensorReader() call on that same sequence.
   void SetSensorManagerForTesting(
       Microsoft::WRL::ComPtr<ISensorManager> sensor_manager);
 
@@ -46,18 +49,14 @@
                             CreateSensorCallback callback) override;
 
  private:
-  void InitSensorManager();
-  void OnInitSensorManager(mojom::SensorType type,
-                           CreateSensorCallback callback);
-  std::unique_ptr<PlatformSensorReaderWinBase> CreateSensorReader(
-      mojom::SensorType type);
-  void SensorReaderCreated(
-      mojom::SensorType type,
-      CreateSensorCallback callback,
-      std::unique_ptr<PlatformSensorReaderWinBase> sensor_reader);
+  class ComStaHelper;
 
-  scoped_refptr<base::SingleThreadTaskRunner> com_sta_task_runner_;
-  Microsoft::WRL::ComPtr<ISensorManager> sensor_manager_;
+  void SensorReaderCreated(mojom::SensorType type,
+                           CreateSensorCallback callback,
+                           ScopedPlatformSensorReaderWinBase sensor_reader);
+
+  const scoped_refptr<base::SingleThreadTaskRunner> com_sta_task_runner_;
+  base::SequenceBound<ComStaHelper> com_sta_helper_;
   base::WeakPtrFactory<PlatformSensorProviderWin> weak_factory_{this};
 };
 
diff --git a/services/device/generic_sensor/platform_sensor_provider_winrt.cc b/services/device/generic_sensor/platform_sensor_provider_winrt.cc
index 26a06f5..8371ea1 100644
--- a/services/device/generic_sensor/platform_sensor_provider_winrt.cc
+++ b/services/device/generic_sensor/platform_sensor_provider_winrt.cc
@@ -82,7 +82,7 @@
 void PlatformSensorProviderWinrt::SensorReaderCreated(
     mojom::SensorType type,
     CreateSensorCallback callback,
-    std::unique_ptr<PlatformSensorReaderWinBase> sensor_reader) {
+    ScopedPlatformSensorReaderWinBase sensor_reader) {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
   if (!sensor_reader) {
     // Fallback options for sensors that can be implemented using sensor
@@ -105,14 +105,16 @@
   scoped_refptr<PlatformSensor> sensor =
       base::MakeRefCounted<PlatformSensorWin>(
           type, GetSensorReadingSharedBufferForType(type), AsWeakPtr(),
-          com_sta_task_runner_, std::move(sensor_reader));
+          std::move(sensor_reader));
   std::move(callback).Run(std::move(sensor));
 }
 
-std::unique_ptr<PlatformSensorReaderWinBase>
+ScopedPlatformSensorReaderWinBase
 PlatformSensorProviderWinrt::CreateSensorReader(mojom::SensorType type) {
   DCHECK(com_sta_task_runner_->RunsTasksInCurrentSequence());
-  return sensor_reader_factory_->CreateSensorReader(type);
+  return ScopedPlatformSensorReaderWinBase(
+      sensor_reader_factory_->CreateSensorReader(type).release(),
+      base::OnTaskRunnerDeleter(com_sta_task_runner_));
 }
 
 }  // namespace device
diff --git a/services/device/generic_sensor/platform_sensor_provider_winrt.h b/services/device/generic_sensor/platform_sensor_provider_winrt.h
index 153754c..b94bb99a 100644
--- a/services/device/generic_sensor/platform_sensor_provider_winrt.h
+++ b/services/device/generic_sensor/platform_sensor_provider_winrt.h
@@ -8,11 +8,10 @@
 #include "base/memory/weak_ptr.h"
 #include "base/task/single_thread_task_runner.h"
 #include "services/device/generic_sensor/platform_sensor_provider.h"
+#include "services/device/generic_sensor/platform_sensor_reader_win_base.h"
 
 namespace device {
 
-class PlatformSensorReaderWinBase;
-
 // Helper class used to instantiate new PlatformSensorReaderWinBase instances.
 class SensorReaderFactory {
  public:
@@ -43,13 +42,14 @@
                             CreateSensorCallback callback) override;
 
  private:
-  std::unique_ptr<PlatformSensorReaderWinBase> CreateSensorReader(
-      mojom::SensorType type);
+  // Wraps the factory-created reader in a pointer whose deleter posts back to
+  // |com_sta_task_runner_|, so the reader's COM objects are always released in
+  // the STA, including when the reply below never runs.
+  ScopedPlatformSensorReaderWinBase CreateSensorReader(mojom::SensorType type);
 
-  void SensorReaderCreated(
-      mojom::SensorType type,
-      CreateSensorCallback callback,
-      std::unique_ptr<PlatformSensorReaderWinBase> sensor_reader);
+  void SensorReaderCreated(mojom::SensorType type,
+                           CreateSensorCallback callback,
+                           ScopedPlatformSensorReaderWinBase sensor_reader);
 
   // The Windows.Devices.Sensors WinRT API supports both STA and MTA
   // threads. STA was chosen as PlatformSensorWin can only handle STA.
diff --git a/services/device/generic_sensor/platform_sensor_reader_win.cc b/services/device/generic_sensor/platform_sensor_reader_win.cc
index 5736e653..b4cb755 100644
--- a/services/device/generic_sensor/platform_sensor_reader_win.cc
+++ b/services/device/generic_sensor/platform_sensor_reader_win.cc
@@ -21,12 +21,14 @@
 #include "base/notimplemented.h"
 #include "base/numerics/angle_conversions.h"
 #include "base/numerics/math_constants.h"
+#include "base/numerics/safe_conversions.h"
 #include "base/task/single_thread_task_runner.h"
 #include "base/time/time.h"
 #include "base/win/scoped_propvariant.h"
 #include "services/device/generic_sensor/generic_sensor_consts.h"
 #include "services/device/public/cpp/generic_sensor/platform_sensor_configuration.h"
 #include "services/device/public/cpp/generic_sensor/sensor_reading.h"
+#include "services/device/public/cpp/generic_sensor/sensor_traits.h"
 
 namespace device {
 
@@ -256,7 +258,15 @@
... (file diff truncated)

diff --git a/services/device/generic_sensor/platform_sensor_and_provider_unittest_win.cc b/services/device/generic_sensor/platform_sensor_and_provider_unittest_win.cc
index 2bb63e2..e69d7369 100644
--- a/services/device/generic_sensor/platform_sensor_and_provider_unittest_win.cc
+++ b/services/device/generic_sensor/platform_sensor_and_provider_unittest_win.cc
@@ -6,14 +6,23 @@
 #include <sensors.h>
 #include <wrl/implements.h>
 
+#include <limits>
+#include <vector>
+
 #include "base/compiler_specific.h"
 #include "base/functional/bind.h"
+#include "base/functional/callback_helpers.h"
 #include "base/memory/read_only_shared_memory_region.h"
 #include "base/memory/shared_memory_mapping.h"
 #include "base/numerics/angle_conversions.h"
 #include "base/numerics/math_constants.h"
 #include "base/run_loop.h"
+#include "base/synchronization/waitable_event.h"
+#include "base/task/single_thread_task_runner.h"
+#include "base/test/bind.h"
 #include "base/test/task_environment.h"
+#include "base/test/test_future.h"
+#include "base/threading/thread_restrictions.h"
 #include "base/win/propvarutil.h"
 #include "base/win/scoped_com_initializer.h"
 #include "base/win/scoped_propvariant.h"
@@ -199,9 +208,21 @@
 
     // Overrides default ISensorManager with mocked interface.
     provider_ = std::make_unique<PlatformSensorProviderWin>();
+    com_sta_task_runner_ = provider_->GetComStaTaskRunnerForTesting();
     provider_->SetSensorManagerForTesting(std::move(manager));
   }
 
+  void TearDown() override {
+    provider_.reset();
+    // Follow the teardown chain across sequences: finish any in-flight reader
+    // creation on the COM STA, run the replies that drop those readers, then
+    // let the readers' deleters run on the COM STA. Only then are the mock COM
+    // objects they hold references to safe to verify.
+    FlushComStaTaskRunner();
+    FlushMainSequence();
+    FlushComStaTaskRunner();
+  }
+
  protected:
   void SensorCreated(scoped_refptr<PlatformSensor> sensor) {
     platform_sensor_ = sensor;
@@ -229,14 +250,38 @@
                       const PlatformSensorConfiguration& config) {
     run_loop_ = std::make_unique<base::RunLoop>();
     bool ret = sensor->StartListening(client, config);
-    if (ret)
+    if (ret) {
       run_loop_->Run();
+    }
     run_loop_ = nullptr;
     return ret;
   }
 
   void QuitInnerLoop() { run_loop_->Quit(); }
 
+  // Runs |task| on the COM STA task runner and waits for it to complete.
+  void RunOnComStaTaskRunner(base::OnceClosure task) {
+    base::test::TestFuture<void> future;
+    com_sta_task_runner_->PostTaskAndReply(FROM_HERE, std::move(task),
+                                           future.GetCallback());
+    EXPECT_TRUE(future.Wait());
+  }
+
+  // Waits until everything already queued on the COM STA task runner has run.
+  void FlushComStaTaskRunner() { RunOnComStaTaskRunner(base::DoNothing()); }
+
+  // Waits until everything already queued on the main sequence has run. The
+  // quit closure is posted behind the pending tasks, so FIFO ordering makes
+  // this deterministic without resorting to RunUntilIdle(). Tasks that those
+  // tasks post in turn are not waited for; the callers below pair this with
+  // FlushComStaTaskRunner() to follow the work across sequences explicitly.
+  void FlushMainSequence() {
+    base::RunLoop run_loop;
+    base::SingleThreadTaskRunner::GetCurrentDefault()->PostTask(
+        FROM_HERE, run_loop.QuitClosure());
+    run_loop.Run();
+  }
+
   void SetUnsupportedSensor(REFSENSOR_TYPE_ID sensor) {
     EXPECT_CALL(*(sensor_manager_.Get()), GetSensorsByType(sensor, _))
         .WillRepeatedly(
@@ -280,6 +325,7 @@
     // data change events.
     ON_CALL(*(sensor_.Get()), SetEventSink(NotNull()))
         .WillByDefault([this](ISensorEvents* events) {
+          EXPECT_TRUE(com_sta_task_runner_->RunsTasksInCurrentSequence());
           events->AddRef();
           sensor_events_.Attach(events);
           if (this->run_loop_) {
@@ -295,6 +341,7 @@
     // longer interested in sensor events and ISensorEvents can be released.
     ON_CALL(*(sensor_.Get()), SetEventSink(IsNull()))
         .WillByDefault([this](ISensorEvents* events) {
+          EXPECT_TRUE(com_sta_task_runner_->RunsTasksInCurrentSequence());
           sensor_events_.Reset();
           if (this->run_loop_) {
             task_environment_.GetMainThreadTaskRunner()->PostTask(
@@ -321,18 +368,24 @@
         });
   }
 
+  // The event generators below run on the COM STA task runner because that is
+  // where COM delivers ISensorEvents callbacks for an apartment-threaded
+  // ISensor: the apartment that called SetEventSink() receives them.
+
   // Generates OnLeave event, e.g. when sensor is disconnected.
   void GenerateLeaveEvent() {
     if (!sensor_events_)
       return;
-    sensor_events_->OnLeave(SENSOR_ID());
+    RunOnComStaTaskRunner(base::BindLambdaForTesting(
+        [&]() { sensor_events_->OnLeave(SENSOR_ID()); }));
   }
 
   // Generates OnStateChangedLeave event.
   void GenerateStateChangeEvent(SensorState state) {
     if (!sensor_events_)
       return;
-    sensor_events_->OnStateChanged(sensor_.Get(), state);
+    RunOnComStaTaskRunner(base::BindLambdaForTesting(
+        [&]() { sensor_events_->OnStateChanged(sensor_.Get(), state); }));
   }
 
   struct PropertyKeyCompare {
@@ -373,11 +426,14 @@
               return S_OK;
             }));
 
-    sensor_events_->OnDataUpdated(sensor_.Get(), data_report.Get());
+    RunOnComStaTaskRunner(base::BindLambdaForTesting([&]() {
+      sensor_events_->OnDataUpdated(sensor_.Get(), data_report.Get());
+    }));
   }
 
   base::win::ScopedCOMInitializer com_initializer_;
   base::test::TaskEnvironment task_environment_;
+  scoped_refptr<base::SingleThreadTaskRunner> com_sta_task_runner_;
   Microsoft::WRL::ComPtr<MockISensorManager> sensor_manager_;
   Microsoft::WRL::ComPtr<MockISensorCollection> sensor_collection_;
   Microsoft::WRL::ComPtr<MockISensor> sensor_;
@@ -449,7 +505,8 @@
   EXPECT_CALL(*(sensor_.Get()), SetEventSink(IsNull())).Times(1);
   EXPECT_CALL(*(sensor_.Get()), SetProperties(NotNull(), _))
       .WillRepeatedly(
-          [](IPortableDeviceValues* props, IPortableDeviceValues** result) {
+          [this](IPortableDeviceValues* props, IPortableDeviceValues** result) {
+            EXPECT_TRUE(com_sta_task_runner_->RunsTasksInCurrentSequence());
             ULONG value = 0;
             HRESULT hr = props->GetUnsignedIntegerValue(
                 SENSOR_PROPERTY_CURRENT_REPORT_INTERVAL, &value);
@@ -789,4 +846,264 @@
   EXPECT_FALSE(quaternion_sensor);
 }
 
+// Tests that rapid start and stop operations are serialized on the COM STA
+// task runner, i.e. that the event sink is alternately set and cleared.
+TEST_F(PlatformSensorAndProviderTestWin, RapidStartAndStop) {
+  constexpr int kIterations = 50;
+
+  SetSupportedReportingFrequency(10);
+  SetSupportedSensor(SENSOR_TYPE_AMBIENT_LIGHT);
+
+  // Written on the COM STA sequence; read after flushing it below. true means
+  // the sink was set, false that it was cleared.
+  std::vector<bool> sink_transitions;
+  EXPECT_CALL(*(sensor_.Get()), SetEventSink(NotNull()))
+      .Times(kIterations)
+      .WillRepeatedly([&](ISensorEvents* events) {
+        EXPECT_TRUE(com_sta_task_runner_->RunsTasksInCurrentSequence());
+        sink_transitions.push_back(true);
+        events->AddRef();
+        sensor_events_.Attach(events);
+        return S_OK;
+      });
+  EXPECT_CALL(*(sensor_.Get()), SetEventSink(IsNull()))
+      .Times(kIterations)
+      .WillRepeatedly([&](ISensorEvents*) {
+        EXPECT_TRUE(com_sta_task_runner_->RunsTasksInCurrentSequence());
+        sink_transitions.push_back(false);
+        sensor_events_.Reset();
+        return S_OK;
+      });
+
+  auto sensor = CreateSensor(SensorType::AMBIENT_LIGHT);
+  ASSERT_TRUE(sensor);
+
+  auto client = std::make_unique<NiceMock<MockPlatformSensorClient>>(sensor);
+  PlatformSensorConfiguration configuration(10);
+
+  for (int i = 0; i < kIterations; ++i) {
+    EXPECT_TRUE(sensor->StartListening(client.get(), configuration));
+    EXPECT_TRUE(sensor->StopListening(client.get(), configuration));
+  }
+  FlushComStaTaskRunner();
+
+  ASSERT_EQ(sink_transitions.size(), size_t{2 * kIterations});
+  for (size_t i = 0; i < sink_transitions.size(); ++i) {
+    EXPECT_EQ(sink_transitions[i], i % 2 == 0) << "transition " << i;
+  }
+}
+
+// Tests that configurations with invalid frequencies (non-finite, <= 0, or
+// > max) are rejected.
+TEST_F(PlatformSensorAndProviderTestWin, StartWithInvalidFrequency) {
+  SetSupportedReportingFrequency(10);
+  SetSupportedSensor(SENSOR_TYPE_AMBIENT_LIGHT);
+
+  auto sensor = CreateSensor(SensorType::AMBIENT_LIGHT);
+  ASSERT_TRUE(sensor);
+
+  auto client = std::make_unique<NiceMock<MockPlatformSensorClient>>(sensor);
+
+  for (double frequency : {0.0, -1.0, std::numeric_limits<double>::quiet_NaN(),
+                           std::numeric_limits<double>::infinity()}) {
+    SCOPED_TRACE(frequency);
+    // Note: an invalid frequency can only be constructed through the setter,
+    // and only because PlatformSensorConfiguration::set_frequency() DCHECKs the
+    // previous value rather than its argument. If that DCHECK is ever fixed,
+    // this loop needs another way to build an invalid configuration.
+    PlatformSensorConfiguration invalid_frequency;
+    invalid_frequency.set_frequency(frequency);
+    EXPECT_FALSE(sensor->StartListening(client.get(), invalid_frequency));
+  }
+
+  PlatformSensorConfiguration excessive_frequency(20);
+  EXPECT_FALSE(sensor->StartListening(client.get(), excessive_frequency));
+}
+
+// Tests that when setting reporting properties fails on the COM thread,
+// the sensor reports an error gracefully to the client.
+TEST_F(PlatformSensorAndProviderTestWin, SensorStartFailsOnComError) {
+  SetSupportedReportingFrequency(10);
+  SetSupportedSensor(SENSOR_TYPE_AMBIENT_LIGHT);
+
+  EXPECT_CALL(*(sensor_.Get()), SetProperties(NotNull(), _))
+      .WillOnce([this](IPortableDeviceValues*, IPortableDeviceValues**) {
+        EXPECT_TRUE(com_sta_task_runner_->RunsTasksInCurrentSequence());
+        return E_FAIL;
+      });
+
+  auto sensor = CreateSensor(SensorType::AMBIENT_LIGHT);
+  ASSERT_TRUE(sensor);
+
+  auto client = std::make_unique<NiceMock<MockPlatformSensorClient>>(sensor);
+  PlatformSensorConfiguration configuration(10);
+
+  base::test::TestFuture<void> error_future;
+  EXPECT_CALL(*client, OnSensorError())
+      .WillOnce(base::test::InvokeFuture(error_future));
+
+  EXPECT_TRUE(sensor->StartListening(client.get(), configuration));
+  EXPECT_TRUE(error_future.Wait());
+}
+
+// Tests that when setting the event sink fails on the COM thread,
+// the sensor reports an error gracefully to the client.
+TEST_F(PlatformSensorAndProviderTestWin, SensorStartFailsOnSetEventSinkError) {
+  SetSupportedReportingFrequency(10);
+  SetSupportedSensor(SENSOR_TYPE_AMBIENT_LIGHT);
+
+  EXPECT_CALL(*(sensor_.Get()), SetProperties(NotNull(), _))
+      .WillOnce([this](IPortableDeviceValues*, IPortableDeviceValues**) {
+        EXPECT_TRUE(com_sta_task_runner_->RunsTasksInCurrentSequence());
+        return S_OK;
+      });
+  EXPECT_CALL(*(sensor_.Get()), SetEventSink(NotNull()))
+      .WillOnce([this](ISensorEvents*) {
+        EXPECT_TRUE(com_sta_task_runner_->RunsTasksInCurrentSequence());
+        return E_FAIL;
+      });
+
+  auto sensor = CreateSensor(SensorType::AMBIENT_LIGHT);
+  ASSERT_TRUE(sensor);
+
+  auto client = std::make_unique<NiceMock<MockPlatformSensorClient>>(sensor);
+  PlatformSensorConfiguration configuration(10);
+
+  base::test::TestFuture<void> error_future;
+  EXPECT_CALL(*client, OnSensorError())
+      .WillOnce(base::test::InvokeFuture(error_future));
+
+  EXPECT_TRUE(sensor->StartListening(client.get(), configuration));
+  EXPECT_TRUE(error_future.Wait());
+}
+
+// Tests that an asynchronous start failure is dropped when the client has
+// already stopped listening. PlatformSensor::NotifySensorError() disables the
+// sensor for every client, so a superseded start must not report one.
+TEST_F(PlatformSensorAndProviderTestWin, SupersededStartFailureIsNotReported) {
+  SetSupportedReportingFrequency(10);
+  SetSupportedSensor(SENSOR_TYPE_AMBIENT_LIGHT);
... (truncated)