Bug 156312 - [WK2] Add API to WKWebViewConfiguration to control autoplay policy.
Summary: [WK2] Add API to WKWebViewConfiguration to control autoplay policy.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jer Noble
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2016-04-06 14:37 PDT by Jer Noble
Modified: 2016-10-31 09:18 PDT (History)
12 users (show)

See Also:


Attachments
Patch (20.15 KB, patch)
2016-04-12 11:47 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews105 for mac-yosemite-wk2 (862.46 KB, application/zip)
2016-04-12 12:38 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews121 for ios-simulator-wk2 (823.13 KB, application/zip)
2016-04-12 12:56 PDT, Build Bot
no flags Details
Patch (20.93 KB, patch)
2016-04-12 13:22 PDT, Jer Noble
mitz: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews121 for ios-simulator-wk2 (822.92 KB, application/zip)
2016-04-12 14:30 PDT, Build Bot
no flags Details
Follow-up patch (2.46 KB, patch)
2016-05-17 09:10 PDT, Jer Noble
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jer Noble 2016-04-06 14:37:09 PDT
[WK2] Add API to WKWebViewConfiguration to control autoplay policy.
Comment 1 Jer Noble 2016-04-06 14:39:15 PDT
rdar://problem/25584201
Comment 2 Jer Noble 2016-04-12 11:47:14 PDT
Created attachment 276255 [details]
Patch
Comment 3 WebKit Commit Bot 2016-04-12 11:49:11 PDT
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.
Comment 4 Build Bot 2016-04-12 12:38:10 PDT
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.
Comment 5 Build Bot 2016-04-12 12:38:13 PDT
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
Comment 6 Build Bot 2016-04-12 12:56:14 PDT
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
Comment 7 Build Bot 2016-04-12 12:56:17 PDT
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
Comment 8 Jer Noble 2016-04-12 13:22:52 PDT
Created attachment 276268 [details]
Patch
Comment 9 WebKit Commit Bot 2016-04-12 13:23:38 PDT
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.
Comment 10 Build Bot 2016-04-12 14:30:52 PDT
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
Comment 11 Build Bot 2016-04-12 14:30:55 PDT
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
Comment 12 Anders Carlsson 2016-04-15 17:20:32 PDT
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?
Comment 13 Jer Noble 2016-04-15 17:42:55 PDT
(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 14 mitz 2016-04-18 23:45:15 PDT
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.
Comment 15 Jer Noble 2016-04-19 13:44:43 PDT
(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.
Comment 16 Jer Noble 2016-05-16 11:22:31 PDT
Committed r200951: <http://trac.webkit.org/changeset/200951>
Comment 17 Csaba Osztrogonác 2016-05-17 06:47:16 PDT
(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 )
Comment 18 Jer Noble 2016-05-17 09:04:52 PDT
(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.
Comment 19 Jer Noble 2016-05-17 09:09:59 PDT
Reopening to attach new patch.
Comment 20 Jer Noble 2016-05-17 09:10:03 PDT
Created attachment 279130 [details]
Follow-up patch
Comment 21 WebKit Commit Bot 2016-05-17 09:39:49 PDT
Comment on attachment 279130 [details]
Follow-up patch

Clearing flags on attachment: 279130

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