[sensors] Generic Sensors Framework blink side

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

+// 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.
+
+#ifndef SensorProviderProxy_h
+#define SensorProviderProxy_h
+
+#include "core/page/PageVisibilityObserver.h"
+#include "device/generic_sensor/public/interfaces/sensor.mojom-blink.h"
+#include "device/generic_sensor/public/interfaces/sensor_provider.mojom-blink.h"
+#include "platform/Supplementable.h"
+#include "platform/heap/Handle.h"
+
+namespace blink {
+
+class LocalFrame;
+class SensorProxy;
+
+// This class wraps 'SensorProvider' mojo interface and it manages
+// 'SensorProxy' instances. It's created lazily for a frame and deleted if:
+// - the frame is deleted
+// - there are no SensorProxies left (i.e. there are no active Sensor
+//   instances within the frame).
+// - a fatal error has occured (mojo IPC channel got broken)
+class SensorProviderProxy final
+    : public GarbageCollectedFinalized<SensorProviderProxy>
+    , public Supplement<LocalFrame>
+    , public PageVisibilityObserver {

[haraken, 2016/09/06 05:04:29]

Do you really need to make both Sensor and SensorProviderProxy
PageVisibilityObservers?

[Mikhail, 2016/09/06 07:13:58]

when page goes invisible sensors stop polling from buffer and
SensorProviderProxy notifies platform side that it might stop notification to
the frame and can reconsider the applied platform sensor configurations.

Alternatively SensorProviderProxy could notify Sensors itself but it does not
look good IMO since SensorProviderProxy should notify about platform sensor
updates, not about page visibility.

+    USING_GARBAGE_COLLECTED_MIXIN(SensorProviderProxy);
+    WTF_MAKE_NONCOPYABLE(SensorProviderProxy);
+public:
+    static SensorProviderProxy* getOrCreateForFrame(LocalFrame*);
+
+    ~SensorProviderProxy();
+
+    SensorProxy* getOrCreateSensor(device::mojom::blink::SensorType);
+
+    DECLARE_VIRTUAL_TRACE();
+
+private:
+    friend class SensorProxy; // To call removeSensor().
+
+    explicit SensorProviderProxy(LocalFrame*);
+
+    // PageVisibilityObserver overrides.
+    void pageVisibilityChanged() override;
+
+    void removeSensor(SensorProxy*);
+    device::mojom::blink::SensorProvider* sensorProvider() const { return m_sensorProvider.get(); }
+
+    void handleSensorError();
+
+    using SensorsSet = HeapHashSet<Member<SensorProxy>>;

[haraken, 2016/09/06 05:04:29]

I think this should be HeapHashSet<WeakMember<>>. Then you can stop calling
removeObserver() in Sensor's pre-finalizer.

I hope we can entirely remove the pre-finalizer from Sensor.

+    SensorsSet m_sensors;
+    static const char s_supplementKey[];
+
+    WeakMember<LocalFrame> m_frame;

[haraken, 2016/09/06 05:04:29]

Why weak?

[Mikhail, 2016/09/06 07:13:58]

Should it be just a member? I thought frame is owning SensorProviderProxy (as a
supplement) not the opposite

+
+    device::mojom::blink::SensorProviderPtr m_sensorProvider;
+};
+
+} // namespace blink
+
+#endif // SensorProviderProxy_h