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}

components/variations/net/variations_http_headers.cc

@@ -196,7 +196,8 @@ variations::mojom::GoogleWebVisibility GetVisibilityKey(
     const network::ResourceRequest& resource_request) {
   bool use_first_party_visibility =
       IsFirstPartyContext(owner, resource_request) &&
-      base::FeatureList::IsEnabled(internal::kRestrictGoogleWebVisibility);
+      VariationsIdsProvider::GetInstance()
+          ->IsRestrictGoogleWebVisibilityEnabled();
 
   return use_first_party_visibility
              ? variations::mojom::GoogleWebVisibility::FIRST_PARTY

components/variations/variations_ids_provider.cc

@@ -215,8 +215,13 @@ 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.
+  // Must be done before AddObserver() to avoid deadlock in
+  // OnFieldTrialGroupFinalized().
+  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);
   DCHECK(success);
 
@@ -290,8 +295,7 @@ std::string VariationsIdsProvider::GenerateBase64EncodedProto(
         proto.add_variation_id(entry.first);
         break;
       case GOOGLE_WEB_PROPERTIES_FIRST_PARTY:
-        if (base::FeatureList::IsEnabled(
-                internal::kRestrictGoogleWebVisibility)) {
+        if (IsRestrictGoogleWebVisibilityEnabled()) {
           if (is_first_party_context)
             proto.add_variation_id(entry.first);
         } else {
@@ -305,8 +309,7 @@ std::string VariationsIdsProvider::GenerateBase64EncodedProto(
         proto.add_trigger_variation_id(entry.first);
         break;
       case GOOGLE_WEB_PROPERTIES_TRIGGER_FIRST_PARTY:
-        if (base::FeatureList::IsEnabled(
-                internal::kRestrictGoogleWebVisibility)) {
+        if (IsRestrictGoogleWebVisibilityEnabled()) {
           if (is_first_party_context)
             proto.add_trigger_variation_id(entry.first);
         } else {

components/variations/variations_ids_provider.h

@@ -126,6 +126,12 @@ class VariationsIdsProvider : public base::FieldTrialList::Observer,
   // Resets any cached state for tests.
   void ResetForTesting();
 
+  // Returns true if kRestrictGoogleWebVisibility is enabled and false
+  // otherwise.
+  bool IsRestrictGoogleWebVisibilityEnabled() {
+    return is_restrict_google_web_visibility_enabled_;
+  }
+
  private:
   friend struct base::DefaultSingletonTraits<VariationsIdsProvider>;
 
@@ -221,6 +227,9 @@ class VariationsIdsProvider : public base::FieldTrialList::Observer,
   // Whether or not we've initialized the caches.
   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.
   // This consists of a list of valid IDs, and the actual transmitted header.
   std::set<VariationIDEntry> variation_ids_set_;