ChromeDriver FetchUrlTest unit test failures in debug builds |
|||||
Issue descriptionAfter the change to "Migrate chrome/test/chromedriver/net/net_util.cc using SimpleURLLoader" (r601478), the following four unit tests started failing in debug builds: FetchUrlTest.ConnectionClose FetchUrlTest.Http200 FetchUrlTest.HttpNon200 FetchUrlTest.NoServer To repro, create a debug build configuration, and then build and run the tests with the following commands: autoninja -C out/Default chromedriver_tests out/Default/chromedriver_tests --gtest_filter=FetchUrlTest.* Here is an example called stack at the point of failure: [77076:77081:1106/184041.036301:640220973814:FATAL:interface_endpoint_client.cc(225)] Check failed: (sequence_checker_).CalledOnValidSequence(). #0 0x7f4190e7dc2d base::debug::StackTrace::StackTrace() #1 0x7f4190b73cfa base::debug::StackTrace::StackTrace() #2 0x7f4190be59db logging::LogMessage::~LogMessage() #3 0x7f419102d89b mojo::InterfaceEndpointClient::Accept() #4 0x7f418a65ac8c network::mojom::URLLoaderFactoryProxy::CreateLoaderAndStart() #5 0x7f418f3ebc92 network::WeakWrapperSharedURLLoaderFactory::CreateLoaderAndStart() #6 0x7f418f3d4ee5 network::(anonymous namespace)::SimpleURLLoaderImpl::StartRequest() #7 0x7f418f3cf633 network::(anonymous namespace)::SimpleURLLoaderImpl::Start() #8 0x7f418f3c9636 network::(anonymous namespace)::SimpleURLLoaderImpl::DownloadToStringOfUnboundedSizeUntilCrashAndDie() #9 0x556e4e301471 (anonymous namespace)::SyncUrlFetcher::FetchOnIOThread() #10 0x556e4e3017cd _ZN4base8internal13FunctorTraitsIMN12_GLOBAL__N_114SyncUrlFetcherEFvvEvE6InvokeIS5_PS3_JEEEvT_OT0_DpOT1_ #11 0x556e4e301714 _ZN4base8internal12InvokeHelperILb0EvE8MakeItSoIMN12_GLOBAL__N_114SyncUrlFetcherEFvvEJPS5_EEEvOT_DpOT0_ #12 0x556e4e3016c5 _ZN4base8internal7InvokerINS0_9BindStateIMN12_GLOBAL__N_114SyncUrlFetcherEFvvEJNS0_17UnretainedWrapperIS4_EEEEEFvvEE7RunImplIS6_NSt3__15tupleIJS8_EEEJLm0EEEEvOT_OT0_NSD_16integer_sequenceImJXspT1_EEEE #13 0x556e4e301609 _ZN4base8internal7InvokerINS0_9BindStateIMN12_GLOBAL__N_114SyncUrlFetcherEFvvEJNS0_17UnretainedWrapperIS4_EEEEEFvvEE7RunOnceEPNS0_13BindStateBaseE #14 0x7f4190b2341e _ZNO4base12OnceCallbackIFvvEE3RunEv #15 0x7f4190b75342 base::debug::TaskAnnotator::RunTask() #16 0x7f4190d9f7f2 base::internal::TaskTracker::RunOrSkipTask() #17 0x7f4190eb80fc base::internal::TaskTrackerPosix::RunOrSkipTask() #18 0x556e4e5aec81 base::test::ScopedTaskEnvironment::TestTaskTracker::RunOrSkipTask() #19 0x7f4190d9d42c base::internal::TaskTracker::RunAndPopNextTask() #20 0x7f4190d828c5 base::internal::SchedulerWorker::RunWorker() #21 0x7f4190d8204c base::internal::SchedulerWorker::RunPooledWorker() #22 0x7f4190d81f6f base::internal::SchedulerWorker::ThreadMain() #23 0x7f4190eb960d base::(anonymous namespace)::ThreadFunc() #24 0x7f418936d494 start_thread #25 0x7f4186486a8f clone
,
Nov 7
,
Nov 7
Will check tomorrow, first thing, johnchen. I assume commit bots do not run these tests and they could not get caught earlier?
,
Nov 7
The commit bots don't run these tests yet, though we're planning to add them some day. There are some other test servers that run these tests, but only in retail builds. These tests only fail in debug builds, so the failure wasn't noticed for a while.
,
Nov 8
,
Nov 9
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ba0104a1ee8d4feb0ce2703bd3013e9851a05e3e commit ba0104a1ee8d4feb0ce2703bd3013e9851a05e3e Author: Antonio Gomes <tonikitoo@igalia.com> Date: Fri Nov 09 03:40:51 2018 ChromeDriver FetchUrlTest unit test failures in debug builds network::mojom::URLLoaderFactory has stricter threading restrictions than its predecessor (URLRequestContextGetter) - see [1]. This CL adapts an existing subset of chromedriver unittests (namely FetchUrlTest), that deals with url fetching, to this threading restrictions. Basically, the CL allows the test to inject the TaskRunner that the URLLoaderFactory handle was created on, and use it afterwards. A similar approach to this one (of injecting a test-specific TaskRunner instance) was also used in [2]. [1] https://crrev.com/c/1292933 [2] https://crrev.com/c/1174655/14/components/sync/engine/net/http_bridge.cc#148 BUG= 902618 Change-Id: If37bfee8cc891c1a95491cfeb3febf5fe673f52e Reviewed-on: https://chromium-review.googlesource.com/c/1327443 Reviewed-by: John Chen <johnchen@chromium.org> Commit-Queue: Antonio Gomes <tonikitoo@igalia.com> Cr-Commit-Position: refs/heads/master@{#606726} [modify] https://crrev.com/ba0104a1ee8d4feb0ce2703bd3013e9851a05e3e/chrome/test/chromedriver/net/net_util.cc [modify] https://crrev.com/ba0104a1ee8d4feb0ce2703bd3013e9851a05e3e/chrome/test/chromedriver/net/net_util.h [modify] https://crrev.com/ba0104a1ee8d4feb0ce2703bd3013e9851a05e3e/chrome/test/chromedriver/net/net_util_unittest.cc
,
Nov 9
Thanks for the fix! |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by johnchen@chromium.org
, Nov 7Owner: toniki...@igalia.com
Status: Assigned (was: Untriaged)