Commit 85f072c1 authored by Christoph Schwering's avatar Christoph Schwering Committed by Commit Bot

Revert "Prevent deadlock by checking kRestrictGoogleWebVisibility sooner."

This reverts commit b622082c.

Reason for revert: The CL seems to cause flakiness of
VariationsHttpHeadersBrowserTestWithRestrictedVisibility.TestRestrictGoogleWebVisibilityInThirdPartyContexts, crbug.com/1133291

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}

TBR=jwd@chromium.org,asvitkine@chromium.org,caitlinfischer@google.com

Change-Id: I8cbc093f9bd9d978a13eb881d7dc319309a808fe
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1094303, 1133291
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2438057Reviewed-by: default avatarChristoph Schwering <schwering@google.com>
Commit-Queue: Christoph Schwering <schwering@google.com>
Cr-Commit-Position: refs/heads/master@{#811654}
parent cb1356b9
...@@ -196,8 +196,7 @@ variations::mojom::GoogleWebVisibility GetVisibilityKey( ...@@ -196,8 +196,7 @@ variations::mojom::GoogleWebVisibility GetVisibilityKey(
const network::ResourceRequest& resource_request) { const network::ResourceRequest& resource_request) {
bool use_first_party_visibility = bool use_first_party_visibility =
IsFirstPartyContext(owner, resource_request) && IsFirstPartyContext(owner, resource_request) &&
VariationsIdsProvider::GetInstance() base::FeatureList::IsEnabled(internal::kRestrictGoogleWebVisibility);
->IsRestrictGoogleWebVisibilityEnabled();
return use_first_party_visibility return use_first_party_visibility
? variations::mojom::GoogleWebVisibility::FIRST_PARTY ? variations::mojom::GoogleWebVisibility::FIRST_PARTY
......
...@@ -215,13 +215,8 @@ void VariationsIdsProvider::InitVariationIDsCacheIfNeeded() { ...@@ -215,13 +215,8 @@ void VariationsIdsProvider::InitVariationIDsCacheIfNeeded() {
if (variation_ids_cache_initialized_) if (variation_ids_cache_initialized_)
return; return;
// Must be done before AddObserver() to avoid deadlock in // Register for additional cache updates. This is done first to avoid a race
// OnFieldTrialGroupFinalized(). // that could cause registered FieldTrials to be missed.
is_restrict_google_web_visibility_enabled_ =
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); bool success = base::FieldTrialList::AddObserver(this);
DCHECK(success); DCHECK(success);
...@@ -295,7 +290,8 @@ std::string VariationsIdsProvider::GenerateBase64EncodedProto( ...@@ -295,7 +290,8 @@ std::string VariationsIdsProvider::GenerateBase64EncodedProto(
proto.add_variation_id(entry.first); proto.add_variation_id(entry.first);
break; break;
case GOOGLE_WEB_PROPERTIES_FIRST_PARTY: case GOOGLE_WEB_PROPERTIES_FIRST_PARTY:
if (IsRestrictGoogleWebVisibilityEnabled()) { if (base::FeatureList::IsEnabled(
internal::kRestrictGoogleWebVisibility)) {
if (is_first_party_context) if (is_first_party_context)
proto.add_variation_id(entry.first); proto.add_variation_id(entry.first);
} else { } else {
...@@ -309,7 +305,8 @@ std::string VariationsIdsProvider::GenerateBase64EncodedProto( ...@@ -309,7 +305,8 @@ std::string VariationsIdsProvider::GenerateBase64EncodedProto(
proto.add_trigger_variation_id(entry.first); proto.add_trigger_variation_id(entry.first);
break; break;
case GOOGLE_WEB_PROPERTIES_TRIGGER_FIRST_PARTY: case GOOGLE_WEB_PROPERTIES_TRIGGER_FIRST_PARTY:
if (IsRestrictGoogleWebVisibilityEnabled()) { if (base::FeatureList::IsEnabled(
internal::kRestrictGoogleWebVisibility)) {
if (is_first_party_context) if (is_first_party_context)
proto.add_trigger_variation_id(entry.first); proto.add_trigger_variation_id(entry.first);
} else { } else {
......
...@@ -126,12 +126,6 @@ class VariationsIdsProvider : public base::FieldTrialList::Observer, ...@@ -126,12 +126,6 @@ class VariationsIdsProvider : public base::FieldTrialList::Observer,
// Resets any cached state for tests. // Resets any cached state for tests.
void ResetForTesting(); void ResetForTesting();
// Returns true if kRestrictGoogleWebVisibility is enabled and false
// otherwise.
bool IsRestrictGoogleWebVisibilityEnabled() {
return is_restrict_google_web_visibility_enabled_;
}
private: private:
friend struct base::DefaultSingletonTraits<VariationsIdsProvider>; friend struct base::DefaultSingletonTraits<VariationsIdsProvider>;
...@@ -227,9 +221,6 @@ class VariationsIdsProvider : public base::FieldTrialList::Observer, ...@@ -227,9 +221,6 @@ class VariationsIdsProvider : public base::FieldTrialList::Observer,
// Whether or not we've initialized the caches. // Whether or not we've initialized the caches.
bool variation_ids_cache_initialized_; bool variation_ids_cache_initialized_;
// Denotes whether kRestrictGoogleWebVisibility is enabled.
bool is_restrict_google_web_visibility_enabled_ = false;
// Keep a cache of variation IDs that are transmitted in headers to Google. // Keep a cache of variation IDs that are transmitted in headers to Google.
// This consists of a list of valid IDs, and the actual transmitted header. // This consists of a list of valid IDs, and the actual transmitted header.
std::set<VariationIDEntry> variation_ids_set_; std::set<VariationIDEntry> variation_ids_set_;
......
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