Bug 156312 – [WK2] Add API to WKWebViewConfiguration to control autoplay policy.

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Jer Noble]

[WK2] Add API to WKWebViewConfiguration to control autoplay policy.

[Jer Noble]

rdar://problem/25584201

[Jer Noble]

Created attachment 276255 [details]
Patch

[WebKit Commit Bot]

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


ERROR: Source/WebKit2/UIProcess/API/Cocoa/WKWebViewConfiguration.h:83:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Total errors found: 1 in 7 files


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

[Build Bot]

Comment on attachment 276255 [details]
Patch

Attachment 276255 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/1143307

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 276262 [details]
Archive of layout-test-results from ews105 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 276255 [details]
Patch

Attachment 276255 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/1143320

New failing tests:
http/tests/security/contentSecurityPolicy/userAgentShadowDOM/allow-video.html
http/tests/security/contentSecurityPolicy/audio-redirect-allowed.html
http/tests/security/contentSecurityPolicy/userAgentShadowDOM/allow-audio.html
fast/dom/beforeload/video-before-load.html
http/tests/appcache/video.html
compositing/layers-inside-overflow-scroll.html
http/tests/security/contentSecurityPolicy/video-redirect-blocked.html
compositing/video/poster.html
imported/blink/compositing/video/video-controls-layer-creation-squashing.html
http/tests/misc/empty-urls.html
imported/w3c/web-platform-tests/html/dom/interfaces.html
http/tests/security/contentSecurityPolicy/video-with-http-url-allowed-by-csp-media-src-star.html
fast/dom/beforeload/remove-video-in-beforeload-listener.html
http/tests/security/contentSecurityPolicy/video-redirect-allowed.html
http/tests/security/contentSecurityPolicy/media-src-blocked.html
http/tests/security/contentSecurityPolicy/audio-redirect-blocked.html
compositing/video/video-object-position.html
http/tests/security/local-video-src-from-remote.html

[Build Bot]

Created attachment 276264 [details]
Archive of layout-test-results from ews121 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews121  Port: ios-simulator-wk2  Platform: Mac OS X 10.10.5

[Jer Noble]

Created attachment 276268 [details]
Patch

[WebKit Commit Bot]

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


ERROR: Source/WebKit2/UIProcess/API/Cocoa/WKWebViewConfiguration.h:83:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Total errors found: 1 in 8 files


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

[Build Bot]

Comment on attachment 276268 [details]
Patch

Attachment 276268 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/1143953

New failing tests:
http/tests/security/local-video-src-from-remote.html
http/tests/security/contentSecurityPolicy/audio-redirect-allowed.html
http/tests/security/contentSecurityPolicy/userAgentShadowDOM/allow-audio.html
http/tests/appcache/video.html
compositing/video/poster.html
http/tests/security/contentSecurityPolicy/video-redirect-blocked.html
compositing/layers-inside-overflow-scroll.html
compositing/video/video-object-position.html
http/tests/misc/empty-urls.html
imported/w3c/web-platform-tests/html/dom/interfaces.html
http/tests/security/contentSecurityPolicy/video-with-http-url-allowed-by-csp-media-src-star.html
fast/dom/beforeload/remove-video-in-beforeload-listener.html
http/tests/security/contentSecurityPolicy/video-redirect-allowed.html
http/tests/security/contentSecurityPolicy/media-src-blocked.html
http/tests/security/contentSecurityPolicy/userAgentShadowDOM/allow-video.html
imported/blink/compositing/video/video-controls-layer-creation-squashing.html
http/tests/security/contentSecurityPolicy/audio-redirect-blocked.html
fast/dom/beforeload/video-before-load.html

[Build Bot]

Created attachment 276278 [details]
Archive of layout-test-results from ews121 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews121  Port: ios-simulator-wk2  Platform: Mac OS X 10.10.5

[Anders Carlsson]

Comment on attachment 276268 [details]
Patch

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

