New issue
Advanced search Search tips

Issue 623268 link

Starred by 3 users

Issue metadata

Status: Archived
Owner:
Closed: Dec 5
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug

Blocked on:
issue 718940

Blocking:
issue 477150



Sign in to add a comment

OOPIF: layout tests failures when inspecting a cross-site iframe's properties with --site-per-process

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

Issue description

Not being to inspect contents in OOPIFs in the same DevTools window is a known issue that we intend to fix long-term.  It appears to affect the following layout tests:
http/tests/inspector-protocol/request-mixed-content-status-blockable.html [ Timeout ]
http/tests/inspector-protocol/request-mixed-content-status-none.html [ Timeout ]
http/tests/inspector-protocol/request-mixed-content-status-optionally-blockable.html [ Timeout ]
http/tests/inspector/change-iframe-src.html [ Timeout ]
http/tests/inspector/console-cross-origin-iframe-logging.html [ Timeout ]
http/tests/inspector/inspect-iframe-from-different-domain.html [ Timeout ]

Sample repro:
third_party/WebKit/Tools/Scripts/run-webkit-tests -t gn-debug --additional-driver-flag=--isolate-sites-for-testing=*.is http/tests/inspector/inspect-iframe-from-different-domain.html --driver-logging --full-results-html

This particular test tries to directly inspect the <body> of a cross-site iframe, which loads in a different process with --site-per-process.  The other tests also have cross-origin iframes, and the nature of those failures looks similar.

These tests are already disabled for --site-per-process.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jun 25 2016

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

commit 8e9dc9562c7791803a95f84d89dc9a046e1a24b9
Author: alexmos <alexmos@chromium.org>
Date: Sat Jun 25 06:28:14 2016

Add bug numbers for remaining site-per-process layout test failures.

BUG= 623210 , 623265 , 623268 
TBR=lukasza@chromium.org

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

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

Blockedon: 718940
Project Member

Comment 3 by bugdroid1@chromium.org, May 5 2017

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

commit 9a1b65feb44e32a48ad7fb51384fa868bc864008
Author: lukasza <lukasza@chromium.org>
Date: Fri May 05 18:27:23 2017

OOPIFs: Tweaking test expectations for http/tests/inspector layout tests.

This CL tweaks test expectations for http/tests/inspector layout tests,
to account for work that went into making DevTools support OOPIFs.

These tests used to timeout but now are passing fine:
  http/tests/inspector/change-iframe-src.html
  virtual/mojo-loading/http/tests/inspector/change-iframe-src.html

These tests expected a timeout but not are failing with a text diff
(tracked by https://crbug.com/718940):
  http/tests/inspector/console-cross-origin-iframe-logging.html
  virtual/mojo-loading/http/tests/inspector/console-cross-origin-iframe-logging.html

These tests are intermittently timing out locally and on Site Isolation Win
FYI bot:
  http/tests/inspector-protocol/request-referrer-policy.html
  virtual/mojo-loading/http/tests/inspector-protocol/request-referrer-policy.html

BUG=718940,  623268 
NOTRY=true
TEST=Run 10 times all http/tests/inspector tests covered by the expectations file.

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

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

Project Member

Comment 4 by bugdroid1@chromium.org, Jul 20 2017

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

commit b51a81e48c43becbb456ff0f0820abd950ac2834
Author: Alex Moshchuk <alexmos@chromium.org>
Date: Thu Jul 20 17:46:53 2017

Re-enable --site-per-process layout tests that have healed themselves.

NOTRY=true

Bug:  710098 ,  623268 ,  623210 ,  680307 
Change-Id: I3b086fb6310ba8216cbade6cae6de08e689d460d
Reviewed-on: https://chromium-review.googlesource.com/579891
Reviewed-by: Lukasz Anforowicz <lukasza@chromium.org>
Commit-Queue: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488295}
[modify] https://crrev.com/b51a81e48c43becbb456ff0f0820abd950ac2834/third_party/WebKit/LayoutTests/FlagExpectations/site-per-process

Project Member

Comment 5 by bugdroid1@chromium.org, Oct 4 2017

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

