[sensors] Generic Sensors Framework blink side

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

Index: third_party/WebKit/Source/modules/sensor/SensorProviderProxy.cpp
diff --git a/third_party/WebKit/Source/modules/sensor/SensorProviderProxy.cpp b/third_party/WebKit/Source/modules/sensor/SensorProviderProxy.cpp
new file mode 100644
index 0000000000000000000000000000000000000000..4c3d69dc83d4ac694b8943018ef85fbc789e1ffa
--- /dev/null
+++ b/third_party/WebKit/Source/modules/sensor/SensorProviderProxy.cpp
@@ -0,0 +1,97 @@
+// 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/SensorProviderProxy.h"
+
+#include "core/frame/LocalFrame.h"
+#include "modules/sensor/SensorProxy.h"
+#include "platform/mojo/MojoHelper.h"
+#include "public/platform/InterfaceProvider.h"
+
+using namespace device::mojom::blink;
+
+namespace blink {
+
+const char SensorProviderProxy::s_supplementKey[] = "SensorProvider";
+
+// SensorProviderProxy
+SensorProviderProxy::SensorProviderProxy(LocalFrame* frame)
+    : PageVisibilityObserver(frame->document() ? frame->document()->page() : nullptr)
+    , m_frame(frame)
+{
+    frame->interfaceProvider()->getInterface(mojo::GetProxy(&m_sensorProvider));
+    m_sensorProvider.set_connection_error_handler(convertToBaseCallback(WTF::bind(&SensorProviderProxy::handleSensorError, wrapWeakPersistent(this))));
+}
+
+SensorProviderProxy* SensorProviderProxy::getOrCreateForFrame(LocalFrame* frame)
+{
+    DCHECK(frame);
+    SensorProviderProxy* result = static_cast<SensorProviderProxy*>(Supplement<LocalFrame>::from(*frame, s_supplementKey));
+    if (!result) {
+        result = new SensorProviderProxy(frame);
+        Supplement<LocalFrame>::provideTo(*frame, s_supplementKey, result);
+    }
+    return result;
+}
+
+SensorProviderProxy::~SensorProviderProxy()
+{
+}
+
+DEFINE_TRACE(SensorProviderProxy)
+{
+    visitor->trace(m_sensors);
+    visitor->trace(m_frame);
+    PageVisibilityObserver::trace(visitor);
+    Supplement<LocalFrame>::trace(visitor);
+}
+
+void SensorProviderProxy::removeSensor(SensorProxy* sensor)
+{
+    if (m_sensors.contains(sensor))

[timvolodine, 2016/08/25 17:52:32]

if think you could just .remove() without contains

[Mikhail, 2016/08/26 16:42:42]

Indeed, thanks for pointing out!

+        m_sensors.remove(sensor);
+}
+
+SensorProxy* SensorProviderProxy::getOrCreateSensor(device::mojom::blink::SensorType type)
+{
+    for (SensorProxy* sensor : m_sensors) {
+        if (sensor->type() == type)
+            return sensor;
+    }
+    SensorProxy* sensor = new SensorProxy(type, this);
+    m_sensors.add(sensor);
+
+    return sensor;
+}
+
+void SensorProviderProxy::handleSensorError()

[timvolodine, 2016/08/25 17:52:32]

when does this happen?
slightly confused with Sensor::onSensorError()..

[Mikhail, 2016/08/26 16:42:41]

on IPC connection problems or if the browser-side counter part is not
accessible.
Sensor::onSensorError() will be eventually invoked from this call.

+{
+    SensorsSet copy(m_sensors);
+    for (SensorProxy* sensor : copy) {
+        // All sensors are invalidated.
+        sensor->handleSensorError();
+    }
+    DCHECK(m_sensors.isEmpty());
+    if (m_frame)
+        m_frame->removeSupplement(s_supplementKey);
+}
+
+void SensorProviderProxy::pageVisibilityChanged()
+{
+    if (!m_frame)
+        return;
+
+    Page* page = m_frame->document()->page();
+    DCHECK(page);
+
+    if (page->visibilityState() != PageVisibilityStateVisible) {
+        for (SensorProxy* sensor : m_sensors)

[timvolodine, 2016/08/25 17:52:32]

but Sensor is also listening to page visibility.. should this go there instead?

[Mikhail, 2016/08/26 16:42:42]

We could propagate this call from here to sensors having added smth like
"SensorProxy::Observer::pageVisibilityChanged"
but IMO that would be a bit weird because  "SensorProxy::Observer" is supposed
to propagate notification from a platform sensor, not from the page..

[timvolodine, 2016/09/01 19:02:05]

what I meant is to just suspend() in the Sensor class itself when page
visibility changes (so there would be no need for 'for'-loop). would that work?

[Mikhail, 2016/09/02 08:23:43]

That would work, but since there can be multiple Sensor instances within a
single frame that could cause unneeded IPC calls.
An alternative way IMO could be making 'SensorProxy' a PageVisibilityObserver
itself (instead on 'SensorProviderProxy') so that there would be no loops too.
Do you think that would be better?

[timvolodine, 2016/09/02 19:41:28]

not sure why this would result in more IPCs?
would putting "m_sensorProxy->suspend()" in Sensor::pageVisibilityChanged() not
do the same thing?

[Mikhail, 2016/09/05 10:26:27]

All the JS Sensor instances of the same type within a frame share the same
SensorProxy instance, so "m_sensorProxy->suspend()" will be called N times
(assuming N = amount JS Sensor instances within a frame), otherwise (if either
SensorProviderProxy or SensorProxy inherit PageVisibilityObserver themselves) it
will be a single call.

+            sensor->suspend();
+    } else {
+        for (SensorProxy* sensor : m_sensors)
+            sensor->resume();
+    }
+}
+
+} // namespace blink