[Webkit-unassigned] [Bug 93597] Add DeviceProximityController to support Proximity Events

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=93597





--- Comment #30 from Adam Barth <abarth at webkit.org>  2012-08-14 10:48:02 PST ---
(From update of attachment 158275)
View in context: https://bugs.webkit.org/attachment.cgi?id=158275&action=review

> Source/WebCore/Modules/proximity/DeviceProximityController.cpp:43
> +    didChangeDeviceEvent(event);

Should this have a more "verb-like" name?  For example, dispatchDeviceEvent ?

> Source/WebCore/dom/DeviceClient.h:27
> +class DeviceClient {

This should be in WebCore/page because it's per-page state.

> Source/WebCore/dom/DeviceController.cpp:31
> +    , m_timer(this, &DeviceController::timerFired)

Should we give this a more descriptive name?  What sort of timer is it?  What are we supposed to do when the timer fires?

> Source/WebCore/dom/DeviceController.cpp:72
> +void DeviceController::removeAllListeners(DOMWindow* window)

How does removeListener differ from removeAllListeners?  Can a DOMWindow be registered twice?  Does that mean it gets events fired twice?  I understand now how this works, but it's a bit confusing.  Likely we can solve this problem with better names.

> Source/WebCore/dom/DeviceController.cpp:87
> +    RefPtr<Event> rpEvent = event;

Usually we call the RefPtr version just "event" and the PassRefPtr version prpEvent

> Source/WebCore/dom/DeviceController.cpp:98
> +    int count = m_listeners.count(window);

Should count be a size_t?

> Source/WebCore/dom/DeviceController.h:32
> +class DeviceController : public Supplement<Page> {

This should be in WebCore/page because it's per-page state.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.