New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 765779 link

Starred by 1 user

Issue metadata

Status: Untriaged
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 477150



Sign in to add a comment

http/tests/loading/bad-server-subframe.html layout test fails with PlzNavigate *and* --site-per-process

Project Member Reported by lukasza@chromium.org, Sep 15 2017

Issue description

Repro steps:

    $ DISPLAY=:20 third_party/WebKit/Tools/Scripts/run-webkit-tests -t gn -v \
        --additional-drt-flag=--site-per-process --no-retry-failures \
        http/tests/loading/bad-server-subframe.html

The test is very simple:

    <script>
    if (window.testRunner)
        testRunner.dumpAsText();
    </script>
    This is a test of load callbacks. It is only useful inside the regression test tool.<br>
    It also assumes there is no web server running locally on port 54321.<br>
    <iframe name="f1" src="http://localhost:54321"></iframe>

The test fails because

    1) There is 1 extra console messages than before: frame "f1" - didFailLoadWithError

       This might be expected with OOPIFs.

    2) The unique name used in some messages is missing, causing
       WebFrameTestClient::DidFailProvisionalLoad (or rather PrintFrameDescription)
       to print (anonymous) instead of "f1".

       This is a bit weird.


This bug was split off from  issue 765752 .
 
Without PlzNavigate (and with --site-per-process), I see that we end up calling UniqueNameHelper::value() on the *same* UniqueNameHelper object that was used when propagating the "f1" name in the following callstack:

    content::UniqueNameHelper::set_propagated_name()
    content::RenderFrameImpl::CreateChildFrame()
    blink::WebLocalFrameImpl::CreateChildFrame()
    blink::HTMLFrameOwnerElement::LoadOrRedirectSubframe()
    blink::HTMLFrameElementBase::OpenURL()
    blink::HTMLFrameElementBase::SetNameAndOpenURL()
    blink::HTMLFrameElementBase::DidNotifySubtreeInsertionsToDocument()
    blink::ContainerNode::NotifyNodeInserted()
    blink::ContainerNode::ParserAppendChild()
    blink::Insert()
    blink::HTMLConstructionSite::ExecuteTask()
    blink::HTMLConstructionSite::ExecuteQueuedTasks()
    blink::HTMLTreeBuilder::ConstructTree()
    blink::HTMLDocumentParser::ProcessTokenizedChunkFromBackgroundParser()
    blink::HTMLDocumentParser::PumpPendingSpeculations()
    blink::HTMLDocumentParser::ResumeParsingAfterYield()

With PlzNavigate (and with --site-per-process), I see that we call UniqueNameHelper::value() on a *different* UniqueNameHelper object than what was used when propagating the "f1" name in the callstack mentioned above.


Description: Show this description
Project Member

Comment 3 by bugdroid1@chromium.org, Sep 15 2017

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

commit 95a033cae7c9e807af8a43f63e59bd5456472796
Author: Lukasz Anforowicz <lukasza@chromium.org>
Date: Fri Sep 15 20:46:30 2017

Disable tests that fail with *both* PlzNavigate and --site-per-process.

NOTRY=true

Bug: 765779
Bug:  765752 
Change-Id: Ic87167872a8d1ff24de902174bcca99bd6385194
Reviewed-on: https://chromium-review.googlesource.com/669499
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502358}
[modify] https://crrev.com/95a033cae7c9e807af8a43f63e59bd5456472796/third_party/WebKit/LayoutTests/FlagExpectations/site-per-process

Comment 4 by clamy@chromium.org, Sep 18 2017

This might be related to the fact that when OOPIF is enabled, failed navigations in subframes may switch processes with PlzNavigate, while they can't without PlzNavigate.
Cc: nasko@chromium.org
Project Member

Comment 6 by bugdroid1@chromium.org, Sep 27 2017

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

commit c2e46640ac2445238837e30544b96fcd48d583f7
Author: Lukasz Anforowicz <lukasza@chromium.org>
Date: Wed Sep 27 22:48:29 2017

Cover virtual/mojo-loading version of test exclusion

NOTRY=true

Bug: 765779
Change-Id: I4c16248e4497ee4c9978d74e64d1591baf45ade7
Reviewed-on: https://chromium-review.googlesource.com/688654
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#504785}
[modify] https://crrev.com/c2e46640ac2445238837e30544b96fcd48d583f7/third_party/WebKit/LayoutTests/FlagExpectations/site-per-process

Blocking: 477150
Labels: Test-Layout

Sign in to add a comment