[sensors] Own PlatformSensorProvider in DeviceService

This patch changes the ownership model of PlatformSensorProvider from a
leaky singleton to something owned by the DeviceService instance. The
motivating reason is that it means that we can create a new instance of
PlatformSensorProvider for each test rather than having them be reused.
This can cause various flaky test failures as demonstrated by the
associated issue.

To simplify this new ownership the PlatformSensorProvider is now used
on the UI thread rather than the IO thread. As blocking calls are
already posted to background task runners this change should have no
practical effect.

To prevent SensorImpl instances from leaking after the test exits (but
before the Mojo connection errors are handled) they are now explicitly
owned by the SensorProviderImpl that created them.

This patch also fixes PlatformSensorAndProviderTestWin to use a
base::ScopedCOMInitializer rather than calling CoInitialize manually.
This removes another place where state leaked between tests.

Bug: 939808
Change-Id: Id18926c4254c1a82310693750d84abe931e0f385
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1693763
Commit-Queue: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Ovidio de Jesús Ruiz-Henríquez <odejesush@chromium.org>
Cr-Commit-Position: refs/heads/master@{#676110}

services/device/BUILD.gn

@@ -15,6 +15,7 @@ source_set("lib") {
   # dependence should only be for the purpose of embedding the Device Service.
   visibility = [
     ":test_support",
+    ":tests",
     "//content/browser",
   ]
   sources = [
@@ -124,6 +125,7 @@ source_set("tests") {
   ]
 
   deps = [
+    ":lib",
     ":test_support",
     "//base",
     "//base/test:test_support",

services/device/device_service.cc

@@ -15,6 +15,7 @@
 #include "mojo/public/cpp/system/message_pipe.h"
 #include "services/device/bluetooth/bluetooth_system_factory.h"
 #include "services/device/fingerprint/fingerprint.h"
+#include "services/device/generic_sensor/platform_sensor_provider.h"
 #include "services/device/generic_sensor/sensor_provider_impl.h"
 #include "services/device/geolocation/geolocation_config.h"
 #include "services/device/geolocation/geolocation_context.h"
@@ -134,6 +135,12 @@ DeviceService::~DeviceService() {
 #endif
 }
 
+void DeviceService::SetPlatformSensorProviderForTesting(
+    std::unique_ptr<PlatformSensorProvider> provider) {
+  DCHECK(!sensor_provider_);
+  sensor_provider_ = std::make_unique<SensorProviderImpl>(std::move(provider));
+}
+
 void DeviceService::OnStart() {
   registry_.AddInterface<mojom::Fingerprint>(base::Bind(
       &DeviceService::BindFingerprintRequest, base::Unretained(this)));
@@ -314,11 +321,14 @@ void DeviceService::BindScreenOrientationListenerRequest(
 
 void DeviceService::BindSensorProviderRequest(
     mojom::SensorProviderRequest request) {
-  if (io_task_runner_) {
-    io_task_runner_->PostTask(
-        FROM_HERE, base::BindOnce(&device::SensorProviderImpl::Create,
-                                  std::move(request)));
+  if (!sensor_provider_) {
+    auto platform_provider = PlatformSensorProvider::Create();
+    if (!platform_provider)
+      return;
+    sensor_provider_ =
+        std::make_unique<SensorProviderImpl>(std::move(platform_provider));
   }
+  sensor_provider_->Bind(std::move(request));
 }
 
 void DeviceService::BindTimeZoneMonitorRequest(

services/device/device_service.h

@@ -72,8 +72,10 @@ class SerialPortManagerImpl;
 #endif
 
 class DeviceService;
+class PlatformSensorProvider;
 class PowerMonitorMessageBroadcaster;
 class PublicIpAddressLocationNotifier;
+class SensorProviderImpl;
 class TimeZoneMonitor;
 
 #if defined(OS_ANDROID)
@@ -125,6 +127,9 @@ class DeviceService : public service_manager::Service {
 #endif
   ~DeviceService() override;
 
+  void SetPlatformSensorProviderForTesting(
+      std::unique_ptr<PlatformSensorProvider> provider);
+
  private:
   // service_manager::Service:
   void OnStart() override;
@@ -178,6 +183,7 @@ class DeviceService : public service_manager::Service {
       power_monitor_message_broadcaster_;
   std::unique_ptr<PublicIpAddressGeolocationProvider>
       public_ip_address_geolocation_provider_;
+  std::unique_ptr<SensorProviderImpl> sensor_provider_;
   std::unique_ptr<TimeZoneMonitor> time_zone_monitor_;
   std::unique_ptr<usb::DeviceManagerImpl> usb_device_manager_;
   std::unique_ptr<usb::DeviceManagerTest> usb_device_manager_test_;

services/device/device_service_test_base.h

@@ -31,6 +31,7 @@ class DeviceServiceTestBase : public testing::Test {
 
  protected:
   service_manager::Connector* connector() { return connector_.get(); }
+  DeviceService* device_service() { return service_.get(); }
 
   // Can optionally be called to destroy the service before a child test fixture
   // shuts down, in case the DeviceService has dependencies on objects created

services/device/generic_sensor/generic_sensor_service_unittest.cc

@@ -6,21 +6,14 @@
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/macros.h"
-#include "base/memory/ref_counted.h"
-#include "base/memory/singleton.h"
 #include "base/run_loop.h"
-#include "base/single_thread_task_runner.h"
-#include "base/synchronization/waitable_event.h"
 #include "base/test/scoped_feature_list.h"
-#include "base/threading/platform_thread.h"
-#include "build/build_config.h"
-#include "device/base/synchronization/one_writer_seqlock.h"
 #include "mojo/public/cpp/bindings/binding.h"
+#include "services/device/device_service.h"
 #include "services/device/device_service_test_base.h"
 #include "services/device/generic_sensor/fake_platform_sensor_and_provider.h"
 #include "services/device/generic_sensor/platform_sensor.h"
 #include "services/device/generic_sensor/platform_sensor_provider.h"
-#include "services/device/generic_sensor/sensor_provider_impl.h"
 #include "services/device/public/cpp/device_features.h"
 #include "services/device/public/cpp/generic_sensor/sensor_reading.h"
 #include "services/device/public/cpp/generic_sensor/sensor_traits.h"
@@ -154,51 +147,23 @@ class TestSensorClient : public mojom::SensorClient {
 
 class GenericSensorServiceTest : public DeviceServiceTestBase {
  public:
-  GenericSensorServiceTest()
-      : io_loop_finished_event_(
-            base::WaitableEvent::ResetPolicy::AUTOMATIC,
-            base::WaitableEvent::InitialState::NOT_SIGNALED) {}
+  GenericSensorServiceTest() = default;
 
   void SetUp() override {
     scoped_feature_list_.InitWithFeatures(
         {features::kGenericSensor, features::kGenericSensorExtraClasses}, {});
     DeviceServiceTestBase::SetUp();
-    io_task_runner_->PostTask(
-        FROM_HERE, base::BindOnce(&GenericSensorServiceTest::SetUpOnIOThread,
-                                  base::Unretained(this)));
-    io_loop_finished_event_.Wait();
 
-    connector()->BindInterface(mojom::kServiceName, &sensor_provider_);
-  }
-
-  void TearDown() override {
-    io_task_runner_->PostTask(
-        FROM_HERE, base::BindOnce(&GenericSensorServiceTest::TearDownOnIOThread,
-                                  base::Unretained(this)));
-    io_loop_finished_event_.Wait();
-  }
-
-  void SetUpOnIOThread() {
     fake_platform_sensor_provider_ = new FakePlatformSensorProvider();
-    PlatformSensorProvider::SetProviderForTesting(
-        fake_platform_sensor_provider_);
-    io_loop_finished_event_.Signal();
+    device_service()->SetPlatformSensorProviderForTesting(
+        base::WrapUnique(fake_platform_sensor_provider_));
+    connector()->BindInterface(mojom::kServiceName, &sensor_provider_);
   }
 
-  void TearDownOnIOThread() {
-    PlatformSensorProvider::SetProviderForTesting(nullptr);
-
-    DCHECK(fake_platform_sensor_provider_);
-    delete fake_platform_sensor_provider_;
-    fake_platform_sensor_provider_ = nullptr;
-
-    io_loop_finished_event_.Signal();
-  }
   mojom::SensorProviderPtr sensor_provider_;
-  base::WaitableEvent io_loop_finished_event_;
   base::test::ScopedFeatureList scoped_feature_list_;
 
-  // FakePlatformSensorProvider must be created and deleted in IO thread.
+  // This object is owned by the DeviceService instance.
   FakePlatformSensorProvider* fake_platform_sensor_provider_;
 
   DISALLOW_COPY_AND_ASSIGN(GenericSensorServiceTest);

services/device/generic_sensor/platform_sensor_and_provider_unittest.cc

@@ -21,7 +21,6 @@ class PlatformSensorProviderTest : public testing::Test {
  public:
   PlatformSensorProviderTest() {
     provider_ = std::make_unique<FakePlatformSensorProvider>();
-    PlatformSensorProvider::SetProviderForTesting(provider_.get());
   }
 
  protected:

services/device/generic_sensor/platform_sensor_and_provider_unittest_win.cc

@@ -3,7 +3,6 @@
 // found in the LICENSE file.
 
 #include <SensorsApi.h>
-#include <objbase.h>
 #include <sensors.h>
 
 #include "base/bind.h"
@@ -12,6 +11,7 @@
 #include "base/test/scoped_task_environment.h"
 #include "base/win/iunknown_impl.h"
 #include "base/win/propvarutil.h"
+#include "base/win/scoped_com_initializer.h"
 #include "base/win/scoped_propvariant.h"
 #include "services/device/generic_sensor/fake_platform_sensor_and_provider.h"
 #include "services/device/generic_sensor/generic_sensor_consts.h"
@@ -195,7 +195,6 @@ class PlatformSensorAndProviderTestWin : public ::testing::Test {
             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>();
@@ -203,14 +202,8 @@ class PlatformSensorAndProviderTestWin : public ::testing::Test {
     sensor_manager_->QueryInterface(IID_PPV_ARGS(&manager));
 
     // Overrides default ISensorManager with mocked interface.
-    PlatformSensorProviderWin::GetInstance()->SetSensorManagerForTesting(
-        manager);
-  }
-
-  void TearDown() override {
-    Microsoft::WRL::ComPtr<ISensorManager> null_manager;
-    PlatformSensorProviderWin::GetInstance()->SetSensorManagerForTesting(
-        null_manager);
+    provider_ = std::make_unique<PlatformSensorProviderWin>();
+    provider_->SetSensorManagerForTesting(std::move(manager));
   }
 
  protected:
@@ -223,7 +216,7 @@ class PlatformSensorAndProviderTestWin : public ::testing::Test {
   // PlatformSensorProvider::CreateSensorCallback completion.
   scoped_refptr<PlatformSensor> CreateSensor(mojom::SensorType type) {
     run_loop_ = std::make_unique<base::RunLoop>();
-    PlatformSensorProviderWin::GetInstance()->CreateSensor(
+    provider_->CreateSensor(
         type, base::Bind(&PlatformSensorAndProviderTestWin::SensorCreated,
                          base::Unretained(this)));
     run_loop_->Run();
@@ -388,10 +381,12 @@ class PlatformSensorAndProviderTestWin : public ::testing::Test {
     sensor_events_->OnDataUpdated(sensor_.get(), data_report.Get());
   }
 
+  base::win::ScopedCOMInitializer com_initializer_;
   base::test::ScopedTaskEnvironment scoped_task_environment_;
   scoped_refptr<MockISensorManager> sensor_manager_;
   scoped_refptr<MockISensorCollection> sensor_collection_;
   scoped_refptr<MockISensor> sensor_;
+  std::unique_ptr<PlatformSensorProviderWin> provider_;
   Microsoft::WRL::ComPtr<ISensorEvents> sensor_events_;
   scoped_refptr<PlatformSensor> platform_sensor_;
   // Inner run loop used to wait for async sensor creation callback.
@@ -541,8 +536,7 @@ TEST_F(PlatformSensorAndProviderTestWin, GetMaximumSupportedFrequencyFallback) {
 
 // Tests that Accelerometer readings are correctly converted.
 TEST_F(PlatformSensorAndProviderTestWin, CheckAccelerometerReadingConversion) {
-  mojo::ScopedSharedBufferHandle handle =
-      PlatformSensorProviderWin::GetInstance()->CloneSharedBufferHandle();
+  mojo::ScopedSharedBufferHandle handle = provider_->CloneSharedBufferHandle();
   mojo::ScopedSharedBufferMapping mapping = handle->MapAtOffset(
       sizeof(SensorReadingSharedBuffer),
       SensorReadingSharedBuffer::GetOffset(SensorType::ACCELEROMETER));
@@ -580,8 +574,7 @@ TEST_F(PlatformSensorAndProviderTestWin, CheckAccelerometerReadingConversion) {
 
 // Tests that Gyroscope readings are correctly converted.
 TEST_F(PlatformSensorAndProviderTestWin, CheckGyroscopeReadingConversion) {
-  mojo::ScopedSharedBufferHandle handle =
-      PlatformSensorProviderWin::GetInstance()->CloneSharedBufferHandle();
+  mojo::ScopedSharedBufferHandle handle = provider_->CloneSharedBufferHandle();
   mojo::ScopedSharedBufferMapping mapping = handle->MapAtOffset(
       sizeof(SensorReadingSharedBuffer),
       SensorReadingSharedBuffer::GetOffset(SensorType::GYROSCOPE));
@@ -620,8 +613,7 @@ TEST_F(PlatformSensorAndProviderTestWin, CheckGyroscopeReadingConversion) {
 
 // Tests that Magnetometer readings are correctly converted.
 TEST_F(PlatformSensorAndProviderTestWin, CheckMagnetometerReadingConversion) {
-  mojo::ScopedSharedBufferHandle handle =
-      PlatformSensorProviderWin::GetInstance()->CloneSharedBufferHandle();
+  mojo::ScopedSharedBufferHandle handle = provider_->CloneSharedBufferHandle();
   mojo::ScopedSharedBufferMapping mapping = handle->MapAtOffset(
       sizeof(SensorReadingSharedBuffer),
       SensorReadingSharedBuffer::GetOffset(SensorType::MAGNETOMETER));
@@ -662,8 +654,7 @@ TEST_F(PlatformSensorAndProviderTestWin, CheckMagnetometerReadingConversion) {
 // provided.
 TEST_F(PlatformSensorAndProviderTestWin,
        CheckDeviceOrientationEulerAnglesReadingConversion) {
-  mojo::ScopedSharedBufferHandle handle =
-      PlatformSensorProviderWin::GetInstance()->CloneSharedBufferHandle();
+  mojo::ScopedSharedBufferHandle handle = provider_->CloneSharedBufferHandle();
   mojo::ScopedSharedBufferMapping mapping =
       handle->MapAtOffset(sizeof(SensorReadingSharedBuffer),
                           SensorReadingSharedBuffer::GetOffset(
@@ -705,8 +696,7 @@ TEST_F(PlatformSensorAndProviderTestWin,
 // provided.
 TEST_F(PlatformSensorAndProviderTestWin,
        CheckDeviceOrientationQuaternionReadingConversion) {
-  mojo::ScopedSharedBufferHandle handle =
-      PlatformSensorProviderWin::GetInstance()->CloneSharedBufferHandle();
+  mojo::ScopedSharedBufferHandle handle = provider_->CloneSharedBufferHandle();
   mojo::ScopedSharedBufferMapping mapping =
       handle->MapAtOffset(sizeof(SensorReadingSharedBuffer),
                           SensorReadingSharedBuffer::GetOffset(

services/device/generic_sensor/platform_sensor_fusion_unittest.cc

@@ -30,7 +30,6 @@ class PlatformSensorFusionTest : public testing::Test {
  public:
   PlatformSensorFusionTest() {
     provider_ = std::make_unique<FakePlatformSensorProvider>();
-    PlatformSensorProvider::SetProviderForTesting(provider_.get());
   }
 
  protected:

services/device/generic_sensor/platform_sensor_provider.cc

@@ -14,32 +14,18 @@
 #include "services/device/generic_sensor/platform_sensor_provider_linux.h"
 #endif
 
-namespace {
-
-static device::PlatformSensorProvider* g_provider_for_testing = nullptr;
-
-}  // namespace
-
 namespace device {
 
 // static
-void PlatformSensorProvider::SetProviderForTesting(
-    PlatformSensorProvider* provider) {
-  g_provider_for_testing = provider;
-}
-
-// static
-PlatformSensorProvider* PlatformSensorProvider::GetInstance() {
-  if (g_provider_for_testing)
-    return g_provider_for_testing;
+std::unique_ptr<PlatformSensorProvider> PlatformSensorProvider::Create() {
 #if defined(OS_MACOSX)
-  return PlatformSensorProviderMac::GetInstance();
+  return std::make_unique<PlatformSensorProviderMac>();
 #elif defined(OS_ANDROID)
-  return PlatformSensorProviderAndroid::GetInstance();
+  return std::make_unique<PlatformSensorProviderAndroid>();
 #elif defined(OS_WIN)
-  return PlatformSensorProviderWin::GetInstance();
+  return std::make_unique<PlatformSensorProviderWin>();
 #elif defined(OS_LINUX) && defined(USE_UDEV)
-  return PlatformSensorProviderLinux::GetInstance();
+  return std::make_unique<PlatformSensorProviderLinux>();
 #else
   return nullptr;
 #endif

services/device/generic_sensor/platform_sensor_provider.h

@@ -5,27 +5,25 @@
 #ifndef SERVICES_DEVICE_GENERIC_SENSOR_PLATFORM_SENSOR_PROVIDER_H_
 #define SERVICES_DEVICE_GENERIC_SENSOR_PLATFORM_SENSOR_PROVIDER_H_
 
+#include <memory>
+
 #include "services/device/generic_sensor/platform_sensor_provider_base.h"
 
 namespace device {
 
-// This a singleton class returning the actual sensor provider
-// implementation for the current platform.
+// This the base class for platform-specific sensor provider implementations.
+// In typical usage a single instance is owned by DeviceService.
 class PlatformSensorProvider : public PlatformSensorProviderBase {
  public:
-  // Returns the PlatformSensorProvider singleton.
+  // Returns a PlatformSensorProvider for the current platform.
   // Note: returns 'nullptr' if there is no available implementation for
   // the current platform.
-  static PlatformSensorProvider* GetInstance();
+  static std::unique_ptr<PlatformSensorProvider> Create();
 
-  // This method allows to set a provider for testing and therefore
-  // skip the default platform provider. This allows testing without
-  // relying on the platform provider.
-  static void SetProviderForTesting(PlatformSensorProvider* provider);
+  ~PlatformSensorProvider() override = default;
 
  protected:
   PlatformSensorProvider() = default;
-  ~PlatformSensorProvider() override = default;
 
   DISALLOW_COPY_AND_ASSIGN(PlatformSensorProvider);
 };

services/device/generic_sensor/platform_sensor_provider_android.cc

@@ -24,13 +24,6 @@ using base::android::ScopedJavaLocalRef;
 
 namespace device {
 
-// static
-PlatformSensorProviderAndroid* PlatformSensorProviderAndroid::GetInstance() {
-  return base::Singleton<
-      PlatformSensorProviderAndroid,
-      base::LeakySingletonTraits<PlatformSensorProviderAndroid>>::get();
-}
-
 PlatformSensorProviderAndroid::PlatformSensorProviderAndroid() {
   JNIEnv* env = AttachCurrentThread();
   j_object_.Reset(Java_PlatformSensorProvider_create(env));

services/device/generic_sensor/platform_sensor_provider_android.h

@@ -16,8 +16,6 @@ class PlatformSensorProviderAndroid : public PlatformSensorProvider {
   PlatformSensorProviderAndroid();
   ~PlatformSensorProviderAndroid() override;
 
-  static PlatformSensorProviderAndroid* GetInstance();
-
   void SetSensorManagerToNullForTesting();
 
  protected:

services/device/generic_sensor/platform_sensor_provider_linux.cc

@@ -43,13 +43,6 @@ bool IsFusionSensorType(mojom::SensorType type) {
 }
 }  // namespace
 
-// static
-PlatformSensorProviderLinux* PlatformSensorProviderLinux::GetInstance() {
-  return base::Singleton<
-      PlatformSensorProviderLinux,
-      base::LeakySingletonTraits<PlatformSensorProviderLinux>>::get();
-}
-
 PlatformSensorProviderLinux::PlatformSensorProviderLinux()
     : sensor_nodes_enumerated_(false),
       sensor_nodes_enumeration_started_(false),

services/device/generic_sensor/platform_sensor_provider_linux.h

@@ -11,11 +11,6 @@
 #include "base/sequenced_task_runner.h"
 #include "services/device/generic_sensor/linux/sensor_device_manager.h"
 
-namespace base {
-template <typename T>
-struct DefaultSingletonTraits;
-}  // namespace base
-
 namespace device {
 
 struct SensorInfoLinux;
@@ -23,15 +18,14 @@ struct SensorInfoLinux;
 class PlatformSensorProviderLinux : public PlatformSensorProvider,
                                     public SensorDeviceManager::Delegate {
  public:
-  static PlatformSensorProviderLinux* GetInstance();
+  PlatformSensorProviderLinux();
+  ~PlatformSensorProviderLinux() override;
 
   // Sets another service provided by tests.
   void SetSensorDeviceManagerForTesting(
       std::unique_ptr<SensorDeviceManager> sensor_device_manager);
 
  protected:
-  ~PlatformSensorProviderLinux() override;
-
   void CreateSensorInternal(mojom::SensorType type,
                             SensorReadingSharedBuffer* reading_buffer,
                             const CreateSensorCallback& callback) override;
@@ -39,8 +33,6 @@ class PlatformSensorProviderLinux : public PlatformSensorProvider,
   void FreeResources() override;
 
  private:
-  friend struct base::DefaultSingletonTraits<PlatformSensorProviderLinux>;
-
   friend class PlatformSensorAndProviderLinuxTest;
 
   // This is also needed for testing, as we create one provider per test, and
@@ -50,8 +42,6 @@ class PlatformSensorProviderLinux : public PlatformSensorProvider,
   using SensorDeviceMap =
       std::unordered_map<mojom::SensorType, std::unique_ptr<SensorInfoLinux>>;
 
-  PlatformSensorProviderLinux();
-
   void SensorDeviceFound(
       mojom::SensorType type,
       SensorReadingSharedBuffer* reading_buffer,

services/device/generic_sensor/platform_sensor_provider_mac.cc

@@ -4,7 +4,6 @@
 
 #include "services/device/generic_sensor/platform_sensor_provider_mac.h"
 
-#include "base/memory/singleton.h"
 #include "services/device/generic_sensor/orientation_quaternion_fusion_algorithm_using_euler_angles.h"
 #include "services/device/generic_sensor/platform_sensor_accelerometer_mac.h"
 #include "services/device/generic_sensor/platform_sensor_ambient_light_mac.h"
@@ -13,13 +12,6 @@
 
 namespace device {
 
-// static
-PlatformSensorProviderMac* PlatformSensorProviderMac::GetInstance() {
-  return base::Singleton<
-      PlatformSensorProviderMac,
-      base::LeakySingletonTraits<PlatformSensorProviderMac>>::get();
-}
-
 PlatformSensorProviderMac::PlatformSensorProviderMac() = default;
 
 PlatformSensorProviderMac::~PlatformSensorProviderMac() = default;

services/device/generic_sensor/platform_sensor_provider_mac.h

@@ -14,8 +14,6 @@ class PlatformSensorProviderMac : public PlatformSensorProvider {
   PlatformSensorProviderMac();
   ~PlatformSensorProviderMac() override;
 
-  static PlatformSensorProviderMac* GetInstance();
-
  protected:
   void CreateSensorInternal(mojom::SensorType type,
                             SensorReadingSharedBuffer* reading_buffer,

services/device/generic_sensor/platform_sensor_provider_unittest_android.cc

@@ -4,6 +4,8 @@
 
 #include "services/device/generic_sensor/platform_sensor_provider_android.h"
 
+#include <memory>
+
 #include "base/bind.h"
 #include "base/macros.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -15,8 +17,7 @@ class PlatformSensorProviderTestAndroid : public testing::Test {
   PlatformSensorProviderTestAndroid() = default;
 
   void SetUp() override {
-    provider_ = PlatformSensorProviderAndroid::GetInstance();
-    ASSERT_TRUE(provider_);
+    provider_ = std::make_unique<PlatformSensorProviderAndroid>();
   }
 
   void CreateSensorCallback(scoped_refptr<PlatformSensor> sensor) {
@@ -24,7 +25,7 @@ class PlatformSensorProviderTestAndroid : public testing::Test {
   }
 
  protected:
-  PlatformSensorProviderAndroid* provider_;
+  std::unique_ptr<PlatformSensorProviderAndroid> provider_;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(PlatformSensorProviderTestAndroid);

services/device/generic_sensor/platform_sensor_provider_win.cc

@@ -10,7 +10,6 @@
 #include <iomanip>
 
 #include "base/bind.h"
-#include "base/memory/singleton.h"
 #include "base/task/post_task.h"
 #include "base/task/task_traits.h"
 #include "base/task_runner_util.h"
@@ -22,24 +21,17 @@
 
 namespace device {
 
-// static
-PlatformSensorProviderWin* PlatformSensorProviderWin::GetInstance() {
-  return base::Singleton<
-      PlatformSensorProviderWin,
-      base::LeakySingletonTraits<PlatformSensorProviderWin>>::get();
-}
-
-void PlatformSensorProviderWin::SetSensorManagerForTesting(
-    Microsoft::WRL::ComPtr<ISensorManager> sensor_manager) {
-  sensor_manager_ = sensor_manager;
-}
-
 PlatformSensorProviderWin::PlatformSensorProviderWin()
     : com_sta_task_runner_(base::CreateCOMSTATaskRunnerWithTraits(
           base::TaskPriority::USER_VISIBLE)) {}
 
 PlatformSensorProviderWin::~PlatformSensorProviderWin() = default;
 
+void PlatformSensorProviderWin::SetSensorManagerForTesting(
+    Microsoft::WRL::ComPtr<ISensorManager> sensor_manager) {
+  sensor_manager_ = sensor_manager;
+}
+
 void PlatformSensorProviderWin::CreateSensorInternal(
     mojom::SensorType type,
     SensorReadingSharedBuffer* reading_buffer,

services/device/generic_sensor/platform_sensor_provider_win.h

@@ -10,11 +10,6 @@
 
 #include "services/device/generic_sensor/platform_sensor_provider.h"
 
-namespace base {
-template <typename T>
-struct DefaultSingletonTraits;
-}  // namespace base
-
 namespace device {
 
 class PlatformSensorReaderWin;
@@ -26,7 +21,8 @@ class PlatformSensorReaderWin;
 // - Constructs PlatformSensorWin on IPC thread and returns it to requester.
 class PlatformSensorProviderWin final : public PlatformSensorProvider {
  public:
-  static PlatformSensorProviderWin* GetInstance();
+  PlatformSensorProviderWin();
+  ~PlatformSensorProviderWin() override;
 
   // Overrides ISensorManager COM interface provided by the system, used
   // only for testing purposes.
@@ -34,18 +30,12 @@ class PlatformSensorProviderWin final : public PlatformSensorProvider {
       Microsoft::WRL::ComPtr<ISensorManager> sensor_manager);
 
  protected:
-  ~PlatformSensorProviderWin() override;
-
   // PlatformSensorProvider interface implementation.
   void CreateSensorInternal(mojom::SensorType type,
                             SensorReadingSharedBuffer* reading_buffer,
                             const CreateSensorCallback& callback) override;
 
  private:
-  friend struct base::DefaultSingletonTraits<PlatformSensorProviderWin>;
-
-  PlatformSensorProviderWin();
-
   void InitSensorManager();
   void OnInitSensorManager(mojom::SensorType type,
                            SensorReadingSharedBuffer* reading_buffer,

services/device/generic_sensor/sensor_provider_impl.cc

@@ -37,23 +37,18 @@ bool IsExtraSensorClass(mojom::SensorType type) {
 
 }  // namespace
 
-// static
-void SensorProviderImpl::Create(
-    mojom::SensorProviderRequest request) {
-  PlatformSensorProvider* provider = PlatformSensorProvider::GetInstance();
-  if (provider) {
-    mojo::MakeStrongBinding(base::WrapUnique(new SensorProviderImpl(provider)),
-                            std::move(request));
-  }
-}
-
-SensorProviderImpl::SensorProviderImpl(PlatformSensorProvider* provider)
-    : provider_(provider) {
+SensorProviderImpl::SensorProviderImpl(
+    std::unique_ptr<PlatformSensorProvider> provider)
+    : provider_(std::move(provider)) {
   DCHECK(provider_);
 }
 
 SensorProviderImpl::~SensorProviderImpl() {}
 
+void SensorProviderImpl::Bind(mojom::SensorProviderRequest request) {
+  bindings_.AddBinding(this, std::move(request));
+}
+
 void SensorProviderImpl::GetSensor(mojom::SensorType type,
                                    GetSensorCallback callback) {
   if (!base::FeatureList::IsEnabled(features::kGenericSensorExtraClasses) &&
@@ -99,8 +94,8 @@ void SensorProviderImpl::SensorCreated(
   init_params->client_request = sensor_impl->GetClient();
 
   mojom::SensorPtrInfo sensor_ptr_info;
-  mojo::MakeStrongBinding(std::move(sensor_impl),
-                          mojo::MakeRequest(&sensor_ptr_info));
+  sensor_bindings_.AddBinding(std::move(sensor_impl),
+                              mojo::MakeRequest(&sensor_ptr_info));
   init_params->sensor = std::move(sensor_ptr_info);
 
   init_params->memory = std::move(cloned_handle);

services/device/generic_sensor/sensor_provider_impl.h

@@ -7,6 +7,8 @@
 
 #include "base/macros.h"
 #include "base/single_thread_task_runner.h"
+#include "mojo/public/cpp/bindings/binding_set.h"
+#include "mojo/public/cpp/bindings/strong_binding_set.h"
 #include "services/device/public/mojom/sensor_provider.mojom.h"
 
 namespace device {
@@ -14,19 +16,18 @@ namespace device {
 class PlatformSensorProvider;
 class PlatformSensor;
 
-// Implementation of SensorProvider mojo interface.
-// Uses PlatformSensorProvider singleton to create platform specific instances
-// of PlatformSensor which are used by SensorImpl.
+// Implementation of SensorProvider mojo interface. Owns an instance of
+// PlatformSensorProvider singleton to create platform specific instances
+// of PlatformSensor which are used by SensorImpl. A single instance of this
+// class is owned by DeviceService.
 class SensorProviderImpl final : public mojom::SensorProvider {
  public:
-  static void Create(
-      mojom::SensorProviderRequest request);
-
+  explicit SensorProviderImpl(std::unique_ptr<PlatformSensorProvider> provider);
   ~SensorProviderImpl() override;
 
- private:
-  explicit SensorProviderImpl(PlatformSensorProvider* provider);
+  void Bind(mojom::SensorProviderRequest request);
 
+ private:
   // SensorProvider implementation.
   void GetSensor(mojom::SensorType type,
                  GetSensorCallback callback) override;
@@ -37,7 +38,9 @@ class SensorProviderImpl final : public mojom::SensorProvider {
                      GetSensorCallback callback,
                      scoped_refptr<PlatformSensor> sensor);
 
-  PlatformSensorProvider* provider_;
+  std::unique_ptr<PlatformSensorProvider> provider_;
+  mojo::BindingSet<mojom::SensorProvider> bindings_;
+  mojo::StrongBindingSet<mojom::Sensor> sensor_bindings_;
   base::WeakPtrFactory<SensorProviderImpl> weak_ptr_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(SensorProviderImpl);