Issue metadata
Sign in to add a comment
|
HeadlessBrowserTest.UserDataDir is flaky |
||||||||||||||||||||||
Issue descriptionFindit identified the culprit r623535 as introducing flaky test(s) summarized in https://findit-for-me.appspot.com/waterfall/flake/flake-culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyQwsSDEZsYWtlQ3VscHJpdCIxY2hyb21pdW0vNDcxYWE3OTRiYmI5ZTNhOTg1NDJlZTIxMzVlYTYxZGRmNGJmODdhMww Please revert the culprit or disable the test(s) asap. If you are the owner, please fix! If the culprit above is wrong, please file a bug using this link: https://bugs.chromium.org/p/chromium/issues/entry?status=Unconfirmed&labels=Pri-1,Test-Findit-Wrong&components=Tools%3ETest%3EFindit%3EFlakiness&summary=%5BFindit%5D%20Flake%20Analyzer%20-%20Wrong%20culprit%20r623535&comment=Link%20to%20Culprit%3A%20https://findit-for-me.appspot.com/waterfall/flake/flake-culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyQwsSDEZsYWtlQ3VscHJpdCIxY2hyb21pdW0vNDcxYWE3OTRiYmI5ZTNhOTg1NDJlZTIxMzVlYTYxZGRmNGJmODdhMww Automatically posted by the findit-for-me app (https://goo.gl/Ot9f7N).
,
Jan 17
(5 days ago)
Thanks, Ken -- removing from sheriff queue.
,
Jan 18
(5 days ago)
Actually it doesn't look like there's anything wrong with the test specifically, but there's instead some raciness in how headless initializes its network context. Here's the crash: 17536:17546:0117/010350.177650:27454745734:FATAL:network_context.cc(1685)] Check failed: network_service_->os_crypt_config_set(). NetworkService::SetCryptConfig must be called before creating a NetworkContext with encrypted cookies. #0 0x0000f75565b5 base::debug::StackTrace::StackTrace() #1 0x0000f72b2fbe base::debug::StackTrace::StackTrace() #2 0x0000f7309113 logging::LogMessage::~LogMessage() #3 0x0000e53fe3e1 network::NetworkContext::ApplyContextParamsToBuilder() #4 0x0000e53fdf70 network::NetworkContext::NetworkContext() #5 0x0000e5457a3c std::__Cr::make_unique<>() #6 0x0000e5452970 network::NetworkService::CreateNetworkContextWithBuilder() #7 0x0000f5efd4c3 headless::HeadlessRequestContextManager::InitializeOnIO() #8 0x0000f5f06a17 base::internal::FunctorTraits<>::Invoke<>() ... It looks like what happens is: 1. HeadlessRequestContextManager's constructor calls MaybeSetUpOSCrypt -> SetCryptConfig() on content::GetNetworkService(), i.e. over the NetworkService mojo interface 2. Later, HeadlessRequestContextManager::Initialize posts a task to the IO thread (InitializeOnIO) which -- when Network Service is disabled -- constructs a new NetworkContext by talking directly to content::GetNetworkServiceImpl(), i.e. it does NOT go over the NetworkService mojo interface This means it's possible for the NetworkContext to be constructed before SetCryptConfig is dispatched to the NetworkServiceImpl. In order to fix this, either #1 needs to call into NetworkServiceImpl directly when the network service is disabled, or #2 needs to go over the mojom interface. Tentatively assigning to caseq@ as author of cookie encryption support in headless.
,
Jan 18
(4 days ago)
,
Jan 18
(4 days ago)
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d1e6fc20abea7450938a27f940db799e197d159c commit d1e6fc20abea7450938a27f940db799e197d159c Author: Andrey Kosyakov <caseq@chromium.org> Date: Fri Jan 18 23:11:15 2019 Headless: fix a race when initializing OSCrypt When running with network service disabled, initialize in on IO thread. With network service, initialize it via mojo interface rather than directly. Bug: 922954 Change-Id: Ifad7080ba27e1b83b895620e054008b164a9fd81 Reviewed-on: https://chromium-review.googlesource.com/c/1422846 Reviewed-by: Dmitry Gozman <dgozman@chromium.org> Reviewed-by: Ken Rockot <rockot@google.com> Commit-Queue: Andrey Kosyakov <caseq@chromium.org> Cr-Commit-Position: refs/heads/master@{#624344} [modify] https://crrev.com/d1e6fc20abea7450938a27f940db799e197d159c/headless/lib/browser/headless_request_context_manager.cc [modify] https://crrev.com/d1e6fc20abea7450938a27f940db799e197d159c/headless/lib/browser/headless_request_context_manager.h
,
Jan 18
(4 days ago)
Thanks a lot for the analysis, Ken! I couldn't reproduce the race locally, but hopefully the commit above fixes it. |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by rockot@google.com
, Jan 17 (5 days ago)