New issue
Advanced search Search tips

Issue 739773 link

Starred by 5 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug



Sign in to add a comment

navigation: BrowserTest.CancelBeforeUnloadResetsURL hangs

Project Member Reported by ellyjo...@chromium.org, Jul 6 2017

Issue description

This test seems to have hung on the Linux Tests bot:

https://uberchromegw.corp.google.com/i/chromium.linux/builders/Linux%20Tests/builds/59071

test stdout is:

[ RUN      ] BrowserTest.CancelBeforeUnloadResetsURL
Xlib:  extension "RANDR" missing on display ":99".
[30286:30286:0706/093041.207109:WARNING:password_store_factory.cc(261)] Using basic (unencrypted) store for password storage. See https://chromium.googlesource.com/chromium/src/+/master/docs/linux_password_storage.md for more information about password storage options.
BrowserTestBase received signal: Terminated. Backtrace:
#0 0x0000025c6877 base::debug::StackTrace::StackTrace()
#1 0x000002af1a41 content::(anonymous namespace)::DumpStackTraceSignalHandler()
#2 0x7f20dbd71cb0 <unknown>
#3 0x7f20dbe2bc5d __poll
#4 0x7f20dcd4afe4 <unknown>
#5 0x7f20dcd4b0ec g_main_context_iteration
#6 0x0000025dfbb6 base::MessagePumpGlib::Run()
#7 0x0000025fd210 base::RunLoop::Run()
#8 0x000002b1f57c content::RunMessageLoop()
#9 0x00000266b633 InProcessBrowserTest::QuitBrowsers()
#10 0x00000266b4ef InProcessBrowserTest::PostRunTestOnMainThread()
#11 0x000002af17c2 content::BrowserTestBase::ProxyRunTestOnMainThreadLoop()
#12 0x000002958da7 ChromeBrowserMainParts::PreMainMessageLoopRunImpl()
#13 0x000002957bba ChromeBrowserMainParts::PreMainMessageLoopRun()
#14 0x00000177c3ed content::BrowserMainLoop::PreMainMessageLoopRun()
#15 0x000001a76297 content::StartupTaskRunner::RunAllTasksNow()
#16 0x00000177a7cd content::BrowserMainLoop::CreateStartupTasks()
#17 0x00000177edac content::BrowserMainRunnerImpl::Initialize()
#18 0x000001778002 content::BrowserMain()
#19 0x0000025b12cd content::ContentMainRunnerImpl::Run()
#20 0x000003bc5aa2 service_manager::Main()
#21 0x0000025aff72 content::ContentMain()
#22 0x000002af1493 content::BrowserTestBase::SetUp()
#23 0x000002669ad5 InProcessBrowserTest::SetUp()
#24 0x00000129dfbe testing::Test::Run()
#25 0x00000129eae0 testing::TestInfo::Run()
#26 0x00000129efc7 testing::TestCase::Run()
#27 0x0000012a5227 testing::internal::UnitTestImpl::RunAllTests()
#28 0x0000012a4eb3 testing::UnitTest::Run()
#29 0x0000026793a4 base::TestSuite::Run()
#30 0x0000025bb8d9 ChromeTestSuiteRunner::RunTestSuite()
#31 0x000002b1c7fb content::LaunchTests()
#32 0x0000025bb847 main
#33 0x7f20dbd5cf45 __libc_start_main
#34 0x000000555150 <unknown>

I don't see any obvious culprit change. Tagging for net triage.
 
What makes you think this is a net issue?  That test looks to use a modal Javascript dialog, and test navigation features, as opposed to any unusual case from a network standpoint.
Components: -Internals>Network UI>Browser>Navigation
Tossing into the navigation bin, per above comment.
Labels: Sheriff-Chromium
Owner: toyoshim@chromium.org
Status: Assigned (was: Available)
This is broken again: https://build.chromium.org/p/chromium.linux/builders/Linux%20Tests/builds/63523

Happened three times in a row, so probably a reliable failure. toyoshim@, could this change of yours https://chromium-review.googlesource.com/725026 which landed in the first failing batch be the issue?
Cc: toyoshim@chromium.org
Owner: creis@chromium.org
My change should not affect pages that do not use service worker, and after the three failures, it continues passing.

Assigning creis@ who added the test, and is a navigation expert.

Comment 6 by creis@chromium.org, Oct 19 2017

Cc: a...@chromium.org
Looks like it has been flaky for a while.  (The test is ancient, from r83388 in 2011.  Not sure how long it has been flaky.)  

https://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=BrowserTest.CancelBeforeUnloadResetsURL

Many of the failures are in site_per_process_browser_tests (which just runs browser_tests with --site-per-process), but I see failures in normal browser_tests as well (e.g., https://build.chromium.org/p/chromium.linux/builders/Linux%20Tests/builds/63522).

I was able to repro locally on Linux after a couple runs, where it looks like the test is hanging after the test is over.  I'll take a closer look.  (Maybe it's related to the AppModalDialog?)

Comment 7 by battre@chromium.org, Oct 23 2017

Labels: -Sheriff-Chromium
Assigned to owner who offered to take a closer look. --> Taking out of sheriff queue.

Comment 8 by creis@chromium.org, Nov 2 2017

Status: Started (was: Assigned)
CL started here: https://chromium-review.googlesource.com/c/chromium/src/+/750330

I'm moving the test over to content/ where we can control the dialog a bit more, and trying to avoid one place where the test waits for an ACK.
Project Member

Comment 9 by bugdroid1@chromium.org, Nov 3 2017

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

commit 15d60b1371a8e405891f25a3db2b2ae0a004b034
Author: Charles Reis <creis@chromium.org>
Date: Fri Nov 03 17:43:47 2017

Rewrite flaky CancelBeforeUnloadResetsURL test and update fix.

We can invalidate the URL in the address bar without waiting for the
beforeunload ACK from the renderer, making the test easier to write.
We're keeping the invalidation in DidCancelLoading after the ACK as
well, since it's not clear whether that's safe to remove.

Also, the test can be implemented in content/ rather than chrome/.

BUG= 739773 
TEST=No behavior change, new test stays green.

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Change-Id: I66450965b5a8ac517211194590f47044d6ab50d3
Reviewed-on: https://chromium-review.googlesource.com/750330
Reviewed-by: Nate Chapin <japhet@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Charlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513826}
[modify] https://crrev.com/15d60b1371a8e405891f25a3db2b2ae0a004b034/chrome/browser/ui/browser_browsertest.cc
[modify] https://crrev.com/15d60b1371a8e405891f25a3db2b2ae0a004b034/content/browser/frame_host/render_frame_host_delegate.h
[modify] https://crrev.com/15d60b1371a8e405891f25a3db2b2ae0a004b034/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/15d60b1371a8e405891f25a3db2b2ae0a004b034/content/browser/frame_host/render_frame_host_impl_browsertest.cc
[modify] https://crrev.com/15d60b1371a8e405891f25a3db2b2ae0a004b034/content/browser/renderer_host/render_view_host_delegate.h
[modify] https://crrev.com/15d60b1371a8e405891f25a3db2b2ae0a004b034/content/browser/web_contents/web_contents_impl.cc
[modify] https://crrev.com/15d60b1371a8e405891f25a3db2b2ae0a004b034/content/browser/web_contents/web_contents_impl.h

Status: Fixed (was: Started)
Seems to be green now.

Sign in to add a comment