New issue
Advanced search Search tips

Issue 617288 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner: ----
Closed: Jun 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

OOPIF: http/tests/security/frameNavigation/not-opener.html times out

Project Member Reported by alex...@chromium.org, Jun 3 2016

Issue description

Repro steps:
$ ninja -C out/gn ... blink_tests
$ third_party/WebKit/Tools/Scripts/run-webkit-tests -t gn -v --no-retry-failures --additional-drt-flag=--site-per-process http/tests/security/frameNavigation/not-opener.html

The test times out with --site-per-process but passes without it.

Expected behavior:

Page A1 window.opens two popups, A2 and B.  A2 is named "targetWindow".  Then, B executes window.open(url_in_site_A, "targetWindow"), i.e. it resolves A2 by name and navigates it to a URL on site A.  Navigating A2 is expected to fail: B isn't allowed to navigate A2 since it doesn't match any of the allowed cases in  https://crbug.com/511474#c1  (namely, it isn't same-origin with A2's opener or any of its ancestors).  Currently, when this happens, window.open just creates a new window instead, so the correct behavior is for this to create a third popup and navigate that to url_in_site_A.

Actual behavior:

The test hangs because the third popup is never created.

I dug into this, and it seems there are two distinct problems involved.

Problem 1:
The third popup isn't created because WebViewTestClient::createView returns nullptr here: 

  if (!test_runner_->canOpenWindows())
    return nullptr;

The test's main page does have a "testRunner.setCanOpenWindows();", but it doesn't look like this is replicated to the cross-site popup B.  Lukasz, is that expected for this flag?

Problem 2:
Commenting out the canOpenWindows check above, everything else in the test finishes correctly, except for this console message, which is not present in --site-per-process:

CONSOLE ERROR: line 4: Unsafe JavaScript attempt to initiate navigation for frame with URL 'http://127.0.0.1:8000/security/frameNavigation/resources/ready.html' from frame with URL 'http://localhost:8000/security/frameNavigation/resources/not-opener-helper.html'. The frame attempting navigation is neither same-origin with the target, nor is it the target's parent or opener.

There is a good explanation for this.  When creating named frame proxies, we intentionally don't create a proxy for A2 in B's SiteInstance, since B isn't supposed to reach A2 (see https://goo.gl/A55qEy).  Thus, while the default mode finds the target frame and then concludes that it isn't supposed to navigate it (in findFrameForNavigation) and outputs the console message, in --site-per-process mode we simply don't find the target frame by name.  The subsequent behavior is the same in both modes -- we create a new popup like we're supposed to.

Given all this, I don't think this is high priority to fix.  Problem 1 is a test harness issue.  For problem 2, it doesn't seem worthwhile to create additional proxies for the sake of maintaining error messages when trying to access them, and behavior-wise we're already doing the right thing.

 
Thanks for digging into this Alex.  Yes - for replicating the test flag across OOPIFs, we need to move TestRunner::can_open_windows_ field into layout_test_runtime_flags.h.
Project Member

Comment 2 by bugdroid1@chromium.org, Jun 3 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/81e255f71f16b73d30d81bf00a874886b815a7d9

commit 81e255f71f16b73d30d81bf00a874886b815a7d9
Author: alexmos <alexmos@chromium.org>
Date: Fri Jun 03 22:25:13 2016

Add bug number for http/tests/security/frameNavigation/not-opener.html in site-per-process

BUG= 617288 
NOTRY=true

Review-Url: https://codereview.chromium.org/2037773004
Cr-Commit-Position: refs/heads/master@{#397821}

[modify] https://crrev.com/81e255f71f16b73d30d81bf00a874886b815a7d9/third_party/WebKit/LayoutTests/FlagExpectations/site-per-process

Replicating TestRunner::can_open_windows_ across renderer processes helps avoid a timeout for:

http/tests/security/frameNavigation/not-opener.html
http/tests/security/frameNavigation/xss-DENIED-targeted-link-navigation.html

Unfortunately, there are tests that depend on not replicating the flag across processes (separate processes are possible even without --site-per-process when the test creates new windows).  In particular, the test below fails after fixing the replication:

http/tests/navigation/no-referrer-target-blank.html
Project Member

Comment 5 by bugdroid1@chromium.org, Jun 8 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/85230becec08b0d0c120f700d8372b2ec204aaba

commit 85230becec08b0d0c120f700d8372b2ec204aaba
Author: lukasza <lukasza@chromium.org>
Date: Tue Jun 07 23:59:20 2016

Silencing console messages during some layout tests.

The console messages emitted during the modified tests were different
with and without --site-per-process flag.  The tests are able to verify
the expected behavior without depending on presence of absence of the
console message, and therefore the console output has been silenced to
unify test output between runs with and without --site-per-process flag.

BUG= 617288 

Review-Url: https://codereview.chromium.org/2042043002
Cr-Commit-Position: refs/heads/master@{#398423}

[modify] https://crrev.com/85230becec08b0d0c120f700d8372b2ec204aaba/third_party/WebKit/LayoutTests/FlagExpectations/site-per-process
[modify] https://crrev.com/85230becec08b0d0c120f700d8372b2ec204aaba/third_party/WebKit/LayoutTests/http/tests/security/frameNavigation/not-opener-expected.txt
[modify] https://crrev.com/85230becec08b0d0c120f700d8372b2ec204aaba/third_party/WebKit/LayoutTests/http/tests/security/frameNavigation/not-opener.html
[modify] https://crrev.com/85230becec08b0d0c120f700d8372b2ec204aaba/third_party/WebKit/LayoutTests/http/tests/security/frameNavigation/xss-DENIED-targeted-link-navigation-expected.txt
[modify] https://crrev.com/85230becec08b0d0c120f700d8372b2ec204aaba/third_party/WebKit/LayoutTests/http/tests/security/frameNavigation/xss-DENIED-targeted-link-navigation.html

Status: Fixed (was: Available)

Sign in to add a comment