[Sensors] Error handler in Senor class should stop the platform sensor

third_party/WebKit/Source/modules/sensor/Sensor.cpp

 // Copyright 2016 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 "modules/sensor/Sensor.h"
 
 #include "core/dom/Document.h"
 #include "core/dom/ExceptionCode.h"
 #include "core/dom/TaskRunnerHelper.h"
 #include "core/inspector/ConsoleMessage.h"
@@ skipping 40 matching lines @@
           ConsoleMessage::Create(kJSMessageSource, kInfoMessageLevel,
                                  "Frequency is limited to 60 Hz.");
       execution_context->AddConsoleMessage(console_message);
     }
   }
 }
 
 Sensor::~Sensor() = default;
 
 void Sensor::start() {
-  if (state_ != Sensor::SensorState::kIdle)
-    return;
-
-  InitSensorProxyIfNeeded();
-  if (!sensor_proxy_) {
-    ReportError(kInvalidStateError,
-                "The Sensor is no longer associated to a frame.");
-    return;
-  }
-
-  last_update_timestamp_ = WTF::MonotonicallyIncreasingTime();
   StartListening();
 }
 
 void Sensor::stop() {
-  if (state_ == Sensor::SensorState::kIdle)
-    return;
-
   StopListening();
 }
 
 // Getters
 bool Sensor::activated() const {
   return state_ == SensorState::kActivated;
 }
 
 DOMHighResTimeStamp Sensor::timestamp(ScriptState* script_state,
                                       bool& is_null) const {
@@ skipping 70 matching lines @@
     return;
 
   auto provider = SensorProviderProxy::From(document->GetFrame());
   sensor_proxy_ = provider->GetSensorProxy(type_);
 
   if (!sensor_proxy_)
     sensor_proxy_ = provider->CreateSensorProxy(type_, document->GetPage());
 }
 
 void Sensor::ContextDestroyed(ExecutionContext*) {
-  if (state_ == Sensor::SensorState::kActivated ||
-      state_ == Sensor::SensorState::kActivating)
-    StopListening();
+  StopListening();
 }
 
 void Sensor::OnSensorInitialized() {
   if (state_ != Sensor::SensorState::kActivating)
     return;
 
-  StartListening();
+  RequestAddConfiguration();
 }
 
 void Sensor::NotifySensorChanged(double timestamp) {
   if (state_ != Sensor::SensorState::kActivated)
     return;
 
   DCHECK_GT(configuration_->frequency, 0.0);
   double period = 1 / configuration_->frequency;
 
   if (timestamp - last_update_timestamp_ >= period) {
     last_update_timestamp_ = timestamp;
     NotifySensorReadingChanged();
   }
 }
 
 void Sensor::OnSensorError(ExceptionCode code,
                            const String& sanitized_message,
                            const String& unsanitized_message) {
-  ReportError(code, sanitized_message, unsanitized_message);
+  HandleError(code, sanitized_message, unsanitized_message);
 }
 
