Move the Chrome Variations HTTP header helper file into...

Move the Chrome Variations HTTP header helper file into c/b/metrics/variations, and fix up lock behavior a bit.

Specifically:
  (1) Reduces the duration for which the lock is held when appending HTTP headers.
  (2) Adds a debug assertion that the lock is correctly held when calling UpdateVariationIDsHeaderValue().

BUG=none

Review URL: https://chromiumcodereview.appspot.com/11821019

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@175967 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/autocomplete/search_provider.cc

@@ -26,10 +26,10 @@
 #include "chrome/browser/autocomplete/history_url_provider.h"
 #include "chrome/browser/autocomplete/keyword_provider.h"
 #include "chrome/browser/autocomplete/url_prefix.h"
-#include "chrome/browser/chrome_metrics_helper.h"
 #include "chrome/browser/history/history.h"
 #include "chrome/browser/history/history_service_factory.h"
 #include "chrome/browser/history/in_memory_database.h"
+#include "chrome/browser/metrics/variations/variations_http_header_provider.h"
 #include "chrome/browser/net/url_fixer_upper.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/profiles/profile.h"
@@ -654,7 +654,7 @@ net::URLFetcher* SearchProvider::CreateSuggestFetcher(
   fetcher->SetLoadFlags(net::LOAD_DO_NOT_SAVE_COOKIES);
   // Add Chrome experiment state to the request headers.
   net::HttpRequestHeaders headers;
-  ChromeMetricsHelper::GetInstance()->AppendHeaders(
+  chrome_variations::VariationsHttpHeaderProvider::GetInstance()->AppendHeaders(
       fetcher->GetOriginalURL(), profile_->IsOffTheRecord(), false, &headers);
   fetcher->SetExtraRequestHeaders(headers.ToString());
   fetcher->Start();

chrome/browser/chrome_metrics_helper.cc → chrome/browser/metrics/variations/variations_http_header_provider.cc

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/chrome_metrics_helper.h"
+#include "chrome/browser/metrics/variations/variations_http_header_provider.h"
 
 #include "base/base64.h"
 #include "base/memory/singleton.h"
@@ -16,14 +16,17 @@
 
 using content::BrowserThread;
 
-ChromeMetricsHelper* ChromeMetricsHelper::GetInstance() {
-  return Singleton<ChromeMetricsHelper>::get();
+namespace chrome_variations {
+
+VariationsHttpHeaderProvider* VariationsHttpHeaderProvider::GetInstance() {
+  return Singleton<VariationsHttpHeaderProvider>::get();
 }
 
-void ChromeMetricsHelper::AppendHeaders(const GURL& url,
-                                        bool incognito,
-                                        bool uma_enabled,
-                                        net::HttpRequestHeaders* headers) {
+void VariationsHttpHeaderProvider::AppendHeaders(
+    const GURL& url,
+    bool incognito,
+    bool uma_enabled,
+    net::HttpRequestHeaders* headers) {
   // Note the criteria for attaching Chrome experiment headers:
   // 1. We only transmit to *.google.<TLD> domains. NOTE that this use of
   //    google_util helpers to check this does not guarantee that the URL is
@@ -46,32 +49,40 @@ void ChromeMetricsHelper::AppendHeaders(const GURL& url,
   // Lazily initialize the header, if not already done, before attempting to
   // transmit it.
   InitVariationIDsCacheIfNeeded();
-  base::AutoLock scoped_lock(lock_);
-  if (!variation_ids_header_.empty())
-    headers->SetHeaderIfMissing("X-Chrome-Variations", variation_ids_header_);
+
+  std::string variation_ids_header_copy;
+  {
+    base::AutoLock scoped_lock(lock_);
+    variation_ids_header_copy = variation_ids_header_;
+  }
+
+  if (!variation_ids_header_copy.empty()) {
+    headers->SetHeaderIfMissing("X-Chrome-Variations",
+                                variation_ids_header_copy);
+  }
 }
 
-ChromeMetricsHelper::ChromeMetricsHelper()
+VariationsHttpHeaderProvider::VariationsHttpHeaderProvider()
     : variation_ids_cache_initialized_(false) {
 }
 
-ChromeMetricsHelper::~ChromeMetricsHelper() {
+VariationsHttpHeaderProvider::~VariationsHttpHeaderProvider() {
 }
 
-void ChromeMetricsHelper::OnFieldTrialGroupFinalized(
+void VariationsHttpHeaderProvider::OnFieldTrialGroupFinalized(
     const std::string& trial_name,
     const std::string& group_name) {
-  chrome_variations::VariationID new_id =
-      chrome_variations::GetGoogleVariationID(
-          chrome_variations::GOOGLE_WEB_PROPERTIES, trial_name, group_name);
-  if (new_id == chrome_variations::kEmptyID)
+  VariationID new_id =
+      GetGoogleVariationID(GOOGLE_WEB_PROPERTIES, trial_name, group_name);
+  if (new_id == kEmptyID)
     return;
+
   base::AutoLock scoped_lock(lock_);
   variation_ids_set_.insert(new_id);
   UpdateVariationIDsHeaderValue();
 }
 
-void ChromeMetricsHelper::InitVariationIDsCacheIfNeeded() {
+void VariationsHttpHeaderProvider::InitVariationIDsCacheIfNeeded() {
   base::AutoLock scoped_lock(lock_);
   if (variation_ids_cache_initialized_)
     return;
@@ -84,12 +95,12 @@ void ChromeMetricsHelper::InitVariationIDsCacheIfNeeded() {
   base::FieldTrial::ActiveGroups initial_groups;
   base::FieldTrialList::GetActiveFieldTrialGroups(&initial_groups);
   for (base::FieldTrial::ActiveGroups::const_iterator it =
-       initial_groups.begin(); it != initial_groups.end(); ++it) {
-    const chrome_variations::VariationID id =
-        chrome_variations::GetGoogleVariationID(
-            chrome_variations::GOOGLE_WEB_PROPERTIES, it->trial_name,
-            it->group_name);
-    if (id != chrome_variations::kEmptyID)
+           initial_groups.begin();
+       it != initial_groups.end(); ++it) {
+    const VariationID id =
+        GetGoogleVariationID(GOOGLE_WEB_PROPERTIES, it->trial_name,
+                             it->group_name);
+    if (id != kEmptyID)
       variation_ids_set_.insert(id);
   }
   UpdateVariationIDsHeaderValue();
@@ -97,7 +108,9 @@ void ChromeMetricsHelper::InitVariationIDsCacheIfNeeded() {
   variation_ids_cache_initialized_ = true;
 }
 
-void ChromeMetricsHelper::UpdateVariationIDsHeaderValue() {
+void VariationsHttpHeaderProvider::UpdateVariationIDsHeaderValue() {
+  lock_.AssertAcquired();
+
   // The header value is a serialized protobuffer of Variation IDs which is
   // base64 encoded before transmitting as a string.
   if (variation_ids_set_.empty())
@@ -113,9 +126,10 @@ void ChromeMetricsHelper::UpdateVariationIDsHeaderValue() {
   }
 
   metrics::ChromeVariations proto;
-  for (std::set<chrome_variations::VariationID>::const_iterator it =
-      variation_ids_set_.begin(); it != variation_ids_set_.end(); ++it)
+  for (std::set<VariationID>::const_iterator it = variation_ids_set_.begin();
+       it != variation_ids_set_.end(); ++it) {
     proto.add_variation_id(*it);
+  }
 
   std::string serialized;
   proto.SerializeToString(&serialized);
@@ -132,3 +146,5 @@ void ChromeMetricsHelper::UpdateVariationIDsHeaderValue() {
                  << serialized;
   }
 }
+
+}  // namespace chrome_variations

chrome/browser/chrome_metrics_helper.h → chrome/browser/metrics/variations/variations_http_header_provider.h

@@ -2,8 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef CHROME_BROWSER_CHROME_METRICS_HELPER_H_
-#define CHROME_BROWSER_CHROME_METRICS_HELPER_H_
+#ifndef CHROME_BROWSER_METRICS_VARIATIONS_VARIATIONS_HTTP_HEADER_PROVIDER_H_
+#define CHROME_BROWSER_METRICS_VARIATIONS_VARIATIONS_HTTP_HEADER_PROVIDER_H_
 
 #include <set>
 #include <string>
@@ -27,12 +27,14 @@ class ProfileIOData;
 
 template <typename T> struct DefaultSingletonTraits;
 
+namespace chrome_variations {
+
 // A helper class for maintaining Chrome experiments and metrics state
 // transmitted in custom HTTP request headers.
 // This class is a thread-safe singleton.
-class ChromeMetricsHelper : base::FieldTrialList::Observer {
+class VariationsHttpHeaderProvider : base::FieldTrialList::Observer {
  public:
-  static ChromeMetricsHelper* GetInstance();
+  static VariationsHttpHeaderProvider* GetInstance();
 
   // Adds Chrome experiment and metrics state as custom headers to |headers|.
   // Some headers may not be set given the |incognito| mode or whether
@@ -44,10 +46,10 @@ class ChromeMetricsHelper : base::FieldTrialList::Observer {
                      net::HttpRequestHeaders* headers);
 
  private:
-  friend struct DefaultSingletonTraits<ChromeMetricsHelper>;
+  friend struct DefaultSingletonTraits<VariationsHttpHeaderProvider>;
 
-  ChromeMetricsHelper();
-  virtual ~ChromeMetricsHelper();
+  VariationsHttpHeaderProvider();
+  virtual ~VariationsHttpHeaderProvider();
 
   // base::FieldTrialList::Observer implementation.
   // This will add the variation ID associated with |trial_name| and
@@ -62,7 +64,8 @@ class ChromeMetricsHelper : base::FieldTrialList::Observer {
   void InitVariationIDsCacheIfNeeded();
 
   // Takes whatever is currently in |variation_ids_set_| and recreates
-  // |variation_ids_header_| with it.
+  // |variation_ids_header_| with it.  Assumes the the |lock_| is currently
+  // held.
   void UpdateVariationIDsHeaderValue();
 
   // Guards |variation_ids_cache_initialized_|, |variation_ids_set_| and
@@ -77,7 +80,9 @@ class ChromeMetricsHelper : base::FieldTrialList::Observer {
   std::set<chrome_variations::VariationID> variation_ids_set_;
   std::string variation_ids_header_;
 
-  DISALLOW_COPY_AND_ASSIGN(ChromeMetricsHelper);
+  DISALLOW_COPY_AND_ASSIGN(VariationsHttpHeaderProvider);
 };
 
-#endif  // CHROME_BROWSER_CHROME_METRICS_HELPER_H_
+}  // namespace chrome_variations
+
+#endif  // CHROME_BROWSER_METRICS_VARIATIONS_VARIATIONS_HTTP_HEADER_PROVIDER_H_

chrome/browser/renderer_host/chrome_resource_dispatcher_host_delegate.cc

@@ -9,7 +9,6 @@
 #include "base/base64.h"
 #include "base/logging.h"
 #include "chrome/browser/browser_process.h"
-#include "chrome/browser/chrome_metrics_helper.h"
 #include "chrome/browser/content_settings/host_content_settings_map.h"
 #include "chrome/browser/download/download_request_limiter.h"
 #include "chrome/browser/download/download_resource_throttle.h"
@@ -17,6 +16,7 @@
 #include "chrome/browser/extensions/user_script_listener.h"
 #include "chrome/browser/external_protocol/external_protocol_handler.h"
 #include "chrome/browser/google/google_util.h"
+#include "chrome/browser/metrics/variations/variations_http_header_provider.h"
 #include "chrome/browser/net/load_timing_observer.h"
 #include "chrome/browser/net/resource_prefetch_predictor_observer.h"
 #include "chrome/browser/prerender/prerender_manager.h"
@@ -173,9 +173,11 @@ void ChromeResourceDispatcherHostDelegate::RequestBeginning(
     ProfileIOData* io_data = ProfileIOData::FromResourceContext(
         resource_context);
     bool incognito = io_data->is_incognito();
-    ChromeMetricsHelper::GetInstance()->AppendHeaders(
-        request->url(), incognito,
-        !incognito && io_data->GetMetricsEnabledStateOnIOThread(), &headers);
+    chrome_variations::VariationsHttpHeaderProvider::GetInstance()->
+        AppendHeaders(request->url(),
+                      incognito,
+                      !incognito && io_data->GetMetricsEnabledStateOnIOThread(),
+                      &headers);
     request->SetExtraRequestHeaders(headers);
   }
 

chrome/chrome_browser.gypi

@@ -431,8 +431,6 @@
         'browser/chrome_browser_main_x11.h',
         'browser/chrome_content_browser_client.cc',
         'browser/chrome_content_browser_client.h',
-        'browser/chrome_metrics_helper.cc',
-        'browser/chrome_metrics_helper.h',
         'browser/chrome_page_zoom.cc',
         'browser/chrome_page_zoom.h',
         'browser/chrome_page_zoom_constants.cc',
@@ -1076,6 +1074,8 @@
         'browser/metrics/tracking_synchronizer_observer.h',
         'browser/metrics/variations/resource_request_allowed_notifier.cc',
         'browser/metrics/variations/resource_request_allowed_notifier.h',
+        'browser/metrics/variations/variations_http_header_provider.cc',
+        'browser/metrics/variations/variations_http_header_provider.h',
         'browser/metrics/variations/variations_service.cc',
         'browser/metrics/variations/variations_service.h',
         'browser/native_window_notification_source.h',