[webkit-reviews] review granted: [Bug 103751] Add support for the 'unpause()' method on MediaController. : [Attachment 176993] Patch

[bugzilla-daemon at webkit.org]

Eric Carlson <eric.carlson at apple.com> has granted Jer Noble
<jer.noble at apple.com>'s request for review:
Bug 103751: Add support for the 'unpause()' method on MediaController.
https://bugs.webkit.org/show_bug.cgi?id=103751

Attachment 176993: Patch
https://bugs.webkit.org/attachment.cgi?id=176993&action=review

------- Additional Comments from Eric Carlson <eric.carlson at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=176993&action=review


> Source/WebCore/html/MediaController.cpp:301
> +	   return ASCIILiteral("waiting");
> +    case PLAYING:
> +	   return ASCIILiteral("playing");
> +    case ENDED:
> +	   return ASCIILiteral("ended");

It would be much more efficient to create these just once.

> Source/WebCore/html/MediaController.h:70
>      virtual bool paused() const { return m_paused; }
>      virtual void play();
>      virtual void pause();
> +    void unpause();

Why is this the only non-virtual function?

> LayoutTests/media/media-controller-unpause.html:11
> +	   function start() {

Nit: the brace should be on a new line.

> LayoutTests/media/media-controller-unpause.html:22
> +	   function canplaythrough() {

Ditto.

> LayoutTests/media/media-controller-unpause.html:29
> +	   function pause() {

Ditto.

> LayoutTests/media/media-controller-unpause.html:36
> +	   function play() {

Ditto.

> LayoutTests/media/media-controller-unpause.html:44
> +	   function playing() {

Ditto.