Bug 147797 – Media Session: generalize isFocusedContentMediaElementPlaying so it works with other playback attributes

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Matt Rajca]

Media Session Focus manager's 'isFocusedContentMediaElementPlaying' method will be refactored to a more general 'valueForPlaybackAttribute' so it works with other attributes (IsNextTrackControlEnabled and IsPreviousTrackControlEnabled).

[Matt Rajca]

Created attachment 258538 [details]
Patch

[Matt Rajca]

Comment on attachment 258538 [details]
Patch

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

> Source/WebKit2/UIProcess/WebMediaSessionFocusManager.h:35
> +#include <WebCore/MediaRemoteControls.h>

I removed this include after sending out the patch since we don't need it here.

[Simon Fraser (smfr)]

Comment on attachment 258538 [details]
Patch

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

> Source/WebKit2/UIProcess/WebMediaSessionFocusManager.h:66
> +    HashMap<WKMediaSessionFocusManagerPlaybackAttribute, bool> m_playbackAttributes;

Using a HashMap here is overkill, especially since you could put WKMediaSessionFocusManagerPlaybackAttribute into a bit mask.

[Matt Rajca]

Created attachment 258625 [details]
Patch

[Matt Rajca]

(In reply to comment #3)
> Comment on attachment 258538 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=258538&action=review
> 
> > Source/WebKit2/UIProcess/WebMediaSessionFocusManager.h:66
> > +    HashMap<WKMediaSessionFocusManagerPlaybackAttribute, bool> m_playbackAttributes;
> 
> Using a HashMap here is overkill, especially since you could put
> WKMediaSessionFocusManagerPlaybackAttribute into a bit mask.

Switched over!

[Simon Fraser (smfr)]

Comment on attachment 258625 [details]
Patch

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

> Source/WebKit2/UIProcess/API/C/WKMediaSessionFocusManager.h:44
>  enum {
> -    IsPlaying = 1 << 0
> +    IsPlaying                     = 1 << 0,
> +    IsPreviousTrackControlEnabled = 1 << 1,
> +    IsNextTrackControlEnabled     = 1 << 2,
>  };
>  typedef uint32_t WKMediaSessionFocusManagerPlaybackAttribute;

This should be enum WKMediaSessionFocusManagerPlaybackAttribute {  ... }
typedef uint32_t WKMediaSessionFocusManagerPlaybackAttributes;

> Source/WebKit2/UIProcess/WebMediaSessionFocusManager.h:65
> +    WKMediaSessionFocusManagerPlaybackAttribute m_playbackAttributes { 0 };

WKMediaSessionFocusManagerPlaybackAttributes

[Matt Rajca]

Created attachment 258630 [details]
Patch

[Matt Rajca]

(In reply to comment #6)
> Comment on attachment 258625 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=258625&action=review
> 
> > Source/WebKit2/UIProcess/API/C/WKMediaSessionFocusManager.h:44
> >  enum {
> > -    IsPlaying = 1 << 0
> > +    IsPlaying                     = 1 << 0,
> > +    IsPreviousTrackControlEnabled = 1 << 1,
> > +    IsNextTrackControlEnabled     = 1 << 2,
> >  };
> >  typedef uint32_t WKMediaSessionFocusManagerPlaybackAttribute;
> 
> This should be enum WKMediaSessionFocusManagerPlaybackAttribute {  ... }
> typedef uint32_t WKMediaSessionFocusManagerPlaybackAttributes;
> 
> > Source/WebKit2/UIProcess/WebMediaSessionFocusManager.h:65
> > +    WKMediaSessionFocusManagerPlaybackAttribute m_playbackAttributes { 0 };
> 
> WKMediaSessionFocusManagerPlaybackAttributes

Fixed.

[Simon Fraser (smfr)]

Comment on attachment 258630 [details]
Patch

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

> Source/WebKit2/UIProcess/API/C/WKMediaSessionFocusManager.h:65
> +WK_EXPORT bool WKMediaSessionFocusManagerValueForPlaybackAttribute(WKMediaSessionFocusManagerRef manager, WKMediaSessionFocusManagerPlaybackAttribute attribute);

No need to name the parameters here.

[Matt Rajca]

Created attachment 258633 [details]
Patch

[Matt Rajca]

Committed r188218: <http://trac.webkit.org/changeset/188218>