Issue metadata
Sign in to add a comment
|
browser_side_navigation_content_browsertests is failing (WebContentsImplBrowserTest.DismissingBeforeUnloadDialogInvalidatesUrl) |
||||||||||||||||||||
Issue descriptionbrowser_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
,
Jun 20 2017
,
Jun 20 2017
,
Jun 20 2017
This has been reverted in https://chromium.googlesource.com/chromium/src/+/15f5b5b2ef86151279bbad41a0976375b9f40977
,
Jun 20 2017
,
Jun 20 2017
,
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.
,
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.
,
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.
,
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.
,
Sep 26
|
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by pdr@chromium.org
, Jun 20 2017