> Source/WebKit2/UIProcess/API/Cocoa/WKWebViewConfiguration.h:83
> +typedef NS_OPTIONS(NSUInteger, WKAudiovisualMediaTypes) {

Do we need to call them Audiovisual media types?

[Jer Noble]

(In reply to comment #12)
> Comment on attachment 276268 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=276268&action=review
> 
> > Source/WebKit2/UIProcess/API/Cocoa/WKWebViewConfiguration.h:83
> > +typedef NS_OPTIONS(NSUInteger, WKAudiovisualMediaTypes) {
> 
> Do we need to call them Audiovisual media types?

"Media" is a heavily overloaded term. I wanted to distinguish them from "media queries" media.

Comment on attachment 276268 [details]
Patch

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

r=me, with concern about binary compatibility

> Source/WebKit2/UIProcess/API/Cocoa/WKWebViewConfiguration.mm:141
> -    _requiresUserActionForVideoPlayback = NO;
> -    _requiresUserActionForAudioPlayback = NO;
> +    _mediaTypesRequiringUserActionForPlayback = WKAudiovisualMediaTypeAudio;

The change log doesn’t mention it, but this is changing the default behavior, right? It seems risky to do so because existing clients may be relying on the old behavior. Especially since clients that use the old API can’t even sense the new behavior: if they call the old requiresUserActionForMediaPlayback it returns NO, even though audio doesn’t require user action.

> Source/WebKit2/UIProcess/API/Cocoa/WKWebViewConfiguration.mm:587
> +    return self.mediaTypesRequiringUserActionForPlayback & WKAudiovisualMediaTypeVideo;

This only works because WKAudiovisualMediaTypeVideo is less than 255. Even though this is unlikely to change, I’d write this either as !!(self.mediaTypesRequiringUserActionForPlayback & WKAudiovisualMediaTypeVideo) or self.mediaTypesRequiringUserActionForPlayback & WKAudiovisualMediaTypeVideo == WKAudiovisualMediaTypeVideo.

> Source/WebKit2/UIProcess/API/Cocoa/WKWebViewConfiguration.mm:600
> +    return self.mediaTypesRequiringUserActionForPlayback & WKAudiovisualMediaTypeAudio;

Ditto.

[Jer Noble]

(In reply to comment #14)
> Comment on attachment 276268 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=276268&action=review
> 
> r=me, with concern about binary compatibility
> 
> > Source/WebKit2/UIProcess/API/Cocoa/WKWebViewConfiguration.mm:141
> > -    _requiresUserActionForVideoPlayback = NO;
> > -    _requiresUserActionForAudioPlayback = NO;
> > +    _mediaTypesRequiringUserActionForPlayback = WKAudiovisualMediaTypeAudio;
> 
> The change log doesn’t mention it, but this is changing the default
> behavior, right? It seems risky to do so because existing clients may be
> relying on the old behavior. Especially since clients that use the old API
> can’t even sense the new behavior: if they call the old
> requiresUserActionForMediaPlayback it returns NO, even though audio doesn’t
> require user action.

Yes, this is changing the default behavior.  We can (and should!) add a linked-on-or-after check to protect clients who haven't re-compiled with the new API, so that their default behavior stays the same.

> > Source/WebKit2/UIProcess/API/Cocoa/WKWebViewConfiguration.mm:587
> > +    return self.mediaTypesRequiringUserActionForPlayback & WKAudiovisualMediaTypeVideo;
> 
> This only works because WKAudiovisualMediaTypeVideo is less than 255. Even
> though this is unlikely to change, I’d write this either as
> !!(self.mediaTypesRequiringUserActionForPlayback &
> WKAudiovisualMediaTypeVideo) or
> self.mediaTypesRequiringUserActionForPlayback & WKAudiovisualMediaTypeVideo
> == WKAudiovisualMediaTypeVideo.

Ok.

> > Source/WebKit2/UIProcess/API/Cocoa/WKWebViewConfiguration.mm:600
> > +    return self.mediaTypesRequiringUserActionForPlayback & WKAudiovisualMediaTypeAudio;
> 
> Ditto.

Ok.

[Jer Noble]

Committed r200951: <http://trac.webkit.org/changeset/200951>

[Csaba Osztrogonác]

(In reply to comment #16)
> Committed r200951: <http://trac.webkit.org/changeset/200951>

FYI: It made all media tests timeout and bots early exit on GTK and EFL ports.

( cc-ing port maintainers )

[Jer Noble]

(In reply to comment #17)
> (In reply to comment #16)
> > Committed r200951: <http://trac.webkit.org/changeset/200951>
> 
> FYI: It made all media tests timeout and bots early exit on GTK and EFL
> ports.
> 
> ( cc-ing port maintainers )

Ah, this was my mistake. I'll take care of it.

[Jer Noble]

Reopening to attach new patch.

[Jer Noble]

Created attachment 279130 [details]
Follow-up patch

[WebKit Commit Bot]

Comment on attachment 279130 [details]
Follow-up patch

Clearing flags on attachment: 279130

Committed r201016: <http://trac.webkit.org/changeset/201016>