window.internals.settings.setMediaPlaybackRequiresUserGesture is not replicated across OOPIFs |
|||
Issue description
Repro:
$ third_party/WebKit/Tools/Scripts/run-webkit-tests \
--additional-drt-flag=--site-per-process \
-t gn -v http/tests/webaudio/autoplay-crossorigin.html
,
Oct 3 2016
It seems that the following lines from http/tests/webaudio/autoplay-crossorigin.html:
if ('internals' in window)
window.internals.settings.setMediaPlaybackRequiresUserGesture(true);
only have effect in a single renderer process. A simple fix would be to replicate the lines above into third_party/WebKit/LayoutTests/http/tests/webaudio/resources/autoplay-crossorigin-iframe.html (FWIW, I've verified that the test starts to pass [in --site-per-process mode] on my machine after this fix).
Anyway - for now I think I'll just quickly prepare a CL to disable the test for --site-per-process mode to make the bot green again (and we can decide about a proper fix later).
,
Oct 3 2016
,
Oct 3 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c1bc520322913b5b384414980909b6937beab98f commit c1bc520322913b5b384414980909b6937beab98f Author: lukasza <lukasza@chromium.org> Date: Mon Oct 03 18:58:03 2016 Skip http/tests/webaudio/autoplay-crossorigin.html in --site-per-process mode. BUG= 652366 NOTRY=true Review-Url: https://codereview.chromium.org/2392643002 Cr-Commit-Position: refs/heads/master@{#422477} [modify] https://crrev.com/c1bc520322913b5b384414980909b6937beab98f/third_party/WebKit/LayoutTests/FlagExpectations/site-per-process
,
Oct 4 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f33a8c06201766c37344768aaf99846f78857ff3 commit f33a8c06201766c37344768aaf99846f78857ff3 Author: lukasza <lukasza@chromium.org> Date: Tue Oct 04 10:50:13 2016 Move setMediaPlaybackRequiresUserGesture to the cross-origin frame. Before this CL the http/tests/webaudio/autoplay-crossorigin.html layout test would fail if the main frame and the cross-origin frame are in different renderer processes. After this CL, the internal settings are set from the same frame that hosts AudioContext object - this ensures that setting the settings and getting the settings happens from the same renderer process (and consequently ensures that the test passes both with and without --site-per-process flag). BUG= 652366 Review-Url: https://codereview.chromium.org/2389963002 Cr-Commit-Position: refs/heads/master@{#422746} [modify] https://crrev.com/f33a8c06201766c37344768aaf99846f78857ff3/third_party/WebKit/LayoutTests/FlagExpectations/site-per-process [modify] https://crrev.com/f33a8c06201766c37344768aaf99846f78857ff3/third_party/WebKit/LayoutTests/http/tests/webaudio/autoplay-crossorigin.html [modify] https://crrev.com/f33a8c06201766c37344768aaf99846f78857ff3/third_party/WebKit/LayoutTests/http/tests/webaudio/resources/autoplay-crossorigin-iframe.html
,
Oct 4 2016
https://build.chromium.org/p/chromium.fyi/builders/Site%20Isolation%20Linux/builds/11713 includes the fix from #c5 and is green.
,
Oct 4 2016
Should this be marked as fixed?
,
Oct 4 2016
+dcheng to keep me honest wrt the tradeoff between 1) fixing individual tests to make them OOPIF-friendly, vs 2) fixing the test infrastructure so that all tests are automatically OOPIF-friendly. I think it is indeed okay to mark the whole bug as fixed: As I pointed out in https://crrev.com/2389963002/#msg4 I don't think we have a good reason today to start replicating all of blink::WebSettings: - It should be relatively straightforward to fix individual tests (by moving or duplicating calls to window.internals.settings.setFoo(...) - Hopefully not too many tests will be impacted by lack of replication (right now the test found in this bug is the only known case) - Enabling replication is complicated: there is no notification that components/test_runner could listen to, to figure out when WebSettings changes (to start replication); additionally there is no support today for serializing and restoring updates to WebSettings (needed to replicated the updates). |
|||
►
Sign in to add a comment |
|||
Comment 1 by rtoy@chromium.org
, Oct 3 2016