New issue
Advanced search Search tips

Issue 734835 link

Starred by 2 users

Issue metadata

Status: Assigned
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug-Regression



Sign in to add a comment

browser_side_navigation_content_browsertests is failing (WebContentsImplBrowserTest.DismissingBeforeUnloadDialogInvalidatesUrl)

Project Member Reported by pdr@chromium.org, Jun 20 2017

Issue description

browser_side_navigation_content_browsertests is failing (WebContentsImplBrowserTest.DismissingBeforeUnloadDialogInvalidatesUrl)

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


The following test is failing:
WebContentsImplBrowserTest.DismissingBeforeUnloadDialogInvalidatesUrl

Link to build:
https://uberchromegw.corp.google.com/i/chromium.win/builders/Win10%20Tests%20x64/builds/12971

I think this is a regression from:
https://chromium.googlesource.com/chromium/src/+/bcce9cb42a1578ba95ead8b64e66101a8ae5e40b
 

Comment 1 by pdr@chromium.org, Jun 20 2017

Summary: browser_side_navigation_content_browsertests failing (was: browser_side_navigation_content_browsertests failing on chromium.win/Win10 Tests x64)
Here's another failing bot: https://uberchromegw.corp.google.com/i/chromium.linux/builders/Linux%20Tests/builds/58277

I'm going to go ahead and roll this out.

Comment 2 by pdr@chromium.org, Jun 20 2017

Description: Show this description

Comment 3 by pdr@chromium.org, Jun 20 2017

Summary: browser_side_navigation_content_browsertests is failing (WebContentsImplBrowserTest.DismissingBeforeUnloadDialogInvalidatesUrl) (was: browser_side_navigation_content_browsertests failing)

Comment 5 by hayato@chromium.org, Jun 20 2017

Labels: -Sheriff-Chromium
Cc: tmartino@chromium.org
 Issue 734862  has been merged into this issue.

Comment 7 by nick@chromium.org, Jun 20 2017

Thanks for this report and for rolling back the change.

It seems the test is hanging, but flakily. I have been able to repro at least once locally.

Comment 8 by nick@chromium.org, Jun 21 2017

Don't understand the problem yet, but I believe I understand the source of the flakiness: it seems the test hangs in the case where, when we initiate the second navigation, is_waiting_for_beforeunload_ack_ is still true from the first navigation.

Comment 9 by nick@chromium.org, Jun 21 2017

I reverted everything but the new-test part of my change, and can still repro the intermittent hang. This suggests the behavior here is preexisting.

Still digging to determine if it's a test bug or a functional bug. RenderFrameHostImpl::DispatchBeforeUnload definitely seems to deliberately drop the dispatch operation on the floor here, but this intermediate state is weird, all the following are simultaneously true:
  - The first load has stopped already (from the perspective of content::WaitForLoadStop)
 - The omnibox state has reset to the original URL, as the pending entry has been discarded.
 - BUT we're still waiting for an beforeunload ACK to report back the "proceed=false" information (even though proceed=false came from an earlier dialog result in the browser process).

We could paper over it by waiting longer (until is_waiting_for_beforeunload_ack_ transitions back to false) but before doing that I want to prove there's not a more serious bug lurking underneath.

Comment 10 by nick@chromium.org, Jun 21 2017

The hang seems to go away if I change my implementation of RunBeforeUnloadDialog to run the cancellation callback immediately, instead of doing a PostTask. This makes sense, since every PostTask operation provides an opportunity for nondeterminism. I don't actually see a bug here, just my own confusion, so this is the approach I'll go with.
Owner: ----

Sign in to add a comment