[Sensors] Improvements in shared buffer managing

third_party/WebKit/Source/modules/sensor/SensorProxy.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/SensorProxy.h"
 
 #include "core/frame/LocalFrame.h"
 #include "modules/sensor/SensorProviderProxy.h"
 #include "platform/mojo/MojoHelper.h"
 
@@ skipping 81 matching lines @@
 }
 
 const device::mojom::blink::SensorConfiguration* SensorProxy::defaultConfig()
     const {
   DCHECK(isInitialized());
   return m_defaultConfig.get();
 }
 
 void SensorProxy::updateInternalReading() {
   DCHECK(isInitialized());
-  Reading* reading = static_cast<Reading*>(m_sharedBuffer.get());
-  m_reading = *reading;
+  int readAttempts = 0;
+  const int kMaxReadAttemptsCount = 10;

[Ken Rockot(use gerrit already), 2016/10/10 16:02:08]

What is the practical difference between trying 1 time vs trying 10 times vs
trying 100 times? Is this really what we want? What is the likelihood that the
attempt will fail once, twice, three times in a row? This just feels arbitrary
and fragile.

[Mikhail, 2016/10/10 18:29:20]

I do not think there is a significant likelihood of failing even once: even when
writes and reads are processed with the same frequency, there is always a lag
between 2 timers (on platform side and in render process) due to IPC mojo calls
so they won't actually overlap.

Assuming that writing takes approximately same amount of asm instructions (x) as
reading, 10*x times should be enough to distinguish between an accidental
failure and a fatal error.

+  while (!tryReadFromBuffer()) {
+    if (++readAttempts == kMaxReadAttemptsCount) {
+      handleSensorError();
+      return;
+    }
+  }
 }
 
 void SensorProxy::RaiseError() {
   handleSensorError();
 }
 
 void SensorProxy::SensorReadingChanged() {
   for (Observer* observer : m_observers)
     observer->onSensorReadingChanged();
 }
@@ skipping 12 matching lines @@
     observer->onSensorError(code, sanitizedMessage, unsanitizedMessage);
 }
 
 void SensorProxy::onSensorCreated(SensorInitParamsPtr params,
                                   SensorClientRequest clientRequest) {
   DCHECK_EQ(Initializing, m_state);
   if (!params) {
     handleSensorError(NotFoundError, "Sensor is not present on the platform.");
     return;
   }
+  const size_t kReadBufferSize = sizeof(ReadingBuffer);
 
-  DCHECK_EQ(0u, params->buffer_offset % SensorInitParams::kReadBufferSize);
+  DCHECK_EQ(0u, params->buffer_offset % kReadBufferSize);
 
   m_mode = params->mode;
   m_defaultConfig = std::move(params->default_configuration);
   if (!m_defaultConfig) {
     handleSensorError();
     return;
   }
 
   DCHECK(m_sensor.is_bound());
   m_clientBinding.Bind(std::move(clientRequest));
 
   m_sharedBufferHandle = std::move(params->memory);
   DCHECK(!m_sharedBuffer);
-  m_sharedBuffer = m_sharedBufferHandle->MapAtOffset(
-      SensorInitParams::kReadBufferSize, params->buffer_offset);
+  m_sharedBuffer =
+      m_sharedBufferHandle->MapAtOffset(kReadBufferSize, params->buffer_offset);
 
   if (!m_sharedBuffer) {
     handleSensorError();
     return;
   }
 
   auto errorCallback =
       WTF::bind(&SensorProxy::handleSensorError, wrapWeakPersistent(this),
                 UnknownError, String("Internal error"), String());
   m_sensor.set_connection_error_handler(
       convertToBaseCallback(std::move(errorCallback)));
 
   m_state = Initialized;
   for (Observer* observer : m_observers)
     observer->onSensorInitialized();
 }
 
+bool SensorProxy::tryReadFromBuffer() {
+  DCHECK(isInitialized());
+  ReadingBuffer* buffer = static_cast<ReadingBuffer*>(m_sharedBuffer.get());
+  device::OneWriterSeqLock& seqlock = buffer->seqlock.value();
+  auto version = seqlock.ReadBegin();
+  auto reading = buffer->reading;
+  if (seqlock.ReadRetry(version))
+    return false;
+  m_reading = reading;
+  return true;
+}
+
 }  // namespace blink