Commit 4260dcdd authored by Caitlin Fischer's avatar Caitlin Fischer Committed by Commit Bot

Reland "Prevent deadlock by checking kRestrictGoogleWebVisibility..."

The original CL for this change stored the value of the feature in
VariationIdsProvider. However, the provider lives only in the browser
process. Thus, when the renderer process checks the value of the
feature via the provider in variations_http_headers.cc, the value is
always seen as false.

The fix is to check the FeatureList, which lives in both processes.

I locally reproduced the failure using the failing builders gn args:
https://ci.chromium.org/p/chromium/builders/ci/Linux%20Tests%20(dbg)(1)/91893?

I confirm that the fix passes with these gn args, too.

This is a reland of b622082c

Original change's description:
> Prevent deadlock by checking kRestrictGoogleWebVisibility sooner.
>
> More specifically, check the feature's state after field trials have
> been created from the variations seed and before the variations
> header is updated.
>
> Checking the state for the first time in
> VariationsIdsProvider::GenerateBase64EncodedProto() could cause a
> deadlock.
>
> Bug: 1094303
> Change-Id: I53fe3446e3a4567d4b9122b54642b5744eb8b46e
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2431556
> Commit-Queue: Caitlin Fischer <caitlinfischer@google.com>
> Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
> Reviewed-by: Jesse Doherty <jwd@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#811566}

Bug: 1094303
Change-Id: I7f73c1cb089635d4084b37646307561d235b6a25
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2436766
Commit-Queue: Caitlin Fischer <caitlinfischer@google.com>
Reviewed-by: default avatarAlexei Svitkine <asvitkine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#811965}
parent 6fbfb08f
......@@ -215,8 +215,15 @@ void VariationsIdsProvider::InitVariationIDsCacheIfNeeded() {
if (variation_ids_cache_initialized_)
return;
// Register for additional cache updates. This is done first to avoid a race
// that could cause registered FieldTrials to be missed.
// Query the kRestrictGoogleWebVisibility feature to activate the
// associated field trial, if any, so that querying it in
// OnFieldTrialGroupFinalized() does not result in deadlock.
//
// Note: Must be done before the AddObserver() call below.
base::FeatureList::IsEnabled(internal::kRestrictGoogleWebVisibility);
// Register for additional cache updates. This is done before initializing the
// cache to avoid a race that could cause registered FieldTrials to be missed.
bool success = base::FieldTrialList::AddObserver(this);
DCHECK(success);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment