Autoplay feature policy prevents autoplay with Drive presentations |
||||||||||
Issue descriptionSTR: 1. create slides with Google Drive 2. Insert > Video 3. Put some YouTube keywords 4. Press "Select" 5. Right click on the video 6. Format Options 7. Unbundled "Video Playback" 8. Check "Autoplay when presenting" 9. Press PRESENT Expected result: video autoplays Actual result: video does not autoplay Note: if closing the tab and re-opening the video seems to autoplay when pressing PRESENT (ie. only step 9 because video is already there). I tried to debug and for what I can see, the feature policy rejects the frame. However, it seems that the frame is set up as: main frame > iframe (same origin) > youtube. I wonder if this is surfacing a bug in allow=autoplay or feature policy. Assigning to beccahughes@ to investigate more.
,
Jun 19 2018
It looks like this may be a feature policy bug. We have the following frame tree: top - same origin frame - youtube allow="autoplay" On the YouTube frame we get false from IsFeatureEnabled where this should be true.
,
Jun 22 2018
Any update here? Thanks!
,
Jun 23 2018
I spent some time yesterday (and today) looking at this; it does appear to be a FP bug, but I'm not sure exactly where it's coming from. I'm going to instrument my local chrome and see if I can tell what's happening.
The request for autoplay is indeed being denied by feature policy; while a similar youtube embed in the same document is able to use it. The middle page, which is same-origin to the top-level page, appears to have the correct policy, but it is not being created correctly in the deepest frame (youtube). Even when trying, in devtools, to reload the frame with
frame.src = frame.src
the policy is not correctly constructed. Neither autoplay nor EME are being granted to the frame.
,
Jun 28 2018
This appears to be a site isolation bug (adding Internals>Sandbox>SiteIsolation component), as it does not appear when running chrome with --disable-site-isolation-trials. The policy is being constructed incorrectly in the innermost (youtube) frame. That frame has two remote parent frames (both docs.google.com), and the middle frame's remote proxy in the youtube renderer has an incorrect policy (it does not have any default-'self' features inherited; as if it were cross-origin to its parent.)
,
Jun 28 2018
Thanks for spotting this! I can repro on Windows 67.0.3396.99 Stable and 69.0.3475.0 Canary. I'm curious about the note that closing and re-opening the tab makes the problem go away. I've noticed that it also starts working again if you switch to another slide in the presentation and come back to the affected slide. You can make the bug return by toggling the autoplay checkbox and presenting again. (This suggests there are some reasonable workarounds for this, though we should obviously aim to get it fixed.) Ian, do you have a sense for whether the fix will be tricky? It would be great to target a merge to M68 if possible, but this doesn't seem super concerning if not, given the mitigating factors. I'm CC'ing alexmos@ in case there are things about replication state to discuss. Thanks!
,
Jun 30 2018
(Sorry, Alex, your response came into the wrong mailbox :( ) I've been tracing it to see where the incorrect policy is actually being created -- I've managed to find a simpler repro case (so I should be able to write a test for this). It appears to only happen when the proxy for the same-origin subframe is created inside of an existing SiteInstance. So if we have the frame tree: a.com / b.com and then add another frame, same-origin with the root: a.com / \ b.com a.com/1 Then in b.com's renderer, the policy for the new frame is created with a null origin first, which acts as cross-origin to the root, and then the origin is changed, but the logic in FeaturePolicy::CreateFromPolicyWithOrigin doesn't recalculate the inherited features, so autoplay is disabled there. (In the slides case, the reason this is failing is that the "format options" pane contains an existing iframe to https://www.youtube.com, so when you try to present, that renderer already exists, and when the presentation iframe gets constructed, its proxy in the youtube renderer gets the wrong policy, and the actual presentation video can't use autoplay.) I think this should be reasonably straightforward to fix; this week is probably reasonable.
,
Jun 30 2018
(And by Alex, I mean Charlie, of course -- sorry)
,
Jul 7
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bd2a91ac57dcfe16ce53877c776843a6cf1f8216 commit bd2a91ac57dcfe16ce53877c776843a6cf1f8216 Author: Ian Clelland <iclelland@chromium.org> Date: Sat Jul 07 05:13:57 2018 Fix feature policy construction in proxies. When a proxy for a newly-created frame is created in an existing renderer, the Feature-Policy header and container policy need to be applied after the origin is set. Previously, setting the proxy's origin (as the last step in frame navigation) would reuse the feature policy which was created during frame initialization without taking into account either the HTTP header or the new origin's relationship to the parent frame. This CL ensures that the remote frame stores a copy of the initial feature policy header, which it can use to reconstruct the correct policy when the origin is set. The FeaturePolicy::CreateFromPolicyWithOrigin method, which was used to replicate an existing policy with an updated origin, is removed, as it appears to never be the right behaviour. Bug: 852102 Change-Id: I3e5137737a5aac102f247aa52ede842f1210f53f Reviewed-on: https://chromium-review.googlesource.com/1124931 Reviewed-by: Daniel Cheng <dcheng@chromium.org> Reviewed-by: Alex Moshchuk <alexmos@chromium.org> Commit-Queue: Ian Clelland <iclelland@chromium.org> Cr-Commit-Position: refs/heads/master@{#573149} [modify] https://crrev.com/bd2a91ac57dcfe16ce53877c776843a6cf1f8216/content/browser/site_per_process_browsertest.cc [modify] https://crrev.com/bd2a91ac57dcfe16ce53877c776843a6cf1f8216/third_party/blink/common/feature_policy/feature_policy.cc [modify] https://crrev.com/bd2a91ac57dcfe16ce53877c776843a6cf1f8216/third_party/blink/public/common/feature_policy/feature_policy.h [modify] https://crrev.com/bd2a91ac57dcfe16ce53877c776843a6cf1f8216/third_party/blink/renderer/core/execution_context/security_context.cc [modify] https://crrev.com/bd2a91ac57dcfe16ce53877c776843a6cf1f8216/third_party/blink/renderer/core/execution_context/security_context.h [modify] https://crrev.com/bd2a91ac57dcfe16ce53877c776843a6cf1f8216/third_party/blink/renderer/core/exported/web_remote_frame_impl.cc [modify] https://crrev.com/bd2a91ac57dcfe16ce53877c776843a6cf1f8216/third_party/blink/renderer/core/exported/web_remote_frame_impl.h
,
Jul 8
Verified on ToT that Drive presentations autoplay correctly with the fix.
,
Jul 9
Per comment #6, requesting merge to M68 branch.
,
Jul 9
This bug requires manual review: M68 has already been promoted to the beta branch, so this requires manual review Please contact the milestone owner if you have questions. Owners: cmasso@(Android), kariahda@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jul 9
Thanks iclelland@! Glad you were able to track it down and come up with a test for it. I can confirm the fix on Windows Canary 69.0.3486.0 as well. r573149 landed in 69.0.3485.0, and I don't see any new crashes related to it. Seems reasonably safe from my perspective, and worth having in M68 to deal with the regression in Slides. abdulsyed@, your thoughts?
,
Jul 10
Approved for M68. Branch:3440.
,
Jul 10
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e08b90b80a64022720d0b1b75c890fb11f27ab44 commit e08b90b80a64022720d0b1b75c890fb11f27ab44 Author: Ian Clelland <iclelland@chromium.org> Date: Tue Jul 10 18:22:44 2018 Fix feature policy construction in proxies. When a proxy for a newly-created frame is created in an existing renderer, the Feature-Policy header and container policy need to be applied after the origin is set. Previously, setting the proxy's origin (as the last step in frame navigation) would reuse the feature policy which was created during frame initialization without taking into account either the HTTP header or the new origin's relationship to the parent frame. This CL ensures that the remote frame stores a copy of the initial feature policy header, which it can use to reconstruct the correct policy when the origin is set. The FeaturePolicy::CreateFromPolicyWithOrigin method, which was used to replicate an existing policy with an updated origin, is removed, as it appears to never be the right behaviour. TBR=iclelland@chromium.org (cherry picked from commit bd2a91ac57dcfe16ce53877c776843a6cf1f8216) Bug: 852102 Change-Id: I3e5137737a5aac102f247aa52ede842f1210f53f Reviewed-on: https://chromium-review.googlesource.com/1124931 Reviewed-by: Daniel Cheng <dcheng@chromium.org> Reviewed-by: Alex Moshchuk <alexmos@chromium.org> Commit-Queue: Ian Clelland <iclelland@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#573149} Reviewed-on: https://chromium-review.googlesource.com/1131926 Reviewed-by: Ian Clelland <iclelland@chromium.org> Cr-Commit-Position: refs/branch-heads/3440@{#635} Cr-Branched-From: 010ddcfda246975d194964ccf20038ebbdec6084-refs/heads/master@{#561733} [modify] https://crrev.com/e08b90b80a64022720d0b1b75c890fb11f27ab44/content/browser/site_per_process_browsertest.cc [modify] https://crrev.com/e08b90b80a64022720d0b1b75c890fb11f27ab44/third_party/blink/common/feature_policy/feature_policy.cc [modify] https://crrev.com/e08b90b80a64022720d0b1b75c890fb11f27ab44/third_party/blink/public/common/feature_policy/feature_policy.h [modify] https://crrev.com/e08b90b80a64022720d0b1b75c890fb11f27ab44/third_party/blink/renderer/core/execution_context/security_context.cc [modify] https://crrev.com/e08b90b80a64022720d0b1b75c890fb11f27ab44/third_party/blink/renderer/core/execution_context/security_context.h [modify] https://crrev.com/e08b90b80a64022720d0b1b75c890fb11f27ab44/third_party/blink/renderer/core/exported/web_remote_frame_impl.cc [modify] https://crrev.com/e08b90b80a64022720d0b1b75c890fb11f27ab44/third_party/blink/renderer/core/exported/web_remote_frame_impl.h
,
Jul 10
Thanks for the merge, iclelland@! |
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 by ssaviano@google.com
, Jun 19 2018