New issue
Advanced search Search tips

Issue 661725 link

Starred by 2 users

Issue metadata

Status: Duplicate
Merged: issue 161068
Owner:
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 477150



Sign in to add a comment

Propagating user gesture via postMessage doesn't work for OOPIFs

Project Member Reported by lukasza@chromium.org, Nov 2 2016

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.

 
Owner: japhet@chromium.org
Status: Assigned (was: Untriaged)
The test was introduced recently in r429132 and I think never worked with --site-per-process (i.e. there were no green builds on the Site Isolation FYI bots after this test has landed).
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.
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'.
It seems to me that maybe RenderFrameProxy::forwardPostMessage possibly needs to forward a user-gesture state into a new field inside FrameMsg_PostMessage_Params.
Summary: Propagating user gesture via postMessage doesn't work for OOPIFs (was: h/t/s/frameNavigation/xss-ALLOWED-top-navigation-after-postMessage.html fails with --site-per-process)
Project Member

Comment 6 by bugdroid1@chromium.org, 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

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.

Comment 8 by nasko@chromium.org, 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? 
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.
Cc: japhet@chromium.org
Owner: alex...@chromium.org
Reassigning to alexmos based on comment #7.
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
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)
Project Member

Comment 13 by bugdroid1@chromium.org, 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

 Issue 695208  has been merged into this issue.
Project Member

Comment 15 by bugdroid1@chromium.org, 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

Mergedinto: 161068
Status: Duplicate (was: Assigned)
Project Member

Comment 17 by bugdroid1@chromium.org, 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