Bug 124983 – Nix Upstream: Adding MediaControlsNix to WebCore

• Status: RESOLVED
• Resolution: INVALID
• Component: WebCore Misc.

[Thiago de Barros Lacerda]

Nix Upstream: Adding MediaControlsNix to WebCore

[Thiago de Barros Lacerda]

Created attachment 218015 [details]
Patch

[Thiago de Barros Lacerda]

Created attachment 218016 [details]
Patch

[Eric Carlson]

Have you seen Source/WebCore/Modules/mediacontrols/? The Apple ports use this to implement media controls with JavaScript. The WinCairo port is looking at implementing media controls this was as well [1].

That approach is *much* simpler in the long run. 

[1] https://bugs.webkit.org/show_bug.cgi?id=124868

[Benjamin Poulain]

Comment on attachment 218016 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=218016&action=review

> Source/WebCore/html/shadow/MediaControlsNix.cpp:60
> +    return 0;

return nullptr.

> Source/WebCore/html/shadow/MediaControlsNix.cpp:145
> +    Page* page = document().page();
> +    if (!page)

if (!document().page())
    return

> Source/WebCore/html/shadow/MediaControlsNix.cpp:166
> +    double now = m_mediaController->currentTime();
> +    double duration = m_mediaController->duration();

This should be done _after_ the early return.

> Source/WebCore/html/shadow/MediaControlsNix.cpp:170
> +
> +    Page* page = document().page();
> +    if (!page)
> +        return;

ditto

> Source/WebCore/html/shadow/MediaControlsNix.h:45
> +    void changedMute() OVERRIDE;

Missing virtual.

[Thiago de Barros Lacerda]

(In reply to comment #3)
> Have you seen Source/WebCore/Modules/mediacontrols/? The Apple ports use this to implement media controls with JavaScript. The WinCairo port is looking at implementing media controls this was as well [1].
> 
> That approach is *much* simpler in the long run. 
> 
> [1] https://bugs.webkit.org/show_bug.cgi?id=124868

Thanks Eric, I will take a look on that

[Thiago de Barros Lacerda]

(In reply to comment #4)
> (From update of attachment 218016 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=218016&action=review
> 
> > Source/WebCore/html/shadow/MediaControlsNix.cpp:60
> > +    return 0;
> 
> return nullptr.
> 
> > Source/WebCore/html/shadow/MediaControlsNix.cpp:145
> > +    Page* page = document().page();
> > +    if (!page)
> 
> if (!document().page())
>     return
> 
> > Source/WebCore/html/shadow/MediaControlsNix.cpp:166
> > +    double now = m_mediaController->currentTime();
> > +    double duration = m_mediaController->duration();
> 
> This should be done _after_ the early return.
> 
> > Source/WebCore/html/shadow/MediaControlsNix.cpp:170
> > +
> > +    Page* page = document().page();
> > +    if (!page)
> > +        return;
> 
> ditto
> 
> > Source/WebCore/html/shadow/MediaControlsNix.h:45
> > +    void changedMute() OVERRIDE;
> 
> Missing virtual.

Thanks Benjamin, I will fix those