Bug 103751 – Add support for the 'unpause()' method on MediaController.

• Component: New Bugs
• Status: RESOLVED FIXED

[Jer Noble]

Add support for the 'unpause()' method on MediaController.

[Jer Noble]

Created attachment 176982 [details]
Patch

[WebKit Review Bot]

Attachment 176982 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/medi..." exit_code: 1
LayoutTests/ChangeLog:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
Total errors found: 1 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Jer Noble]

Created attachment 176989 [details]
Patch

Now with 100% more bug numbers in the ChangeLog.

[Jer Noble]

Created attachment 176993 [details]
Patch

Updated media-controller-playback.txt to match new output.

[Eric Carlson]

Comment on attachment 176993 [details]
Patch

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.

[Jer Noble]

(In reply to comment #5)
> (From update of attachment 176993 [details])
> 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.

Okay, I'll add some static functions.

> > 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?

It's the only one which doesn't override something in MediaControllerInterface.

> > LayoutTests/media/media-controller-unpause.html:11
> > +        function start() {
> 
> Nit: the brace should be on a new line.

Will change (and all the dittos).

[Jer Noble]

Committed r136295: <http://trac.webkit.org/changeset/136295>

[Roger Fong]

Hi, I'm getting a windows build failure:

6>c:\cygwin\home\buildbot\slave\win-debug\build\source\webcore\html\mediacontroller.cpp(321) : error C2220: warning treated as error - no 'object' file generated
6>c:\cygwin\home\buildbot\slave\win-debug\build\source\webcore\html\mediacontroller.cpp(321) : warning C4715: 'WebCore::MediaController::playbackState' : not all control paths return a value

[Jer Noble]

Committed r136296: <http://trac.webkit.org/changeset/136296>