-void Sensor::OnStartRequestCompleted(bool result) {
+void Sensor::OnAddConfigurationRequestCompleted(bool result) {
   if (state_ != SensorState::kActivating)
     return;
 
   if (!result) {
-    ReportError(kNotReadableError, "start() call has failed.");
+    HandleError(kNotReadableError, "start() call has failed.");
     return;
   }
 
+  // The initial value for m_lastUpdateTimestamp is set to current time,
+  // so that the first reading update will be notified considering the given
+  // frequency hint.
+  last_update_timestamp_ = WTF::MonotonicallyIncreasingTime();
+
   UpdateState(Sensor::SensorState::kActivated);
+
+  if (GetExecutionContext()) {
+    TaskRunnerHelper::Get(TaskType::kSensor, GetExecutionContext())
+        ->PostTask(BLINK_FROM_HERE, WTF::Bind(&Sensor::NotifyOnActivate,
+                                              WrapWeakPersistent(this)));
+  }
 }
 
 void Sensor::StartListening() {
-  DCHECK(sensor_proxy_);
-  UpdateState(Sensor::SensorState::kActivating);
+  if (state_ != SensorState::kIdle)
+    return;
 
-  sensor_proxy_->AddObserver(this);
-  if (!sensor_proxy_->IsInitialized()) {
-    sensor_proxy_->Initialize();
+  InitSensorProxyIfNeeded();
+  if (!sensor_proxy_) {
+    HandleError(kInvalidStateError,
+                "The Sensor is no longer associated to a frame.");
     return;
   }
 
+  if (sensor_proxy_->IsInitialized()) {
+    RequestAddConfiguration();
+  } else {
+    sensor_proxy_->Initialize();
+  }

[Reilly Grant (use Gerrit), 2017/04/26 14:58:45]

nit: no braces when both if and else clauses are a single line.

[Mikhail, 2017/04/27 07:50:34]

Done.

+
+  sensor_proxy_->AddObserver(this);
+  UpdateState(SensorState::kActivating);
+}
+
+void Sensor::StopListening() {
+  if (state_ == SensorState::kIdle)
+    return;
+
+  DCHECK(sensor_proxy_);
+  if (sensor_proxy_->IsInitialized()) {
+    DCHECK(configuration_);
+    sensor_proxy_->RemoveConfiguration(configuration_->Clone());
+  }
+
+  sensor_proxy_->RemoveObserver(this);
+  UpdateState(Sensor::SensorState::kIdle);
+}
+
+void Sensor::RequestAddConfiguration() {
   if (!configuration_) {
     configuration_ = CreateSensorConfig();
     DCHECK(configuration_);
     DCHECK(configuration_->frequency > 0 &&
            configuration_->frequency <=
                SensorConfiguration::kMaxAllowedFrequency);
   }
 
-  auto start_callback =
-      WTF::Bind(&Sensor::OnStartRequestCompleted, WrapWeakPersistent(this));
+  auto start_callback = WTF::Bind(&Sensor::OnAddConfigurationRequestCompleted,

[Reilly Grant (use Gerrit), 2017/04/26 14:58:45]

nit: no need to save the callback in a local variable before passing it to
sensor_proxy_->AddConfiguration().

[Mikhail, 2017/04/27 07:50:34]

Done.

+                                  WrapWeakPersistent(this));
+  DCHECK(sensor_proxy_);
   sensor_proxy_->AddConfiguration(configuration_->Clone(),
                                   std::move(start_callback));
 }
 
-void Sensor::StopListening() {
-  DCHECK(sensor_proxy_);
-  UpdateState(Sensor::SensorState::kIdle);
-
-  if (sensor_proxy_->IsInitialized()) {
-    DCHECK(configuration_);
-    sensor_proxy_->RemoveConfiguration(configuration_->Clone());
-  }
-  sensor_proxy_->RemoveObserver(this);
-}
-
 void Sensor::UpdateState(Sensor::SensorState new_state) {
-  if (new_state == state_)
-    return;
-
-  if (new_state == SensorState::kActivated && GetExecutionContext()) {
-    DCHECK_EQ(SensorState::kActivating, state_);
-    // The initial value for m_lastUpdateTimestamp is set to current time,
-    // so that the first reading update will be notified considering the given
-    // frequency hint.
-    last_update_timestamp_ = WTF::MonotonicallyIncreasingTime();
-    TaskRunnerHelper::Get(TaskType::kSensor, GetExecutionContext())
-        ->PostTask(BLINK_FROM_HERE, WTF::Bind(&Sensor::NotifyOnActivate,
-                                              WrapWeakPersistent(this)));
-  }
-
   state_ = new_state;
 }
 
-void Sensor::ReportError(ExceptionCode code,
+void Sensor::HandleError(ExceptionCode code,
                          const String& sanitized_message,
                          const String& unsanitized_message) {
-  UpdateState(SensorState::kIdle);
+  StopListening();
+
   if (GetExecutionContext()) {
     auto error =
         DOMException::Create(code, sanitized_message, unsanitized_message);
     TaskRunnerHelper::Get(TaskType::kSensor, GetExecutionContext())
         ->PostTask(BLINK_FROM_HERE,
                    WTF::Bind(&Sensor::NotifyError, WrapWeakPersistent(this),
                              WrapPersistent(error)));
   }
 }
 
@@ skipping 16 matching lines @@
 }
 
 bool Sensor::CanReturnReadings() const {
   if (!IsActivated())
     return false;
   DCHECK(sensor_proxy_);
   return sensor_proxy_->Reading().timestamp != 0.0;
 }
 
 }  // namespace blink