[sensors] Remove source sensors fetching from the PlatformSensorFusion class

This patch moves source sensors fetching logic to the newly created
private PlatformSensorFusion::Factory class.
1) PlatformSensorFusion stops exposing a constructor which does not
initialize it fully.
2) PlatformSensorFusion does not need to check it state internally
as source sensors are initialized at construction time.

Rationals: 
Change-Id: I24416e770d60c1d7f0c8ac2cc8cc1758038bca49
Reviewed-on: https://chromium-review.googlesource.com/760420
Commit-Queue: Mikhail Pozdnyakov <mikhail.pozdnyakov@intel.com>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Jun Cai <juncai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515508}

services/device/generic_sensor/fake_platform_sensor_fusion.cc

@@ -11,11 +11,10 @@ namespace device {
 
 FakePlatformSensorFusion::FakePlatformSensorFusion(
     std::unique_ptr<PlatformSensorFusionAlgorithm> fusion_algorithm)
-    : PlatformSensorFusion(
-          nullptr,
-          nullptr,
-          base::Callback<void(scoped_refptr<device::PlatformSensor>)>(),
-          std::move(fusion_algorithm)) {}
+    : PlatformSensorFusion(nullptr,
+                           nullptr,
+                           std::move(fusion_algorithm),
+                           SourcesMap()) {}
 
 bool FakePlatformSensorFusion::GetSourceReading(mojom::SensorType type,
                                                 SensorReading* result) {

services/device/generic_sensor/platform_sensor_fusion.cc

@@ -11,16 +11,80 @@
 
 namespace device {
 
-PlatformSensorFusion::PlatformSensorFusion(
-    mojo::ScopedSharedBufferMapping mapping,
-    PlatformSensorProvider* provider,
-    const PlatformSensorProviderBase::CreateSensorCallback& callback,
-    std::unique_ptr<PlatformSensorFusionAlgorithm> fusion_algorithm)
-    : PlatformSensor(fusion_algorithm->fused_type(),
-                     std::move(mapping),
-                     provider),
-      callback_(callback),
-      fusion_algorithm_(std::move(fusion_algorithm)) {}
+class PlatformSensorFusion::Factory : public base::RefCounted<Factory> {
+ public:
+  static void CreateSensorFusion(
+      mojo::ScopedSharedBufferMapping mapping,
+      std::unique_ptr<PlatformSensorFusionAlgorithm> fusion_algorithm,
+      const PlatformSensorProviderBase::CreateSensorCallback& callback,
+      PlatformSensorProvider* provider) {
+    scoped_refptr<Factory> factory(new Factory(std::move(mapping),
+                                               std::move(fusion_algorithm),
+                                               std::move(callback), provider));
+    factory->FetchSources();
+  }
+
+ private:
+  friend class base::RefCounted<Factory>;
+
+  Factory(mojo::ScopedSharedBufferMapping mapping,
+          std::unique_ptr<PlatformSensorFusionAlgorithm> fusion_algorithm,
+          const PlatformSensorProviderBase::CreateSensorCallback& callback,
+          PlatformSensorProvider* provider)
+      : fusion_algorithm_(std::move(fusion_algorithm)),
+        result_callback_(std::move(callback)),
+        mapping_(std::move(mapping)),
+        provider_(provider) {
+    const auto& types = fusion_algorithm_->source_types();
+    DCHECK(!types.empty());
+    // Make sure there are no dups.
+    DCHECK(std::adjacent_find(types.begin(), types.end()) == types.end());
+    DCHECK(result_callback_);
+    DCHECK(provider_);
+  }
+
+  ~Factory() = default;
+
+  void FetchSources() {
+    for (mojom::SensorType type : fusion_algorithm_->source_types()) {
+      scoped_refptr<PlatformSensor> sensor = provider_->GetSensor(type);
+      if (sensor) {
+        SensorCreated(std::move(sensor));
+      } else {
+        provider_->CreateSensor(type,
+                                base::Bind(&Factory::SensorCreated, this));
+      }
+    }
+  }
+
+  void SensorCreated(scoped_refptr<PlatformSensor> sensor) {
+    if (!result_callback_) {
+      // It is possible, if this callback has been already called
+      // with nullptr (i.e. failed to fetch some of the required
+      // source sensors). See the condition below.
+      return;
+    }
+
+    if (!sensor) {
+      std::move(result_callback_).Run(nullptr);
+      return;
+    }
+
+    sources_map_[sensor->GetType()] = std::move(sensor);
+    if (sources_map_.size() == fusion_algorithm_->source_types().size()) {
+      scoped_refptr<PlatformSensor> fusion_sensor(new PlatformSensorFusion(
+          std::move(mapping_), provider_, std::move(fusion_algorithm_),
+          std::move(sources_map_)));
+      std::move(result_callback_).Run(fusion_sensor);
+    }
+  }
+
+  std::unique_ptr<PlatformSensorFusionAlgorithm> fusion_algorithm_;
+  PlatformSensorProviderBase::CreateSensorCallback result_callback_;
+  mojo::ScopedSharedBufferMapping mapping_;
+  PlatformSensorProvider* provider_;
+  PlatformSensorFusion::SourcesMap sources_map_;
+};
 
 // static
 void PlatformSensorFusion::Create(
@@ -28,9 +92,38 @@ void PlatformSensorFusion::Create(
     PlatformSensorProvider* provider,
     std::unique_ptr<PlatformSensorFusionAlgorithm> fusion_algorithm,
     const PlatformSensorProviderBase::CreateSensorCallback& callback) {
-  auto fusion_sensor = base::MakeRefCounted<PlatformSensorFusion>(
-      std::move(mapping), provider, callback, std::move(fusion_algorithm));
-  fusion_sensor->FetchSourceSensors(provider);
+  Factory::CreateSensorFusion(std::move(mapping), std::move(fusion_algorithm),
+                              callback, provider);
+}
+
+PlatformSensorFusion::PlatformSensorFusion(
+    mojo::ScopedSharedBufferMapping mapping,
+    PlatformSensorProvider* provider,
+    std::unique_ptr<PlatformSensorFusionAlgorithm> fusion_algorithm,
+    PlatformSensorFusion::SourcesMap sources)
+    : PlatformSensor(fusion_algorithm->fused_type(),
+                     std::move(mapping),
+                     provider),
+      fusion_algorithm_(std::move(fusion_algorithm)),
+      source_sensors_(std::move(sources)),
+      reporting_mode_(mojom::ReportingMode::CONTINUOUS) {
+  for (const auto& pair : source_sensors_)
+    pair.second->AddClient(this);
+
+  fusion_algorithm_->set_fusion_sensor(this);
+
+  if (std::any_of(source_sensors_.begin(), source_sensors_.end(),
+                  [](const SourcesMapEntry& pair) {
+                    return pair.second->GetReportingMode() ==
+                           mojom::ReportingMode::ON_CHANGE;
+                  })) {
+    reporting_mode_ = mojom::ReportingMode::ON_CHANGE;
+  }
+}
+
+PlatformSensorFusion::~PlatformSensorFusion() {
+  for (const auto& pair : source_sensors_)
+    pair.second->RemoveClient(this);
 }
 
 mojom::ReportingMode PlatformSensorFusion::GetReportingMode() {
@@ -49,7 +142,6 @@ PlatformSensorConfiguration PlatformSensorFusion::GetDefaultConfiguration() {
 
 bool PlatformSensorFusion::StartSensor(
     const PlatformSensorConfiguration& configuration) {
-  DCHECK(!SourcesNotReady());
   for (const auto& pair : source_sensors_) {
     if (!pair.second->StartListening(this, configuration))
       return false;
@@ -60,7 +152,6 @@ bool PlatformSensorFusion::StartSensor(
 }
 
 void PlatformSensorFusion::StopSensor() {
-  DCHECK(!SourcesNotReady());
   for (const auto& pair : source_sensors_)
     pair.second->StopListening(this);
 
@@ -114,62 +205,4 @@ bool PlatformSensorFusion::GetSourceReading(mojom::SensorType type,
   return false;
 }
 
-PlatformSensorFusion::~PlatformSensorFusion() {
-  if (SourcesNotReady())
-    return;
-
-  for (const auto& pair : source_sensors_)
-    pair.second->RemoveClient(this);
-}
-
-void PlatformSensorFusion::FetchSourceSensors(
-    PlatformSensorProvider* provider) {
-  if (!provider)
-    return;
-
-  for (mojom::SensorType type : fusion_algorithm_->source_types()) {
-    scoped_refptr<PlatformSensor> sensor = provider->GetSensor(type);
-    if (sensor) {
-      AddSourceSensor(std::move(sensor));
-    } else {
-      provider->CreateSensor(
-          type, base::Bind(&PlatformSensorFusion::CreateSensorCallback, this));
-    }
-  }
-}
-
-void PlatformSensorFusion::CreateSensorCallback(
-    scoped_refptr<PlatformSensor> sensor) {
-  if (sensor)
-    AddSourceSensor(std::move(sensor));
-  else if (callback_)
-    std::move(callback_).Run(nullptr);
-}
-
-void PlatformSensorFusion::AddSourceSensor(
-    scoped_refptr<PlatformSensor> sensor) {
-  DCHECK(sensor);
-  mojom::SensorType type = sensor->GetType();
-  source_sensors_[type] = std::move(sensor);
-
-  if (SourcesNotReady())
-    return;
-
-  reporting_mode_ = mojom::ReportingMode::CONTINUOUS;
-
-  for (const auto& pair : source_sensors_) {
-    pair.second->AddClient(this);
-    if (pair.second->GetReportingMode() == mojom::ReportingMode::ON_CHANGE)
-      reporting_mode_ = mojom::ReportingMode::ON_CHANGE;
-  }
-
-  fusion_algorithm_->set_fusion_sensor(this);
-
-  callback_.Run(this);
-}
-
-bool PlatformSensorFusion::SourcesNotReady() const {
-  return source_sensors_.size() != fusion_algorithm_->source_types().size();
-}
-
 }  // namespace device

services/device/generic_sensor/platform_sensor_fusion.h

@@ -43,12 +43,6 @@ class PlatformSensorFusion : public PlatformSensor,
       std::unique_ptr<PlatformSensorFusionAlgorithm> fusion_algorithm,
       const PlatformSensorProviderBase::CreateSensorCallback& callback);
 
-  PlatformSensorFusion(
-      mojo::ScopedSharedBufferMapping mapping,
-      PlatformSensorProvider* provider,
-      const PlatformSensorProviderBase::CreateSensorCallback& callback,
-      std::unique_ptr<PlatformSensorFusionAlgorithm> fusion_algorithm);
-
   // PlatformSensor:
   mojom::ReportingMode GetReportingMode() override;
   PlatformSensorConfiguration GetDefaultConfiguration() override;
@@ -63,22 +57,25 @@ class PlatformSensorFusion : public PlatformSensor,
   virtual bool GetSourceReading(mojom::SensorType type, SensorReading* result);
 
  protected:
+  class Factory;
+  friend class Factory;
+  using SourcesMap =
+      base::flat_map<mojom::SensorType, scoped_refptr<PlatformSensor>>;
+  using SourcesMapEntry =
+      std::pair<mojom::SensorType, scoped_refptr<PlatformSensor>>;
+  PlatformSensorFusion(
+      mojo::ScopedSharedBufferMapping mapping,
+      PlatformSensorProvider* provider,
+      std::unique_ptr<PlatformSensorFusionAlgorithm> fusion_algorithm,
+      SourcesMap sources);
   ~PlatformSensorFusion() override;
-  // Can only be called once, the first time or after a StopSensor call.
   bool StartSensor(const PlatformSensorConfiguration& configuration) override;
   void StopSensor() override;
 
  private:
-  void FetchSourceSensors(PlatformSensorProvider* provider);
-  void CreateSensorCallback(scoped_refptr<PlatformSensor> sensor);
-  void AddSourceSensor(scoped_refptr<PlatformSensor> sensor);
-  bool SourcesNotReady() const;
-
-  PlatformSensorProviderBase::CreateSensorCallback callback_;
   SensorReading reading_;
-  base::flat_map<mojom::SensorType, scoped_refptr<PlatformSensor>>
-      source_sensors_;
   std::unique_ptr<PlatformSensorFusionAlgorithm> fusion_algorithm_;
+  SourcesMap source_sensors_;
   mojom::ReportingMode reporting_mode_;
 
   DISALLOW_COPY_AND_ASSIGN(PlatformSensorFusion);