Bug 66978 – <video> playlist can not advance when playing in background tab

• Component: Media
• Status: RESOLVED FIXED

[Eric Carlson]

https://bugs.webkit.org/show_bug.cgi?id=34981 changed HTMLMediaElement so it does not begin loading media data when page->canStartMedia() returns false (ie. when its page is in a background tab). This was done to prevent media from beginning to load and play when a page is opened in a background tab, but it also prevents a script in a background tab from change a media element's url, for example to implement a playlist.

[Eric Carlson]

rdar://problem/9917259

[Eric Carlson]

Created attachment 105256 [details]
Proposed patch

[Darin Adler]

Comment on attachment 105256 [details]
Proposed patch

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

> Source/WebCore/ChangeLog:4
> +        <video> playlist can not advance when playing in background tab
> +        https://bugs.webkit.org/show_bug.cgi?id=66978

Change log should give the reason there is no accompanying regression test.

> Source/WebCore/html/HTMLMediaElement.cpp:496
> -    if (m_restrictions & RequireUserGestureForLoadRestriction && !ScriptController::processingUserGesture())
> +    if (requireUserGestureForLoad() && !ScriptController::processingUserGesture())

Seems this improvement, not needed to fix the bug, could be done in a separate patch.

> Source/WebCore/html/HTMLMediaElement.cpp:595
> +    // Once the page has allowed an element load media, it is free to load at will. This allows a 

Should read: "an element *to* load media".

> Source/WebCore/html/HTMLMediaElement.cpp:1501
> -    if (m_restrictions & RequireUserGestureForRateChangeRestriction && !ScriptController::processingUserGesture())
> +    if (requireUserGestureForRateChange() && !ScriptController::processingUserGesture())

Seems this improvement, not needed to fix the bug, could be done in a separate patch.

> Source/WebCore/html/HTMLMediaElement.cpp:1549
> -    if (m_restrictions & RequireUserGestureForRateChangeRestriction && !ScriptController::processingUserGesture())
> +    if (requireUserGestureForRateChange() && !ScriptController::processingUserGesture())

Seems this improvement, not needed to fix the bug, could be done in a separate patch.

> Source/WebCore/html/HTMLMediaElement.h:195
>      bool requireUserGestureForLoad() const { return m_restrictions & RequireUserGestureForLoadRestriction; }
>      bool requireUserGestureForRateChange() const { return m_restrictions & RequireUserGestureForRateChangeRestriction; }
>      bool requireUserGestureForFullScreen() const { return m_restrictions & RequireUserGestureForFullScreenRestriction; }
> +    bool requirePageConsentToLoadMedia() const { return m_restrictions & RequirePageConsentToLoadMedia; }

For future consideration: These function names have verbs in them and can be interpreted as calls to change behavior restrictions rather than to give the state of those restrictions. It would be best to rename them to start with the word “should” so it’s clear they are boolean getters.

Since these are used only in the class it seems they could start as private. We could make them public later if needed.

> Source/WebCore/html/HTMLMediaElement.h:197
> +    void removeBehaviorRestriction(BehaviorRestrictions restriction) { m_restrictions &= ~restriction; }

Since this is used only in the class, we could start it as private. We could make it public later if needed.

[Eric Carlson]

http://trac.webkit.org/changeset/93878