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

Issue 765107 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Out until 24 Jan
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug



Sign in to add a comment

PlzNav: NavigationControllerBrowserTest.EnsureFrameNavigationEntriesClearedOnMismatch{NoSrc} failing on chromium.win/Win10 Tests x64

Project Member Reported by dullweber@chromium.org, Sep 14 2017

Issue description

browser_side_navigation_content_browsertests failing on chromium.win/Win10 Tests x64

Builders failed on: 
- Win10 Tests x64: 
  https://build.chromium.org/p/chromium.win/builders/Win10%20Tests%20x64

NavigationControllerBrowserTest.EnsureFrameNavigationEntriesClearedOnMismatch is flaky on Win10 browser_side_navigation_content_browsertests

https://build.chromium.org/p/chromium.win/builders/Win10%20Tests%20x64/builds/15903
https://build.chromium.org/p/chromium.win/builders/Win10%20Tests%20x64/builds/15886
https://build.chromium.org/p/chromium.win/builders/Win10%20Tests%20x64/builds/15879

[ RUN      ] NavigationControllerBrowserTest.EnsureFrameNavigationEntriesClearedOnMismatch
DevTools listening on ws://127.0.0.1:49420/devtools/browser/2ead5295-8148-4b9e-b0bc-9465c847bbe3
../../content/browser/frame_host/navigation_controller_impl_browsertest.cc(5916): error:       Expected: 2U
      Which is: 2
To be equal to: entry->root_node()->children[0]->children.size()
      Which is: 0
../../content/browser/frame_host/navigation_controller_impl_browsertest.cc(5930): error:       Expected: 2U
      Which is: 2
To be equal to: entry->root_node()->children[0]->children.size()
      Which is: 0
[  FAILED  ] NavigationControllerBrowserTest.EnsureFrameNavigationEntriesClearedOnMismatch, where TypeParam =  and GetParam() =  (566 ms)
[ RUN      ] NavigationControllerBrowserTest.EnsureFrameNavigationEntriesClearedOnMismatch
DevTools listening on ws://127.0.0.1:54792/devtools/browser/9899894b-df8b-45c3-9e78-f83fcf26eae6
../../content/browser/frame_host/navigation_controller_impl_browsertest.cc(5916): error:       Expected: 2U
      Which is: 2
To be equal to: entry->root_node()->children[0]->children.size()
      Which is: 0
../../content/browser/frame_host/navigation_controller_impl_browsertest.cc(5930): error:       Expected: 2U
      Which is: 2
To be equal to: entry->root_node()->children[0]->children.size()
      Which is: 0
[  FAILED  ] NavigationControllerBrowserTest.EnsureFrameNavigationEntriesClearedOnMismatch, where TypeParam =  and GetParam() =  (460 ms)

 
Project Member

Comment 1 by bugdroid1@chromium.org, Sep 14 2017

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

commit c8b3233507d30167dc57e6a9fab2887b121b7d68
Author: Christian Dullweber <dullweber@chromium.org>
Date: Thu Sep 14 10:11:29 2017

Disable EnsureFrameNavigationEntriesClearedOnMismatchNoSrc on Win

NavigationControllerBrowserTest.EnsureFrameNavigationEntriesClearedOnMismatch
is flaky on Win10 browser_side_navigation_content_browsertests.

TBR: nasko@chromium.org
Bug:  765107 
Change-Id: I37491a6ca08903ffe9fa325947107182f0a5cc77
Reviewed-on: https://chromium-review.googlesource.com/666817
Reviewed-by: Christian Dullweber <dullweber@chromium.org>
Commit-Queue: Christian Dullweber <dullweber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501915}
[modify] https://crrev.com/c8b3233507d30167dc57e6a9fab2887b121b7d68/content/browser/frame_host/navigation_controller_impl_browsertest.cc

Comment 2 by creis@chromium.org, Sep 14 2017

Cc: creis@chromium.org jam@chromium.org arthurso...@chromium.org nasko@chromium.org
Labels: Proj-PlzNavigate M-63 Pri-1
Owner: clamy@chromium.org
This is only failing in PlzNavigate, apparently.  (See the target: browser_side_navigation_content_browsertests)  It does appear to be flaky in that mode.

