[Sensors] Decouple sensor readings update from rAF

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"
 #include "core/timing/DOMWindowPerformance.h"
 #include "core/timing/Performance.h"
 #include "device/generic_sensor/public/interfaces/sensor.mojom-blink.h"
 #include "modules/sensor/SensorErrorEvent.h"
 #include "modules/sensor/SensorProviderProxy.h"
 
 using namespace device::mojom::blink;
 
 namespace blink {
 
+namespace {
+
+constexpr double kMinWaitingInterval =
+    1 / device::mojom::blink::SensorConfiguration::kMaxAllowedFrequency;
+
+}  // namespace
+
 Sensor::Sensor(ExecutionContext* execution_context,
                const SensorOptions& sensor_options,
                ExceptionState& exception_state,
                SensorType type)
     : ContextLifecycleObserver(execution_context),
       sensor_options_(sensor_options),
       type_(type),
       state_(SensorState::kIdle),
-      last_update_timestamp_(0.0) {
+      last_update_timestamp_(0.0),
+      pending_reading_update_(false) {
   // Check secure context.
   String error_message;
   if (!execution_context->IsSecureContext(error_message)) {
     exception_state.ThrowDOMException(kSecurityError, error_message);
     return;
   }
 
   // Check top-level browsing context.
   if (!ToDocument(execution_context)->domWindow()->GetFrame() ||
       !ToDocument(execution_context)->GetFrame()->IsMainFrame()) {
@@ skipping 42 matching lines @@
     is_null = true;
     return 0.0;
   }
 
   Performance* performance = DOMWindowPerformance::performance(*window);
   DCHECK(performance);
   DCHECK(sensor_proxy_);
   is_null = false;
 
   return performance->MonotonicTimeToDOMHighResTimeStamp(
-      sensor_proxy_->Reading().timestamp);
+      sensor_proxy_->reading().timestamp);
 }
 
 DEFINE_TRACE(Sensor) {
   visitor->Trace(sensor_proxy_);
   ActiveScriptWrappable::Trace(visitor);
   ContextLifecycleObserver::Trace(visitor);
   EventTargetWithInlineData::Trace(visitor);
 }
 
 bool Sensor::HasPendingActivity() const {
@@ skipping 23 matching lines @@
 }
 
 double Sensor::ReadingValue(int index, bool& is_null) const {
   is_null = !CanReturnReadings();
   return is_null ? 0.0 : ReadingValueUnchecked(index);
 }
 
 double Sensor::ReadingValueUnchecked(int index) const {
   DCHECK(sensor_proxy_);
   DCHECK(index >= 0 && index < device::SensorReading::kValuesCount);
-  return sensor_proxy_->Reading().values[index];
+  return sensor_proxy_->reading().values[index];
 }
 
 void Sensor::InitSensorProxyIfNeeded() {
   if (sensor_proxy_)
     return;
 
   Document* document = ToDocument(GetExecutionContext());
   if (!document || !document->GetFrame())
     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*) {
   StopListening();
 }
 
 void Sensor::OnSensorInitialized() {
   if (state_ != Sensor::SensorState::kActivating)
     return;
 
   RequestAddConfiguration();
 }
 
-void Sensor::NotifySensorChanged(double timestamp) {
+void Sensor::OnSensorReadingChanged(double timestamp) {
   if (state_ != Sensor::SensorState::kActivated)
     return;
 
+  // Return if reading update is already scheduled or the cached
+  // reading is up-to-date.
+  if (pending_reading_update_ ||
+      sensor_proxy_->reading().timestamp == reading_.timestamp)
+    return;
+
+  pending_reading_update_ = true;
+
+  double elapsedTime = timestamp - last_update_timestamp_;
+
   DCHECK_GT(configuration_->frequency, 0.0);
-  double period = 1 / configuration_->frequency;
+  double waitingTime = 1 / configuration_->frequency - elapsedTime;
 
-  if (timestamp - last_update_timestamp_ >= period) {
-    last_update_timestamp_ = timestamp;
-    NotifySensorReadingChanged();
+  // Negative or zero 'waitingTime' means that polling period has elapsed.
+  // We also avoid scheduling if the elapsed time is slightly behind the
+  // polling period.
+  auto sensor_reading_changed =
+      WTF::Bind(&Sensor::UpdateReading, WrapWeakPersistent(this));
+  if (waitingTime < kMinWaitingInterval) {
+    TaskRunnerHelper::Get(TaskType::kSensor, GetExecutionContext())
+        ->PostTask(BLINK_FROM_HERE, std::move(sensor_reading_changed));

[Reilly Grant (use Gerrit), 2017/05/10 14:53:28]

Why not call UpdateReading directly here?

[Mikhail, 2017/05/11 10:46:20]

It would synchronously invoke JS callbacks that might cause problems (like
modifying SensorProxy::observers_ container while it is being iterated through)

[Reilly Grant (use Gerrit), 2017/05/11 18:28:12]

Can you add a comment explaining that here? I wish we had base::ObserverList in
Blink.

[Mikhail, 2017/05/12 08:42:13]

Done.

+  } else {
+    TaskRunnerHelper::Get(TaskType::kSensor, GetExecutionContext())
+        ->PostDelayedTask(BLINK_FROM_HERE, std::move(sensor_reading_changed),
+                          waitingTime * 1000);
   }
 }
 
 void Sensor::OnSensorError(ExceptionCode code,
                            const String& sanitized_message,
                            const String& unsanitized_message) {
   HandleError(code, sanitized_message, unsanitized_message);
 }
 
 void Sensor::OnAddConfigurationRequestCompleted(bool result) {
@@ skipping 81 matching lines @@
   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)));
   }
 }
 
-void Sensor::NotifySensorReadingChanged() {
-  DCHECK(sensor_proxy_);
-
-  if (sensor_proxy_->Reading().timestamp != stored_data_.timestamp) {
-    stored_data_ = sensor_proxy_->Reading();
-    DispatchEvent(Event::Create(EventTypeNames::change));
-  }
+void Sensor::UpdateReading() {
+  last_update_timestamp_ = WTF::MonotonicallyIncreasingTime();
+  reading_ = sensor_proxy_->reading();
+  pending_reading_update_ = false;
+  DispatchEvent(Event::Create(EventTypeNames::change));
 }
 
 void Sensor::NotifyOnActivate() {
   DispatchEvent(Event::Create(EventTypeNames::activate));
 }
 
 void Sensor::NotifyError(DOMException* error) {
   DispatchEvent(
       SensorErrorEvent::Create(EventTypeNames::error, std::move(error)));
 }
 
 bool Sensor::CanReturnReadings() const {
   if (!IsActivated())
     return false;
   DCHECK(sensor_proxy_);
-  return sensor_proxy_->Reading().timestamp != 0.0;
+  return sensor_proxy_->reading().timestamp != 0.0;
 }
 
 }  // namespace blink