New issue
Advanced search Search tips

Issue 922954 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 18
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug
Flaky-Test: HeadlessBrowserTest.UserDataDir



Sign in to add a comment

HeadlessBrowserTest.UserDataDir is flaky

Project Member Reported by Findit, Jan 17 (5 days ago)

Issue description

Comment 1 by rockot@google.com, Jan 17 (5 days ago)

Status: Assigned (was: Untriaged)
The test is wrong. Will fix or revert.

Comment 2 by iclell...@chromium.org, Jan 17 (5 days ago)

Labels: -Sheriff-Chromium
Thanks, Ken -- removing from sheriff queue.

Comment 3 by rockot@google.com, Jan 18 (5 days ago)

Cc: jam@chromium.org rockot@google.com
Owner: caseq@chromium.org
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.

Comment 4 by caseq@chromium.org, Jan 18 (4 days ago)

Status: Started (was: Assigned)
Project Member

Comment 5 by bugdroid1@chromium.org, 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

Comment 6 by caseq@chromium.org, Jan 18 (4 days ago)

Status: Fixed (was: Started)
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