[Webkit-unassigned] [Bug 62698] Support for Battery Status API

[bugzilla-daemon at webkit.org]

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


Adam Barth <abarth at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #132019|review?, commit-queue?      |review+, commit-queue-
               Flag|                            |




--- Comment #50 from Adam Barth <abarth at webkit.org>  2012-03-15 18:12:31 PST ---
(From update of attachment 132019)
View in context: https://bugs.webkit.org/attachment.cgi?id=132019&action=review

This looks good.  I've got a couple nits below, but once those are addressed we can land this patch.  Thanks!

> LayoutTests/ChangeLog:11
> +        * fast/dom/BatteryStatus/add-listener-from-callback-expected.txt: Added.

I might have added these tests to a top-level directory in LayoutTests called battery-status, but the location of the tests aren't super important.

> Source/WebCore/Modules/battery/BatteryController.cpp:60
> +    if (pos == WTF::notFound)
> +        return;

Should it be an error to remove a listener that not in m_listeners?

> Source/WebCore/Modules/battery/BatteryController.h:49
> +    BatteryController(BatteryClient*);

Please add the explicit keyword to single-argument constructors.

> Source/WebCore/Modules/battery/BatteryController.h:52
> +    typedef Vector<BatteryManager*> ListenerVector;

Normally we'd put this declaration right below "private:" with a blank line between it and the constructor.

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