We should re-enable this test in general, and temporarily disable it in PlzNavigate until we find how to fix the flakiness.  (The recent flakiness is a concern-- it's been enabled for both PlzNav and non-PlzNav for a year, so that might suggest that one of the recent PlzNavigate fixes made it flaky.)

Comment 3 by creis@chromium.org, Sep 14 2017

Components: UI>Browser>Navigation
The flaky failures go back to at least https://build.chromium.org/p/chromium.win/builders/Win10%20Tests%20x64/builds/15736, which was on September 9.  I'm not sure if they go back further.  That means the flakes pre-date the fixes for  issue 763106 ,  issue 762945 , and issue 763448, which is good (since those have been merged to M61).

For reference, this test was added in  issue 628677 .

Comment 4 by jam@chromium.org, Sep 14 2017

In the last 200 builds, the earliest flake I see there is
https://build.chromium.org/p/chromium.win/builders/Win10%20Tests%20x64/builds/15736 which is at r500795


 bug 762945  (previous XHRs being cancelled on navigation failure) is at r501427
bug 763448 (webRequest) is at r500969
 bug 763106  (OWA) is at r500995

Since we don't know when it started flaking, it could have been for a while. But thought I'd give the above data to show it's not recent fixes.

Comment 5 by jam@chromium.org, Sep 14 2017

ah looks like our nearly identical messages crossed :)

Comment 6 by creis@chromium.org, Sep 14 2017

Note: this test was about a fix that prevented RFH_INVALID_ORIGIN_ON_COMMIT renderer kills.  It's possible that the flakiness is indicative of a real regression in the wild, since I just got a Finch Chirp about an increase in those kills on stable starting in September:

https://uma.googleplex.com/timeline_v2?sid=3e09494b7976b764ded816c28a2f16a5
Labels: -Sheriff-Chromium
FWIW I am not able to repro at r503452, on Linux with --gtest_repeat=50 :-(
I can't reproduce it on Linux (repeat=100) either.
Owner: nasko@chromium.org
Status: Started (was: Assigned)
Summary: PlzNav: NavigationControllerBrowserTest.EnsureFrameNavigationEntriesClearedOnMismatch{NoSrc} failing on chromium.win/Win10 Tests x64 (was: browser_side_navigation_content_browsertests failing on chromium.win/Win10 Tests x64)
Nasko found that this is due to a bug in the tests, not the code.  Both disabled tests (EnsureFrameNavigationEntriesClearedOnMismatch and NavigationControllerBrowserTest.EnsureFrameNavigationEntriesClearedOnMismatchNoSrc) assume that the tree of FrameNavigationEntries has children in the same order as the FrameTreeNode tree.

That's not true, because FrameTreeNodes are added in the order that the page creates them, but FrameNavigationEntries are added in the order that they commit (which is non-deterministic).  PlzNav must have exacerbated the race.

He's got a fix in progress that will re-enable the tests.
 Issue 766378  has been merged into this issue.
Project Member

Comment 12 by bugdroid1@chromium.org, Oct 6 2017

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

commit f6051161237b285ceb6ba797eb37db3a8af1ee48
Author: Nasko Oskov <nasko@chromium.org>
Date: Fri Oct 06 21:15:39 2017

Enable EnsureFrameNavigationEntriesClearedOnMismatchNoSrc test

Re-enable this test, since it currently passes on Windows locally with
100-200 iterations.

Bug:  765107 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Change-Id: I2c77790b1ecdfd872204525f8d7dfd47ba168f71
Reviewed-on: https://chromium-review.googlesource.com/705271
Reviewed-by: Charlie Reis <creis@chromium.org>
Commit-Queue: Nasko Oskov <nasko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#507179}
[modify] https://crrev.com/f6051161237b285ceb6ba797eb37db3a8af1ee48/content/browser/frame_host/navigation_controller_impl_browsertest.cc

Project Member

Comment 13 by bugdroid1@chromium.org, Oct 6 2017

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

commit e64275931efbe51db8c9ef93116cebe56e18f214
Author: Nasko Oskov <nasko@chromium.org>
Date: Fri Oct 06 23:47:43 2017

Fix flakiness of EnsureFrameNavigationEntriesClearedOnMismatch test.

The test was assuming ordering the tree of FrameNavigationEntries, which
is not guaranteed, since each frame can commit in arbitrary order. This CL
changes the test to find explicitly the node it needs to verify correctness.

Bug:  765107 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Change-Id: I5f220d6465342b11ea32acb532308f00f6af2136
Reviewed-on: https://chromium-review.googlesource.com/702728
Commit-Queue: Nasko Oskov <nasko@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#507230}
[modify] https://crrev.com/e64275931efbe51db8c9ef93116cebe56e18f214/content/browser/frame_host/navigation_controller_impl_browsertest.cc
[modify] https://crrev.com/e64275931efbe51db8c9ef93116cebe56e18f214/content/browser/frame_host/navigation_entry_impl.cc
[modify] https://crrev.com/e64275931efbe51db8c9ef93116cebe56e18f214/content/browser/frame_host/navigation_entry_impl.h

Status: Fixed (was: Started)
I think this is fixed after r507179 and r507230.  Thanks!

Sign in to add a comment