Website autoplay policies passed to the web frame loader should be respected by media elements.
Created attachment 299054 [details] Patch
Comment on attachment 299054 [details] Patch Attachment 299054 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/2905646 New failing tests: imported/w3c/web-platform-tests/html/browsers/history/the-location-interface/location-protocol-setter-non-broken.html
Created attachment 299059 [details] Archive of layout-test-results from ews102 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews102 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Created attachment 299073 [details] Patch
(In reply to comment #4) > Created attachment 299073 [details] > Patch I don't think that bot failure is related. That test was added just a few hours ago.
Comment on attachment 299073 [details] Patch Attachment 299073 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/2906401 New failing tests: imported/w3c/web-platform-tests/html/browsers/history/the-location-interface/location-protocol-setter-non-broken.html
Created attachment 299087 [details] Archive of layout-test-results from ews102 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews102 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Working on a new patch with tests that also run on iOS.
Comment on attachment 299073 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=299073&action=review > Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:821 > + if (coreFrame && !websitePolicies.autoplayEnabled) { I don't think we need to check coreFrame. It was dereferenced on line 804, so if it were null we would have already crashed. > Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:824 > + settings.setVideoPlaybackRequiresUserGesture(true); > + settings.setAudioPlaybackRequiresUserGesture(true); Are these values set back to their default value for the next navigation? Add a test that sets autoplayEnabled back to true and does another navigation to make sure that it behaves as expected. > Tools/TestWebKitAPI/Tests/WebKit2/autoplayCheck.html:12 > + }, 100) Is 100ms enough time for the video to have always started playing when it is allowed to?
(In reply to comment #9) > Comment on attachment 299073 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=299073&action=review > > > Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:821 > > + if (coreFrame && !websitePolicies.autoplayEnabled) { > > I don't think we need to check coreFrame. It was dereferenced on line 804, > so if it were null we would have already crashed. Removed. > > > Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:824 > > + settings.setVideoPlaybackRequiresUserGesture(true); > > + settings.setAudioPlaybackRequiresUserGesture(true); > > Are these values set back to their default value for the next navigation? > Add a test that sets autoplayEnabled back to true and does another > navigation to make sure that it behaves as expected. Good catch. I polished this up and added a test. > > > Tools/TestWebKitAPI/Tests/WebKit2/autoplayCheck.html:12 > > + }, 100) > > Is 100ms enough time for the video to have always started playing when it is > allowed to? It should be, given the video file is around 100 KB and loaded locally.
Created attachment 299267 [details] Patch
Comment on attachment 299267 [details] Patch Attachment 299267 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/2916574 New failing tests: media/ios/autoplay-only-in-main-document.html
Created attachment 299272 [details] Archive of layout-test-results from ews125 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews125 Port: ios-simulator-wk2 Platform: Mac OS X 10.11.6
Created attachment 299275 [details] Patch
Created attachment 299278 [details] Patch
Comment on attachment 299278 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=299278&action=review > Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:821 > + coreFrame->settings().setAudioPlaybackRequiresUserGesture(!websitePolicies.autoplayEnabled); This overrides the existing settings.setAudioPlaybackRequiresUserGesture. We should remove that if it no longer works.
Comment on attachment 299278 [details] Patch Rejecting attachment 299278 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'build', '--no-clean', '--no-update', '--build-style=release', '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 500 characters of output: TestWebKitAPI/Tests/WebKit2/ForceRepaint.cpp -o /Volumes/Data/EWS/WebKit/WebKitBuild/TestWebKitAPI.build/Release/TestWebKitAPILibrary.build/Objects-normal/x86_64/ForceRepaint.o ** BUILD FAILED ** The following build commands failed: CompileC /Volumes/Data/EWS/WebKit/WebKitBuild/TestWebKitAPI.build/Release/TestWebKitAPILibrary.build/Objects-normal/x86_64/WebsitePolicies.o Tests/WebKit2Cocoa/WebsitePolicies.mm normal x86_64 objective-c++ com.apple.compilers.llvm.clang.1_0.compiler (1 failure) Full output: http://webkit-queues.webkit.org/results/2936483
Created attachment 299532 [details] Patch
Created attachment 299537 [details] Patch
Comment on attachment 299537 [details] Patch Clearing flags on attachment: 299537 Committed r211062: <http://trac.webkit.org/changeset/211062>
All reviewed patches have been landed. Closing bug.
This change appears to have caused 4 API test failures: Tests that failed: RequiresUserActionForPlaybackTest.DeprecatedRequiresUserActionForAudioAndVideoPlayback RequiresUserActionForPlaybackTest.DeprecatedRequiresUserActionForAudioButNotVideoPlayback RequiresUserActionForPlaybackTest.RequiresUserActionForAudioAndVideoPlayback RequiresUserActionForPlaybackTest.RequiresUserActionForAudioButNotVideoPlayback https://build.webkit.org/builders/Apple%20Sierra%20Release%20WK1%20(Tests)/builds/3030
(In reply to comment #22) > This change appears to have caused 4 API test failures: > > Tests that failed: > > RequiresUserActionForPlaybackTest. > DeprecatedRequiresUserActionForAudioAndVideoPlayback > > RequiresUserActionForPlaybackTest. > DeprecatedRequiresUserActionForAudioButNotVideoPlayback > > RequiresUserActionForPlaybackTest.RequiresUserActionForAudioAndVideoPlayback > > RequiresUserActionForPlaybackTest. > RequiresUserActionForAudioButNotVideoPlayback > > https://build.webkit.org/builders/Apple%20Sierra%20Release%20WK1%20(Tests)/ > builds/3030 A follow-up patch that addresses these is almost ready.
https://bugs.webkit.org/show_bug.cgi?id=167346 is tracking a fix. Just waiting for one other patch to land on the commit queue. Thanks for the heads up!
Re-opened since this is blocked by bug 167354
Take 2 of this work has landed with regressions fixed: https://bugs.webkit.org/show_bug.cgi?id=167355 *** This bug has been marked as a duplicate of bug 167355 ***