New issue
Advanced search Search tips

Issue 662550 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

Add a navigation test utility that waits until the previous RenderFrame is gone.

Project Member Reported by creis@chromium.org, Nov 4 2016

Issue description

Right now, we have several navigation/OOPIF tests that are disabled for flakiness because they do not wait for the old process to go away after a cross-process navigation commits.  This can cause DepictFrameTree to occasionally fail if the old proxies are still in the tree.

In https://bugs.chromium.org/p/chromium/issues/detail?id=629419#c9, Nick points out that we can work around this by waiting for the RenderFrame to be deleted, but that he has ideas for a larger NavMaker utility that waits for the correct events.  This bug will track that work, with breadcrumbs for which tests are affected.

So far, the list appears to include:
SitePerProcessBrowserTest.CrossSiteIframe (issue 629419)
SitePerProcessBrowserTest.NavigateRemoteFrame (issue 629419)
SitePerProcessBrowserTest.RFPHDestruction (issue 629419)
TopDocumentIsolationTest.NavigateToSubframeSiteWithPopup2 ( issue 611300 )
TopDocumentIsolationTest.FramesForSitesInHistory ( issue 611344 )
NavigationControllerBrowserTest.FrameNavigationEntry_NewSubframe ( issue 646836 )
 
Cc: lukasza@chromium.org
Thanks for opening the bug.  There is one test that quite frequently fails in (I think) this way on Site Isolation Android bot:

FrameTreeBrowserTest.NavigateGrandchildToBlob

Example build: https://build.chromium.org/p/chromium.fyi/builders/Site%20Isolation%20Android/builds/864

Comment 2 by creis@chromium.org, Nov 4 2016

Great-- thanks for mentioning it.  I have a CL in progress to deflake all these with RenderFrameDeletedObserver (without tackling the new test utility idea).

Comment 3 by creis@chromium.org, Nov 4 2016

I'm trying out fixes for most of them in https://codereview.chromium.org/2478933002/.  I'll send it for review once the try jobs pass.

NavigationControllerBrowserTest.FrameNavigationEntry_NewSubframe and FrameTreeBrowserTest.NavigateGrandchildToBlob are proving tricky, so I'll try to take another look at them.  (They don't fail when I inject a delay like the others do, and they time out when I use the same type of fix.)

Comment 4 by creis@chromium.org, Nov 5 2016

Ah, the other two only need the listener in --site-per-process mode (and will time out otherwise).  I'll follow up with a CL for them as well.
Project Member

Comment 5 by bugdroid1@chromium.org, Nov 5 2016

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

commit 7a65c0019399f0a7052ab13bc73439acb116adb8
Author: creis <creis@chromium.org>
Date: Sat Nov 05 00:23:04 2016

Fix flaky OOPIF tests by waiting for the old RenderFrame to be deleted.

This fixes:
SitePerProcessBrowserTest.CrossSiteIframe
SitePerProcessBrowserTest.NavigateRemoteFrame
SitePerProcessBrowserTest.RFPHDestruction
TopDocumentIsolationTest.NavigateToSubframeSiteWithPopup2
TopDocumentIsolationTest.FramesForSitesInHistory

BUG=662550, 629419,  611300 ,  611344 
TEST=Bots stay green.

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

[modify] https://crrev.com/7a65c0019399f0a7052ab13bc73439acb116adb8/content/browser/site_per_process_browsertest.cc
[modify] https://crrev.com/7a65c0019399f0a7052ab13bc73439acb116adb8/content/browser/top_document_isolation_browsertest.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Nov 5 2016

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

commit c6abe5bfc52cc830ee64a6277e19a8ad0fa71e2f
Author: mathp <mathp@chromium.org>
Date: Sat Nov 05 03:40:37 2016

Revert of Fix flaky OOPIF tests by waiting for the old RenderFrame to be deleted. (patchset #1 id:1 of https://codereview.chromium.org/2478933002/ )

Reason for revert:
Causing flakiness still:

https://build.chromium.org/p/chromium.memory/builders/Mac%20ASan%2064%20Tests%20%281%29/builds/23876

Original issue's description:
> Fix flaky OOPIF tests by waiting for the old RenderFrame to be deleted.
>
> This fixes:
> SitePerProcessBrowserTest.CrossSiteIframe
> SitePerProcessBrowserTest.NavigateRemoteFrame
> SitePerProcessBrowserTest.RFPHDestruction
> TopDocumentIsolationTest.NavigateToSubframeSiteWithPopup2
> TopDocumentIsolationTest.FramesForSitesInHistory
>
> BUG=662550, 629419,  611300 ,  611344 
> TEST=Bots stay green.
>
> Committed: https://crrev.com/7a65c0019399f0a7052ab13bc73439acb116adb8
> Cr-Commit-Position: refs/heads/master@{#430081}

TBR=lukasza@chromium.org,creis@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=662550, 629419,  611300 ,  611344 

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

[modify] https://crrev.com/c6abe5bfc52cc830ee64a6277e19a8ad0fa71e2f/content/browser/site_per_process_browsertest.cc
[modify] https://crrev.com/c6abe5bfc52cc830ee64a6277e19a8ad0fa71e2f/content/browser/top_document_isolation_browsertest.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Nov 7 2016

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

commit b2d5f1f334cfeab18119f38708d0cd191e4eac2f
Author: creis <creis@chromium.org>
Date: Mon Nov 07 23:25:05 2016

Fix more flaky OOPIF tests by waiting for the old RenderFrame to be deleted.

This fixes the following tests in --site-per-process mode:
NavigationControllerBrowserTest.FrameNavigationEntry_NewSubframe
FrameTreeBrowserTest.NavigateGrandchildToBlob

BUG=662550,  646836 
TEST=Bots stay green.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

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

[modify] https://crrev.com/b2d5f1f334cfeab18119f38708d0cd191e4eac2f/content/browser/frame_host/frame_tree_browsertest.cc
[modify] https://crrev.com/b2d5f1f334cfeab18119f38708d0cd191e4eac2f/content/browser/frame_host/navigation_controller_impl_browsertest.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Nov 8 2016

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

commit 29f856890eb1f7c96c3853314a10e782d53cb1eb
Author: creis <creis@chromium.org>
Date: Tue Nov 08 01:52:42 2016

Fix flaky OOPIF tests by waiting for the old RenderFrame to be deleted.

This fixes:
SitePerProcessBrowserTest.CrossSiteIframe
SitePerProcessBrowserTest.NavigateRemoteFrame
SitePerProcessBrowserTest.RFPHDestruction
TopDocumentIsolationTest.NavigateToSubframeSiteWithPopup2
TopDocumentIsolationTest.FramesForSitesInHistory

BUG=662550, 629419,  611300 ,  611344 
TEST=Bots stay green.

Committed: https://crrev.com/7a65c0019399f0a7052ab13bc73439acb116adb8
Review-Url: https://codereview.chromium.org/2478933002
Cr-Original-Commit-Position: refs/heads/master@{#430081}
Cr-Commit-Position: refs/heads/master@{#430474}

[modify] https://crrev.com/29f856890eb1f7c96c3853314a10e782d53cb1eb/content/browser/site_per_process_browsertest.cc
[modify] https://crrev.com/29f856890eb1f7c96c3853314a10e782d53cb1eb/content/browser/top_document_isolation_browsertest.cc

Status: Assigned (was: Untriaged)
Owner: ----

Sign in to add a comment