commit 62db39d8dec89187c2518d0046b7ca28d9fc747d
Author: Lukasz Anforowicz <lukasza@chromium.org>
Date: Wed Oct 04 23:23:03 2017

Disable exceptions for tests that have "healed" themselves.

Bug:  758075 ,  700535 ,  582522 ,  602497 ,  616905 
Bug:  623268 ,  623265 ,  678492 
Change-Id: I1bdc648c9aeb97cc1d16b0d8693d04e0c76030de
Reviewed-on: https://chromium-review.googlesource.com/700983
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506557}
[modify] https://crrev.com/62db39d8dec89187c2518d0046b7ca28d9fc747d/third_party/WebKit/LayoutTests/FlagExpectations/site-per-process

Components: Platform>Apps>DevTools
Cc: chenwilliam@chromium.org
 Issue 788801  has been merged into this issue.
Cc: -chenwilliam@chromium.org
Components: -Platform>Apps>DevTools Platform>DevTools
Owner: chenwilliam@chromium.org
Status: Assigned (was: Available)
Will, could you please take a look?

I suspect that mot of these tests need different expectations for OOPIF, since some information (e.g. mixed content urls) is not accessible cross-process.

This one just should not work - I believe it expects iframes to be in the same V8 isolate:
http/tests/inspector/inspect-iframe-from-different-domain.html
Project Member

Comment 9 by bugdroid1@chromium.org, Nov 27 2017

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

commit 72bc6da43e2b5b52a8a32508fc5f777fbc6053e9
Author: Lukasz Anforowicz <lukasza@chromium.org>
Date: Mon Nov 27 19:20:18 2017

Change site-per-process expectations for console-cross-origin...logging

After r518486 the test changed its failure mode in presence of OOPIFs -
it started to time out rather than fail with a text diff.  FWIW, the
overall test failure is tracked by the still active
 https://crbug.com/623268 .

Bug:  623268 
Change-Id: Id394730153af52b6027722541c8a93a55056b370
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/790938
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519362}
[modify] https://crrev.com/72bc6da43e2b5b52a8a32508fc5f777fbc6053e9/third_party/WebKit/LayoutTests/FlagExpectations/site-per-process

Owner: kozy@chromium.org
Blocking: 477150
Labels: -Pri-3 Test-Layout Pri-2
I note that FlagExpectations/site-per-process contains:

 crbug.com/623268  http/tests/inspector-protocol/request-mixed-content-status-blockable.js [ Timeout ]
 crbug.com/623268  http/tests/inspector-protocol/request-mixed-content-status-none.js [ Timeout ]
 crbug.com/623268  http/tests/inspector-protocol/request-mixed-content-status-optionally-blockable.js [ Timeout ]
 crbug.com/623268  http/tests/devtools/console-cross-origin-iframe-logging.js [ Timeout ]

but the tests are also covered by SlowTests:

crbug.com/874695 http/tests/inspector-protocol/request-mixed-content-status-blockable.js [ Slow ]
crbug.com/874695 http/tests/inspector-protocol/request-mixed-content-status-none.js [ Slow ]
crbug.com/874695 http/tests/inspector-protocol/request-mixed-content-status-optionally-blockable.js [ Slow ]
crbug.com/874695 http/tests/devtools/console-cross-origin-iframe-logging.js [ Slow ]

If we can confirm that observed timeouts are just caused by the overall slowness of the tests (i.e. tests would pass if the timeout was increased), then maybe the [ Timeout ] expectation can be simply removed (keeping just the [ Slow ] expectation).

At least for http/tests/inspector-protocol/request-mixed-content-status*, the failures appear to not just be slowness. 

session.protocol.Network.onRequestWillBeSent is expected to be called twice: once for the iframe, and a second time for a subresource. The later isn't happening with --site-per-process.

A similar case in a full build of chrome appears to work: the iframe's subresource appears in the devtools network panel. I'm not sure if it's identical enough to prove that this is a test harness issue, though.
Status: Archived (was: Assigned)
Bulk-closing low priority bugs / feature requests with no action plan.

Sign in to add a comment