Issue metadata
Sign in to add a comment
|
Propagating user gesture via postMessage doesn't work for OOPIFs |
||||||||||||||||||||||||
Issue description$ ninja -C out/gn blink_tests $ third_party/WebKit/Tools/Scripts/run-webkit-tests \ --additional-drt-flag=--site-per-process --no-retry-failures -v -t gn \ http/tests/security/frameNavigation/xss-ALLOWED-top-navigation-after-postMessage.html The test times out when --site-per-process flag is specified.
,
Nov 2 2016
I'll try to take a quick look and disable the test in FlagExpectations/site-per-process if I don't see an obvious fix.
,
Nov 2 2016
The test fails, because top.location assignment throws an exception with --site-per-process saying: SecurityError: Failed to set the 'location' property on 'Window': The current window does not have permission to navigate the target frame to 'http://127.0.0.1:8000/navigation/resources/pass-and-notify-done.html'.
,
Nov 2 2016
It seems to me that maybe RenderFrameProxy::forwardPostMessage possibly needs to forward a user-gesture state into a new field inside FrameMsg_PostMessage_Params.
,
Nov 2 2016
,
Nov 2 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8d6e9a502e76251e663a9312212b66fbf4a6e126 commit 8d6e9a502e76251e663a9312212b66fbf4a6e126 Author: lukasza <lukasza@chromium.org> Date: Wed Nov 02 20:30:16 2016 Test expectation: OOPIFs vs user gesture propagation via postMessage. BUG= 661725 NOTRY=true Review-Url: https://codereview.chromium.org/2469353004 Cr-Commit-Position: refs/heads/master@{#429387} [modify] https://crrev.com/8d6e9a502e76251e663a9312212b66fbf4a6e126/third_party/WebKit/LayoutTests/FlagExpectations/site-per-process
,
Nov 2 2016
postMessage to remote frames doesn't propagate the user gesture, before or after Nate's r429132, which is only tweaking UG behavior for postMessage on local frames. So this test ultimately can't work until we fix the UG plumbing for remote frame postMessages in issue 601584 , which I'm hoping to work on soon. Unfortunately, it's not as easy as passing the user-gesture state in FrameMsg_PostMessage_Params, since sending the postMessage doesn't consume the user gesture, so both the sender and receiver process could potentially consume it after the postMessage event, and we need to ensure only one of them can do it.
,
Nov 2 2016
Can't the act of posting a message disallow consuming the UG bit on the sender side? Whether postMessage is in-process or out-of-process, it is always a task executed in a later iteration of the event loop. Therefore it should be solved somehow already, right?
,
Nov 2 2016
I think the problem is this: just because the post message carries a user gesture with it doesn't mean the target of the post message wants to consume the user gesture. The sender might actually want to consume the user gesture itself to create a new window.
,
Nov 2 2016
Reassigning to alexmos based on comment #7.
,
Nov 4 2016
If the sender wants to consume the gesture to create a new window, doesn't it have to be done before postMessage, even in the single process case? I guess what I don't know is how does this sequence work currently in same process: sender: do some work postMessage do more work that requires user gesture postMessage handler: do work that requires user gesture
,
Nov 4 2016
This should work: both the message sender and the message handler should be able to do work that requires a user gesture. (Of course, if the sender does something that consumes the user gesture, e.g. creating a new window, then the handler will fail when it tries to do user-gesture gated work)
,
Jan 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/96557bfa11c102628943935e7906cff4b3b04a74 commit 96557bfa11c102628943935e7906cff4b3b04a74 Author: alexmos <alexmos@chromium.org> Date: Thu Jan 12 00:14:49 2017 Add expectations for three layout tests failing on Site Isolation bots. This should hopefully get the bots back to green while individual bugs are being investigated. BUG= 680201 , 661725 , 680249 , 680307 NOTRY=true Review-Url: https://codereview.chromium.org/2625233002 Cr-Commit-Position: refs/heads/master@{#443071} [modify] https://crrev.com/96557bfa11c102628943935e7906cff4b3b04a74/third_party/WebKit/LayoutTests/FlagExpectations/site-per-process
,
Feb 22 2017
Issue 695208 has been merged into this issue.
,
Feb 22 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/730a585cf681f768e75ff5fcf65de66b3cbb3fe5 commit 730a585cf681f768e75ff5fcf65de66b3cbb3fe5 Author: dcheng <dcheng@chromium.org> Date: Wed Feb 22 23:15:15 2017 Mark http/tests/security/frameNavigation/xss-ALLOWED-top-navigation-after-postMessage.html as failing in --site-per-process mode. With the framebusting intervention disabled, this seems to emit a console message... but only in --site-per-process mode. BUG= 661725 Review-Url: https://codereview.chromium.org/2711013002 Cr-Commit-Position: refs/heads/master@{#452261} [modify] https://crrev.com/730a585cf681f768e75ff5fcf65de66b3cbb3fe5/third_party/WebKit/LayoutTests/FlagExpectations/site-per-process
,
May 24 2017
,
Mar 5 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/16295fe01a1264e488dc0f7837f53e6f01e74743 commit 16295fe01a1264e488dc0f7837f53e6f01e74743 Author: Lukasz Anforowicz <lukasza@chromium.org> Date: Mon Mar 05 22:18:14 2018 Remove test exceptions for tests that have "healed" themselves Bug: 477150, 789781 , 769508 , 661725 Bug: 611232, 745881 , 801992 Change-Id: If90fb04e97513b99716afd844a2a77ca0905ab3d Reviewed-on: https://chromium-review.googlesource.com/942316 Reviewed-by: Alex Moshchuk <alexmos@chromium.org> Reviewed-by: Charlie Reis <creis@chromium.org> Commit-Queue: Ćukasz Anforowicz <lukasza@chromium.org> Cr-Commit-Position: refs/heads/master@{#540958} [modify] https://crrev.com/16295fe01a1264e488dc0f7837f53e6f01e74743/third_party/WebKit/LayoutTests/FlagExpectations/site-per-process |
|||||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||||
Comment 1 by lukasza@chromium.org
, Nov 2 2016Status: Assigned (was: Untriaged)