diff options
author | asvitkine <asvitkine@chromium.org> | 2015-12-17 18:35:50 -0800 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-12-18 02:36:44 +0000 |
commit | 9a2798399231b89c9becb3c9ca016da34550ebd2 (patch) | |
tree | 0eb51fa3d4becb0b56ba691e234795a946aaf30f | |
parent | aeb6648f175f95009e0eb10a07aca126ba81a878 (diff) | |
download | chromium_src-9a2798399231b89c9becb3c9ca016da34550ebd2.zip chromium_src-9a2798399231b89c9becb3c9ca016da34550ebd2.tar.gz chromium_src-9a2798399231b89c9becb3c9ca016da34550ebd2.tar.bz2 |
Refactor VariationsHttpHeaderProvider.
The goal is to move the bulk of its implementation to the main
variations component, so that it can be used by this CL from JNI:
https://codereview.chromium.org/1528543003/
Creates variations_http_headers.cc in net that still needs to depend
on net, which uses the http header provider internally. Updates
callers of the previous API to use the new simpler API that doesn't
require going through the singleton by clients.
Additionally, also adds variations/synthetic_trials.h and moves
the synthetic trials structs from metrics_service.h to the new
file. This works around an otherwise circular dependency between
metrics and variations, since variations_http_header_provider.cc
depends on synthetic trials.
TBRs below are for owners of downstream users of the API which
is being updated.
BUG=530223
TBR=caitkp@chromium.org,thestig@chromium.org
Review URL: https://codereview.chromium.org/1530133005
Cr-Commit-Position: refs/heads/master@{#365991}
33 files changed, 549 insertions, 487 deletions
diff --git a/chrome/browser/android/contextualsearch/contextual_search_delegate.cc b/chrome/browser/android/contextualsearch/contextual_search_delegate.cc index b4e206e..ded49f7 100644 --- a/chrome/browser/android/contextualsearch/contextual_search_delegate.cc +++ b/chrome/browser/android/contextualsearch/contextual_search_delegate.cc @@ -22,7 +22,7 @@ #include "chrome/common/pref_names.h" #include "components/browser_sync/browser/profile_sync_service.h" #include "components/search_engines/template_url_service.h" -#include "components/variations/net/variations_http_header_provider.h" +#include "components/variations/net/variations_http_headers.h" #include "components/variations/variations_associated_data.h" #include "content/public/browser/android/content_view_core.h" #include "content/public/browser/web_contents.h" @@ -128,11 +128,10 @@ void ContextualSearchDelegate::ContinueSearchTermResolutionRequest() { // Add Chrome experiment state to the request headers. net::HttpRequestHeaders headers; - variations::VariationsHttpHeaderProvider::GetInstance()->AppendHeaders( + variations::AppendVariationHeaders( search_term_fetcher_->GetOriginalURL(), false, // Impossible to be incognito at this point. - false, - &headers); + false, &headers); search_term_fetcher_->SetExtraRequestHeaders(headers.ToString()); SetDiscourseContextAndAddToHeader(*context_); diff --git a/chrome/browser/chrome_browser_main.cc b/chrome/browser/chrome_browser_main.cc index 3f042a7..286cf34 100644 --- a/chrome/browser/chrome_browser_main.cc +++ b/chrome/browser/chrome_browser_main.cc @@ -123,10 +123,10 @@ #include "components/translate/content/browser/browser_cld_utils.h" #include "components/translate/content/common/cld_data_source.h" #include "components/translate/core/browser/translate_download_manager.h" -#include "components/variations/net/variations_http_header_provider.h" #include "components/variations/pref_names.h" #include "components/variations/service/variations_service.h" #include "components/variations/variations_associated_data.h" +#include "components/variations/variations_http_header_provider.h" #include "components/variations/variations_switches.h" #include "components/version_info/version_info.h" #include "content/public/browser/browser_thread.h" diff --git a/chrome/browser/renderer_host/chrome_resource_dispatcher_host_delegate.cc b/chrome/browser/renderer_host/chrome_resource_dispatcher_host_delegate.cc index f552005..a8e6fa8 100644 --- a/chrome/browser/renderer_host/chrome_resource_dispatcher_host_delegate.cc +++ b/chrome/browser/renderer_host/chrome_resource_dispatcher_host_delegate.cc @@ -37,7 +37,7 @@ #include "components/content_settings/core/browser/host_content_settings_map.h" #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.h" #include "components/google/core/browser/google_util.h" -#include "components/variations/net/variations_http_header_provider.h" +#include "components/variations/net/variations_http_headers.h" #include "content/public/browser/browser_thread.h" #include "content/public/browser/notification_service.h" #include "content/public/browser/plugin_service.h" @@ -285,11 +285,9 @@ ChromeResourceDispatcherHostDelegate::ChromeResourceDispatcherHostDelegate() #endif { BrowserThread::PostTask( - BrowserThread::IO, - FROM_HERE, + BrowserThread::IO, FROM_HERE, base::Bind(content::ServiceWorkerContext::AddExcludedHeadersForFetchEvent, - variations::VariationsHttpHeaderProvider::GetInstance() - ->GetVariationHeaderNames())); + variations::GetVariationHeaderNames())); } ChromeResourceDispatcherHostDelegate::~ChromeResourceDispatcherHostDelegate() { @@ -371,12 +369,10 @@ void ChromeResourceDispatcherHostDelegate::RequestBeginning( net::HttpRequestHeaders headers; headers.CopyFrom(request->extra_request_headers()); bool is_off_the_record = io_data->IsOffTheRecord(); - variations::VariationsHttpHeaderProvider::GetInstance()-> - AppendHeaders(request->url(), - is_off_the_record, - !is_off_the_record && - io_data->GetMetricsEnabledStateOnIOThread(), - &headers); + variations::AppendVariationHeaders( + request->url(), is_off_the_record, + !is_off_the_record && io_data->GetMetricsEnabledStateOnIOThread(), + &headers); request->SetExtraRequestHeaders(headers); } diff --git a/chrome/browser/safe_browsing/srt_fetcher_win.cc b/chrome/browser/safe_browsing/srt_fetcher_win.cc index 9e616be..1640804 100644 --- a/chrome/browser/safe_browsing/srt_fetcher_win.cc +++ b/chrome/browser/safe_browsing/srt_fetcher_win.cc @@ -12,6 +12,7 @@ #include "base/command_line.h" #include "base/files/file_path.h" #include "base/metrics/field_trial.h" +#include "base/metrics/histogram_macros.h" #include "base/metrics/sparse_histogram.h" #include "base/prefs/pref_service.h" #include "base/process/launch.h" @@ -35,7 +36,7 @@ #include "chrome/browser/ui/global_error/global_error_service_factory.h" #include "components/component_updater/pref_names.h" #include "components/rappor/rappor_service.h" -#include "components/variations/net/variations_http_header_provider.h" +#include "components/variations/net/variations_http_headers.h" #include "content/public/browser/browser_thread.h" #include "net/base/load_flags.h" #include "net/http/http_status_code.h" @@ -230,9 +231,8 @@ class SRTFetcher : public net::URLFetcherDelegate { ProfileIOData* io_data = ProfileIOData::FromResourceContext( profile_->GetResourceContext()); net::HttpRequestHeaders headers; - variations::VariationsHttpHeaderProvider::GetInstance()->AppendHeaders( - url_fetcher_->GetOriginalURL(), - io_data->IsOffTheRecord(), + variations::AppendVariationHeaders( + url_fetcher_->GetOriginalURL(), io_data->IsOffTheRecord(), ChromeMetricsServiceAccessor::IsMetricsAndCrashReportingEnabled(), &headers); url_fetcher_->SetExtraRequestHeaders(headers.ToString()); diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi index fb8e42a..7de8592 100644 --- a/chrome/chrome_browser.gypi +++ b/chrome/chrome_browser.gypi @@ -3194,7 +3194,7 @@ # transitively via the common target because the proto sources need to # be generated before code in this target can start building. '../components/components.gyp:variations', - '../components/components.gyp:variations_http_provider', + '../components/components.gyp:variations_net', '../components/components.gyp:variations_service', '../components/components.gyp:webdata_common', '../components/components.gyp:webdata_services', diff --git a/chrome/chrome_browser_chromeos.gypi b/chrome/chrome_browser_chromeos.gypi index ff74e43..d75c5b8 100644 --- a/chrome/chrome_browser_chromeos.gypi +++ b/chrome/chrome_browser_chromeos.gypi @@ -1132,7 +1132,7 @@ # transitively via the common target because the proto sources need to # be generated before code in this target can start building. '../components/components.gyp:variations', - '../components/components.gyp:variations_http_provider', + '../components/components.gyp:variations_net', '../components/components.gyp:wallpaper', '../components/components.gyp:wifi_sync', '../components/components_strings.gyp:components_strings', diff --git a/components/autofill.gypi b/components/autofill.gypi index a15cd11..5d07ed2 100644 --- a/components/autofill.gypi +++ b/components/autofill.gypi @@ -96,7 +96,7 @@ 'signin_core_browser', 'signin_core_common', 'sync_driver', - 'variations_http_provider', + 'variations_net', 'webdata_common', ], 'sources': [ diff --git a/components/autofill/core/browser/autofill_download_manager.cc b/components/autofill/core/browser/autofill_download_manager.cc index 59578fe..0b92381 100644 --- a/components/autofill/core/browser/autofill_download_manager.cc +++ b/components/autofill/core/browser/autofill_download_manager.cc @@ -7,6 +7,7 @@ #include <utility> #include "base/bind.h" +#include "base/location.h" #include "base/logging.h" #include "base/numerics/safe_conversions.h" #include "base/rand_util.h" @@ -20,7 +21,7 @@ #include "components/autofill/core/common/autofill_pref_names.h" #include "components/compression/compression_utils.h" #include "components/data_use_measurement/core/data_use_user_data.h" -#include "components/variations/net/variations_http_header_provider.h" +#include "components/variations/net/variations_http_headers.h" #include "net/base/load_flags.h" #include "net/http/http_request_headers.h" #include "net/http/http_response_headers.h" @@ -205,7 +206,7 @@ bool AutofillDownloadManager::StartRequest( // Add Chrome experiment state and GZIP encoding to the request headers. net::HttpRequestHeaders headers; headers.SetHeaderIfMissing("content-encoding", "gzip"); - variations::VariationsHttpHeaderProvider::GetInstance()->AppendHeaders( + variations::AppendVariationHeaders( fetcher->GetOriginalURL(), driver_->IsOffTheRecord(), false, &headers); fetcher->SetExtraRequestHeaders(headers.ToString()); fetcher->Start(); diff --git a/components/components_tests.gyp b/components/components_tests.gyp index 5f16283..df387a3 100644 --- a/components/components_tests.gyp +++ b/components/components_tests.gyp @@ -798,11 +798,12 @@ 'variations/entropy_provider_unittest.cc', 'variations/experiment_labels_unittest.cc', 'variations/metrics_util_unittest.cc', - 'variations/net/variations_http_header_provider_unittest.cc', + 'variations/net/variations_http_headers_unittest.cc', 'variations/service/ui_string_overrider_unittest.cc', 'variations/service/variations_service_unittest.cc', 'variations/study_filtering_unittest.cc', 'variations/variations_associated_data_unittest.cc', + 'variations/variations_http_header_provider_unittest.cc', 'variations/variations_request_scheduler_mobile_unittest.cc', 'variations/variations_request_scheduler_unittest.cc', 'variations/variations_seed_processor_unittest.cc', @@ -1090,7 +1091,7 @@ 'components.gyp:user_prefs_tracked', 'components.gyp:user_prefs_tracked_test_support', 'components.gyp:variations', - 'components.gyp:variations_http_provider', + 'components.gyp:variations_net', 'components.gyp:variations_service', 'components.gyp:version_info', 'components.gyp:web_resource', diff --git a/components/feedback.gypi b/components/feedback.gypi index d51592c..4c25308 100644 --- a/components/feedback.gypi +++ b/components/feedback.gypi @@ -14,7 +14,7 @@ '../third_party/zlib/google/zip.gyp:zip', 'keyed_service_core', 'feedback_proto', - 'components.gyp:variations_http_provider', + 'components.gyp:variations_net', ], 'include_dirs': [ '..', diff --git a/components/feedback/feedback_uploader_chrome.cc b/components/feedback/feedback_uploader_chrome.cc index aea651f4..fa9951b 100644 --- a/components/feedback/feedback_uploader_chrome.cc +++ b/components/feedback/feedback_uploader_chrome.cc @@ -14,7 +14,7 @@ #include "components/feedback/feedback_report.h" #include "components/feedback/feedback_switches.h" #include "components/feedback/feedback_uploader_delegate.h" -#include "components/variations/net/variations_http_header_provider.h" +#include "components/variations/net/variations_http_headers.h" #include "content/public/browser/browser_context.h" #include "content/public/browser/browser_thread.h" #include "net/base/load_flags.h" @@ -57,7 +57,7 @@ void FeedbackUploaderChrome::DispatchReport(const std::string& data) { // Tell feedback server about the variation state of this install. net::HttpRequestHeaders headers; - variations::VariationsHttpHeaderProvider::GetInstance()->AppendHeaders( + variations::AppendVariationHeaders( fetcher->GetOriginalURL(), context_->IsOffTheRecord(), false, &headers); fetcher->SetExtraRequestHeaders(headers.ToString()); diff --git a/components/feedback/feedback_uploader_chrome_unittest.cc b/components/feedback/feedback_uploader_chrome_unittest.cc index 810c040..342e896 100644 --- a/components/feedback/feedback_uploader_chrome_unittest.cc +++ b/components/feedback/feedback_uploader_chrome_unittest.cc @@ -4,10 +4,12 @@ #include "components/feedback/feedback_uploader_chrome.h" +#include <string> + #include "base/message_loop/message_loop.h" #include "base/metrics/field_trial.h" -#include "components/variations/net/variations_http_header_provider.h" #include "components/variations/variations_associated_data.h" +#include "components/variations/variations_http_header_provider.h" #include "content/public/test/test_browser_context.h" #include "net/url_request/test_url_fetcher_factory.h" #include "net/url_request/url_fetcher_delegate.h" diff --git a/components/metrics/metrics_service.cc b/components/metrics/metrics_service.cc index 55aa9f1..f84deaf 100644 --- a/components/metrics/metrics_service.cc +++ b/components/metrics/metrics_service.cc @@ -242,15 +242,6 @@ bool ShouldUploadLog() { } // namespace - -SyntheticTrialGroup::SyntheticTrialGroup(uint32 trial, uint32 group) { - id.name = trial; - id.group = group; -} - -SyntheticTrialGroup::~SyntheticTrialGroup() { -} - // static MetricsService::ShutdownCleanliness MetricsService::clean_shutdown_status_ = MetricsService::CLEANLY_SHUTDOWN; @@ -1025,19 +1016,19 @@ bool MetricsService::UmaMetricsProperlyShutdown() { } void MetricsService::AddSyntheticTrialObserver( - SyntheticTrialObserver* observer) { + variations::SyntheticTrialObserver* observer) { synthetic_trial_observer_list_.AddObserver(observer); if (!synthetic_trial_groups_.empty()) observer->OnSyntheticTrialsChanged(synthetic_trial_groups_); } void MetricsService::RemoveSyntheticTrialObserver( - SyntheticTrialObserver* observer) { + variations::SyntheticTrialObserver* observer) { synthetic_trial_observer_list_.RemoveObserver(observer); } void MetricsService::RegisterSyntheticFieldTrial( - const SyntheticTrialGroup& trial) { + const variations::SyntheticTrialGroup& trial) { for (size_t i = 0; i < synthetic_trial_groups_.size(); ++i) { if (synthetic_trial_groups_[i].id.name == trial.id.name) { if (synthetic_trial_groups_[i].id.group != trial.id.group) { @@ -1049,7 +1040,7 @@ void MetricsService::RegisterSyntheticFieldTrial( } } - SyntheticTrialGroup trial_group = trial; + variations::SyntheticTrialGroup trial_group = trial; trial_group.start_time = base::TimeTicks::Now(); synthetic_trial_groups_.push_back(trial_group); NotifySyntheticTrialObservers(); @@ -1072,7 +1063,8 @@ void MetricsService::CheckForClonedInstall( } void MetricsService::NotifySyntheticTrialObservers() { - FOR_EACH_OBSERVER(SyntheticTrialObserver, synthetic_trial_observer_list_, + FOR_EACH_OBSERVER(variations::SyntheticTrialObserver, + synthetic_trial_observer_list_, OnSyntheticTrialsChanged(synthetic_trial_groups_)); } diff --git a/components/metrics/metrics_service.h b/components/metrics/metrics_service.h index 2345083..0939c13 100644 --- a/components/metrics/metrics_service.h +++ b/components/metrics/metrics_service.h @@ -28,7 +28,7 @@ #include "components/metrics/metrics_log_manager.h" #include "components/metrics/metrics_provider.h" #include "components/metrics/net/network_metrics_provider.h" -#include "components/variations/active_field_trials.h" +#include "components/variations/synthetic_trials.h" class PrefService; class PrefRegistrySimple; @@ -55,39 +55,6 @@ class MetricsServiceAccessor; class MetricsServiceClient; class MetricsStateManager; -// A Field Trial and its selected group, which represent a particular -// Chrome configuration state. For example, the trial name could map to -// a preference name, and the group name could map to a preference value. -struct SyntheticTrialGroup { - public: - ~SyntheticTrialGroup(); - - variations::ActiveGroupId id; - base::TimeTicks start_time; - - private: - // Synthetic field trial users: - friend class MetricsServiceAccessor; - friend class MetricsService; - FRIEND_TEST_ALL_PREFIXES(MetricsServiceTest, RegisterSyntheticTrial); - - // This constructor is private specifically so as to control which code is - // able to access it. New code that wishes to use it should be added as a - // friend class. - SyntheticTrialGroup(uint32 trial, uint32 group); -}; - -// Interface class to observe changes to synthetic trials in MetricsService. -class SyntheticTrialObserver { - public: - // Called when the list of synthetic field trial groups has changed. - virtual void OnSyntheticTrialsChanged( - const std::vector<SyntheticTrialGroup>& groups) = 0; - - protected: - virtual ~SyntheticTrialObserver() {} -}; - // See metrics_service.cc for a detailed description. class MetricsService : public base::HistogramFlattener { public: @@ -216,26 +183,17 @@ class MetricsService : public base::HistogramFlattener { // This value should be true when process has completed shutdown. static bool UmaMetricsProperlyShutdown(); - // Registers a field trial name and group to be used to annotate a UMA report - // with a particular Chrome configuration state. A UMA report will be - // annotated with this trial group if and only if all events in the report - // were created after the trial is registered. Only one group name may be - // registered at a time for a given trial_name. Only the last group name that - // is registered for a given trial name will be recorded. The values passed - // in must not correspond to any real field trial in the code. - // To use this method, SyntheticTrialGroup should friend your class. - void RegisterSyntheticFieldTrial(const SyntheticTrialGroup& trial_group); - // Public accessor that returns the list of synthetic field trials. It must // only be used for testing. void GetCurrentSyntheticFieldTrialsForTesting( std::vector<variations::ActiveGroupId>* synthetic_trials); // Adds an observer to be notified when the synthetic trials list changes. - void AddSyntheticTrialObserver(SyntheticTrialObserver* observer); + void AddSyntheticTrialObserver(variations::SyntheticTrialObserver* observer); // Removes an existing observer of synthetic trials list changes. - void RemoveSyntheticTrialObserver(SyntheticTrialObserver* observer); + void RemoveSyntheticTrialObserver( + variations::SyntheticTrialObserver* observer); // Register the specified |provider| to provide additional metrics into the // UMA log. Should be called during MetricsService initialization only. @@ -258,6 +216,8 @@ class MetricsService : public base::HistogramFlattener { MetricsLogManager* log_manager() { return &log_manager_; } private: + friend class MetricsServiceAccessor; + // The MetricsService has a lifecycle that is stored as a state. // See metrics_service.cc for description of this lifecycle. enum State { @@ -281,7 +241,17 @@ class MetricsService : public base::HistogramFlattener { UNSET }; - typedef std::vector<SyntheticTrialGroup> SyntheticTrialGroups; + typedef std::vector<variations::SyntheticTrialGroup> SyntheticTrialGroups; + + // Registers a field trial name and group to be used to annotate a UMA report + // with a particular Chrome configuration state. A UMA report will be + // annotated with this trial group if and only if all events in the report + // were created after the trial is registered. Only one group name may be + // registered at a time for a given trial_name. Only the last group name that + // is registered for a given trial name will be recorded. The values passed + // in must not correspond to any real field trial in the code. + void RegisterSyntheticFieldTrial( + const variations::SyntheticTrialGroup& trial_group); // Calls into the client to initialize some system profile metrics. void StartInitTask(); @@ -489,7 +459,8 @@ class MetricsService : public base::HistogramFlattener { SyntheticTrialGroups synthetic_trial_groups_; // List of observers of |synthetic_trial_groups_| changes. - base::ObserverList<SyntheticTrialObserver> synthetic_trial_observer_list_; + base::ObserverList<variations::SyntheticTrialObserver> + synthetic_trial_observer_list_; // Execution phase the browser is in. static ExecutionPhase execution_phase_; diff --git a/components/metrics/metrics_service_accessor.cc b/components/metrics/metrics_service_accessor.cc index 864dbe5..ce301c9 100644 --- a/components/metrics/metrics_service_accessor.cc +++ b/components/metrics/metrics_service_accessor.cc @@ -62,7 +62,7 @@ bool MetricsServiceAccessor::RegisterSyntheticFieldTrialWithNameAndGroupHash( if (!metrics_service) return false; - SyntheticTrialGroup trial_group(trial_name_hash, group_name_hash); + variations::SyntheticTrialGroup trial_group(trial_name_hash, group_name_hash); metrics_service->RegisterSyntheticFieldTrial(trial_group); return true; } diff --git a/components/metrics/metrics_service_unittest.cc b/components/metrics/metrics_service_unittest.cc index b306c9b..e3de5ae 100644 --- a/components/metrics/metrics_service_unittest.cc +++ b/components/metrics/metrics_service_unittest.cc @@ -315,10 +315,12 @@ TEST_F(MetricsServiceTest, RegisterSyntheticTrial) { MetricsService service(GetMetricsStateManager(), &client, GetLocalState()); // Add two synthetic trials and confirm that they show up in the list. - SyntheticTrialGroup trial1(HashName("TestTrial1"), HashName("Group1")); + variations::SyntheticTrialGroup trial1(HashName("TestTrial1"), + HashName("Group1")); service.RegisterSyntheticFieldTrial(trial1); - SyntheticTrialGroup trial2(HashName("TestTrial2"), HashName("Group2")); + variations::SyntheticTrialGroup trial2(HashName("TestTrial2"), + HashName("Group2")); service.RegisterSyntheticFieldTrial(trial2); // Ensure that time has advanced by at least a tick before proceeding. WaitUntilTimeChanges(base::TimeTicks::Now()); @@ -345,14 +347,16 @@ TEST_F(MetricsServiceTest, RegisterSyntheticTrial) { WaitUntilTimeChanges(begin_log_time); // Change the group for the first trial after the log started. - SyntheticTrialGroup trial3(HashName("TestTrial1"), HashName("Group2")); + variations::SyntheticTrialGroup trial3(HashName("TestTrial1"), + HashName("Group2")); service.RegisterSyntheticFieldTrial(trial3); service.GetSyntheticFieldTrialsOlderThan(begin_log_time, &synthetic_trials); EXPECT_EQ(1U, synthetic_trials.size()); EXPECT_TRUE(HasSyntheticTrial(synthetic_trials, "TestTrial2", "Group2")); // Add a new trial after the log started and confirm that it doesn't show up. - SyntheticTrialGroup trial4(HashName("TestTrial3"), HashName("Group3")); + variations::SyntheticTrialGroup trial4(HashName("TestTrial3"), + HashName("Group3")); service.RegisterSyntheticFieldTrial(trial4); service.GetSyntheticFieldTrialsOlderThan(begin_log_time, &synthetic_trials); EXPECT_EQ(1U, synthetic_trials.size()); diff --git a/components/omnibox.gypi b/components/omnibox.gypi index 8b2841d..dcb0536 100644 --- a/components/omnibox.gypi +++ b/components/omnibox.gypi @@ -37,7 +37,7 @@ 'search_engines', 'toolbar', 'url_formatter/url_formatter.gyp:url_formatter', - 'variations_http_provider', + 'variations_net', ], 'export_dependent_settings': [ 'component_metrics_proto', diff --git a/components/omnibox/browser/search_provider.cc b/components/omnibox/browser/search_provider.cc index c3c9a25..b26910c 100644 --- a/components/omnibox/browser/search_provider.cc +++ b/components/omnibox/browser/search_provider.cc @@ -33,7 +33,7 @@ #include "components/search_engines/template_url_prepopulate_data.h" #include "components/search_engines/template_url_service.h" #include "components/url_formatter/url_formatter.h" -#include "components/variations/net/variations_http_header_provider.h" +#include "components/variations/net/variations_http_headers.h" #include "grit/components_strings.h" #include "net/base/escape.h" #include "net/base/load_flags.h" @@ -891,7 +891,7 @@ scoped_ptr<net::URLFetcher> SearchProvider::CreateSuggestFetcher( fetcher->SetLoadFlags(net::LOAD_DO_NOT_SAVE_COOKIES); // Add Chrome experiment state to the request headers. net::HttpRequestHeaders headers; - variations::VariationsHttpHeaderProvider::GetInstance()->AppendHeaders( + variations::AppendVariationHeaders( fetcher->GetOriginalURL(), client()->IsOffTheRecord(), false, &headers); fetcher->SetExtraRequestHeaders(headers.ToString()); fetcher->Start(); diff --git a/components/omnibox/browser/zero_suggest_provider.cc b/components/omnibox/browser/zero_suggest_provider.cc index 9b9dcfc..a12a1b4 100644 --- a/components/omnibox/browser/zero_suggest_provider.cc +++ b/components/omnibox/browser/zero_suggest_provider.cc @@ -30,7 +30,7 @@ #include "components/pref_registry/pref_registry_syncable.h" #include "components/search_engines/template_url_service.h" #include "components/url_formatter/url_formatter.h" -#include "components/variations/net/variations_http_header_provider.h" +#include "components/variations/net/variations_http_headers.h" #include "net/base/escape.h" #include "net/base/load_flags.h" #include "net/http/http_request_headers.h" @@ -326,9 +326,9 @@ void ZeroSuggestProvider::Run(const GURL& suggest_url) { fetcher_->SetLoadFlags(net::LOAD_DO_NOT_SAVE_COOKIES); // Add Chrome experiment state to the request headers. net::HttpRequestHeaders headers; - variations::VariationsHttpHeaderProvider::GetInstance()->AppendHeaders( - fetcher_->GetOriginalURL(), client()->IsOffTheRecord(), false, - &headers); + variations::AppendVariationHeaders(fetcher_->GetOriginalURL(), + client()->IsOffTheRecord(), false, + &headers); fetcher_->SetExtraRequestHeaders(headers.ToString()); fetcher_->Start(); LogOmniboxZeroSuggestRequest(ZERO_SUGGEST_REQUEST_SENT); diff --git a/components/suggestions.gypi b/components/suggestions.gypi index 7be7e72..efffdef 100644 --- a/components/suggestions.gypi +++ b/components/suggestions.gypi @@ -20,7 +20,7 @@ 'components.gyp:keyed_service_core', 'components.gyp:pref_registry', 'components.gyp:variations', - 'components.gyp:variations_http_provider', + 'components.gyp:variations_net', ], 'sources': [ 'suggestions/blacklist_store.cc', diff --git a/components/suggestions/suggestions_service.cc b/components/suggestions/suggestions_service.cc index d85194d..4382fde 100644 --- a/components/suggestions/suggestions_service.cc +++ b/components/suggestions/suggestions_service.cc @@ -20,7 +20,7 @@ #include "components/pref_registry/pref_registry_syncable.h" #include "components/suggestions/blacklist_store.h" #include "components/suggestions/suggestions_store.h" -#include "components/variations/net/variations_http_header_provider.h" +#include "components/variations/net/variations_http_headers.h" #include "net/base/escape.h" #include "net/base/load_flags.h" #include "net/base/net_errors.h" @@ -272,8 +272,8 @@ scoped_ptr<net::URLFetcher> SuggestionsService::CreateSuggestionsRequest( request->SetRequestContext(url_request_context_); // Add Chrome experiment state to the request headers. net::HttpRequestHeaders headers; - variations::VariationsHttpHeaderProvider::GetInstance()->AppendHeaders( - request->GetOriginalURL(), false, false, &headers); + variations::AppendVariationHeaders(request->GetOriginalURL(), false, false, + &headers); request->SetExtraRequestHeaders(headers.ToString()); return request; } diff --git a/components/variations.gypi b/components/variations.gypi index 9649110..157f68c 100644 --- a/components/variations.gypi +++ b/components/variations.gypi @@ -50,10 +50,14 @@ 'variations/proto/variations_seed.proto', 'variations/study_filtering.cc', 'variations/study_filtering.h', + "variations/synthetic_trials.cc", + "variations/synthetic_trials.h", 'variations/variations_associated_data.cc', 'variations/variations_associated_data.h', 'variations/variations_experiment_util.cc', 'variations/variations_experiment_util.h', + 'variations/variations_http_header_provider.cc', + 'variations/variations_http_header_provider.h', 'variations/variations_request_scheduler.cc', 'variations/variations_request_scheduler.h', 'variations/variations_request_scheduler_mobile.cc', @@ -119,7 +123,7 @@ }, { # GN version: //components/variations/net:net - 'target_name': 'variations_http_provider', + 'target_name': 'variations_net', 'type': 'static_library', 'include_dirs': [ '..', @@ -136,8 +140,8 @@ 'components.gyp:metrics', ], 'sources': [ - 'variations/net/variations_http_header_provider.cc', - 'variations/net/variations_http_header_provider.h', + 'variations/net/variations_http_headers.cc', + 'variations/net/variations_http_headers.h', ], }, ], diff --git a/components/variations/BUILD.gn b/components/variations/BUILD.gn index a65148a..c490cab 100644 --- a/components/variations/BUILD.gn +++ b/components/variations/BUILD.gn @@ -34,10 +34,14 @@ source_set("variations") { "proto/variations_seed.proto", "study_filtering.cc", "study_filtering.h", + "synthetic_trials.cc", + "synthetic_trials.h", "variations_associated_data.cc", "variations_associated_data.h", "variations_experiment_util.cc", "variations_experiment_util.h", + "variations_http_header_provider.cc", + "variations_http_header_provider.h", "variations_request_scheduler.cc", "variations_request_scheduler.h", "variations_seed_processor.cc", @@ -96,9 +100,10 @@ source_set("unit_tests") { "entropy_provider_unittest.cc", "experiment_labels_unittest.cc", "metrics_util_unittest.cc", - "net/variations_http_header_provider_unittest.cc", + "net/variations_http_headers_unittest.cc", "study_filtering_unittest.cc", "variations_associated_data_unittest.cc", + "variations_http_header_provider_unittest.cc", "variations_request_scheduler_unittest.cc", "variations_seed_processor_unittest.cc", "variations_seed_simulator_unittest.cc", diff --git a/components/variations/net/BUILD.gn b/components/variations/net/BUILD.gn index 0511046..c037d46 100644 --- a/components/variations/net/BUILD.gn +++ b/components/variations/net/BUILD.gn @@ -4,8 +4,8 @@ source_set("net") { sources = [ - "variations_http_header_provider.cc", - "variations_http_header_provider.h", + "variations_http_headers.cc", + "variations_http_headers.h", ] public_deps = [ diff --git a/components/variations/net/variations_http_header_provider_unittest.cc b/components/variations/net/variations_http_header_provider_unittest.cc deleted file mode 100644 index 2cb5c81..0000000 --- a/components/variations/net/variations_http_header_provider_unittest.cc +++ /dev/null @@ -1,238 +0,0 @@ -// Copyright 2014 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "components/variations/net/variations_http_header_provider.h" - -#include <string> - -#include "base/base64.h" -#include "base/message_loop/message_loop.h" -#include "base/metrics/field_trial.h" -#include "base/run_loop.h" -#include "components/variations/entropy_provider.h" -#include "components/variations/proto/client_variations.pb.h" -#include "components/variations/variations_associated_data.h" -#include "net/http/http_request_headers.h" -#include "testing/gtest/include/gtest/gtest.h" -#include "url/gurl.h" - -namespace variations { - -namespace { - -// Decodes the variations header and extracts the variation ids. -bool ExtractVariationIds(const std::string& variations, - std::set<VariationID>* variation_ids, - std::set<VariationID>* trigger_ids) { - std::string serialized_proto; - if (!base::Base64Decode(variations, &serialized_proto)) - return false; - ClientVariations proto; - if (!proto.ParseFromString(serialized_proto)) return false; - for (int i = 0; i < proto.variation_id_size(); ++i) - variation_ids->insert(proto.variation_id(i)); - for (int i = 0; i < proto.trigger_variation_id_size(); ++i) - trigger_ids->insert(proto.trigger_variation_id(i)); - return true; -} - -scoped_refptr<base::FieldTrial> CreateTrialAndAssociateId( - const std::string& trial_name, - const std::string& default_group_name, - IDCollectionKey key, - VariationID id) { - scoped_refptr<base::FieldTrial> trial( - base::FieldTrialList::CreateFieldTrial(trial_name, default_group_name)); - - AssociateGoogleVariationID(key, trial->trial_name(), trial->group_name(), id); - - return trial; -} - -} // namespace - -class VariationsHttpHeaderProviderTest : public ::testing::Test { - public: - VariationsHttpHeaderProviderTest() {} - - ~VariationsHttpHeaderProviderTest() override {} - - void TearDown() override { testing::ClearAllVariationIDs(); } -}; - -TEST_F(VariationsHttpHeaderProviderTest, ShouldAppendHeaders) { - struct { - const char* url; - bool should_append_headers; - } cases[] = { - {"http://google.com", true}, - {"http://www.google.com", true}, - {"http://m.google.com", true}, - {"http://google.ca", true}, - {"https://google.ca", true}, - {"http://google.co.uk", true}, - {"http://google.co.uk:8080/", true}, - {"http://www.google.co.uk:8080/", true}, - {"http://google", false}, - - {"http://youtube.com", true}, - {"http://www.youtube.com", true}, - {"http://www.youtube.ca", true}, - {"http://www.youtube.co.uk:8080/", true}, - {"https://www.youtube.com", true}, - {"http://youtube", false}, - - {"http://www.yahoo.com", false}, - - {"http://ad.doubleclick.net", true}, - {"https://a.b.c.doubleclick.net", true}, - {"https://a.b.c.doubleclick.net:8081", true}, - {"http://www.doubleclick.com", true}, - {"http://www.doubleclick.org", false}, - {"http://www.doubleclick.net.com", false}, - {"https://www.doubleclick.net.com", false}, - - {"http://ad.googlesyndication.com", true}, - {"https://a.b.c.googlesyndication.com", true}, - {"https://a.b.c.googlesyndication.com:8080", true}, - {"http://www.doubleclick.edu", false}, - {"http://www.googlesyndication.com.edu", false}, - {"https://www.googlesyndication.com.com", false}, - - {"http://www.googleadservices.com", true}, - {"http://www.googleadservices.com:8080", true}, - {"https://www.googleadservices.com", true}, - {"https://www.internal.googleadservices.com", true}, - {"https://www2.googleadservices.com", true}, - {"https://www.googleadservices.org", false}, - {"https://www.googleadservices.com.co.uk", false}, - - {"http://WWW.ANDROID.COM", true}, - {"http://www.android.com", true}, - {"http://www.doubleclick.com", true}, - {"http://www.doubleclick.net", true}, - {"http://www.ggpht.com", true}, - {"http://www.googleadservices.com", true}, - {"http://www.googleapis.com", true}, - {"http://www.googlesyndication.com", true}, - {"http://www.googleusercontent.com", true}, - {"http://www.googlevideo.com", true}, - {"http://ssl.gstatic.com", true}, - {"http://www.gstatic.com", true}, - {"http://www.ytimg.com", true}, - {"http://wwwytimg.com", false}, - {"http://ytimg.com", false}, - - {"http://www.android.org", false}, - {"http://www.doubleclick.org", false}, - {"http://www.doubleclick.net", true}, - {"http://www.ggpht.org", false}, - {"http://www.googleadservices.org", false}, - {"http://www.googleapis.org", false}, - {"http://www.googlesyndication.org", false}, - {"http://www.googleusercontent.org", false}, - {"http://www.googlevideo.org", false}, - {"http://ssl.gstatic.org", false}, - {"http://www.gstatic.org", false}, - {"http://www.ytimg.org", false}, - - {"http://a.b.android.com", true}, - {"http://a.b.doubleclick.com", true}, - {"http://a.b.doubleclick.net", true}, - {"http://a.b.ggpht.com", true}, - {"http://a.b.googleadservices.com", true}, - {"http://a.b.googleapis.com", true}, - {"http://a.b.googlesyndication.com", true}, - {"http://a.b.googleusercontent.com", true}, - {"http://a.b.googlevideo.com", true}, - {"http://ssl.gstatic.com", true}, - {"http://a.b.gstatic.com", true}, - {"http://a.b.ytimg.com", true}, - }; - - for (size_t i = 0; i < arraysize(cases); ++i) { - const GURL url(cases[i].url); - EXPECT_EQ(cases[i].should_append_headers, - VariationsHttpHeaderProvider::ShouldAppendHeaders(url)) - << url; - } -} - -TEST_F(VariationsHttpHeaderProviderTest, SetDefaultVariationIds_Valid) { - base::MessageLoop loop; - VariationsHttpHeaderProvider provider; - GURL url("http://www.google.com"); - net::HttpRequestHeaders headers; - std::string variations; - - // Valid experiment ids. - EXPECT_TRUE(provider.SetDefaultVariationIds("12,456,t789")); - provider.InitVariationIDsCacheIfNeeded(); - provider.AppendHeaders(url, false, false, &headers); - EXPECT_TRUE(headers.HasHeader("X-Client-Data")); - headers.GetHeader("X-Client-Data", &variations); - std::set<VariationID> variation_ids; - std::set<VariationID> trigger_ids; - ASSERT_TRUE(ExtractVariationIds(variations, &variation_ids, &trigger_ids)); - EXPECT_TRUE(variation_ids.find(12) != variation_ids.end()); - EXPECT_TRUE(variation_ids.find(456) != variation_ids.end()); - EXPECT_TRUE(trigger_ids.find(789) != trigger_ids.end()); - EXPECT_FALSE(variation_ids.find(789) != variation_ids.end()); -} - -TEST_F(VariationsHttpHeaderProviderTest, SetDefaultVariationIds_Invalid) { - base::MessageLoop loop; - VariationsHttpHeaderProvider provider; - GURL url("http://www.google.com"); - net::HttpRequestHeaders headers; - - // Invalid experiment ids. - EXPECT_FALSE(provider.SetDefaultVariationIds("abcd12,456")); - provider.InitVariationIDsCacheIfNeeded(); - provider.AppendHeaders(url, false, false, &headers); - EXPECT_FALSE(headers.HasHeader("X-Client-Data")); - - // Invalid trigger experiment id - EXPECT_FALSE(provider.SetDefaultVariationIds("12,tabc456")); - provider.InitVariationIDsCacheIfNeeded(); - provider.AppendHeaders(url, false, false, &headers); - EXPECT_FALSE(headers.HasHeader("X-Client-Data")); -} - -TEST_F(VariationsHttpHeaderProviderTest, OnFieldTrialGroupFinalized) { - base::MessageLoop loop; - base::FieldTrialList field_trial_list( - new metrics::SHA1EntropyProvider("test")); - VariationsHttpHeaderProvider provider; - provider.InitVariationIDsCacheIfNeeded(); - - const std::string default_name = "default"; - scoped_refptr<base::FieldTrial> trial_1(CreateTrialAndAssociateId( - "t1", default_name, GOOGLE_WEB_PROPERTIES, 123)); - - ASSERT_EQ(default_name, trial_1->group_name()); - - scoped_refptr<base::FieldTrial> trial_2(CreateTrialAndAssociateId( - "t2", default_name, GOOGLE_WEB_PROPERTIES_TRIGGER, 456)); - - ASSERT_EQ(default_name, trial_2->group_name()); - - // Run the message loop to make sure OnFieldTrialGroupFinalized is called for - // the two field trials. - base::RunLoop().RunUntilIdle(); - - GURL url("http://www.google.com"); - net::HttpRequestHeaders headers; - provider.AppendHeaders(url, false, false, &headers); - std::string variations; - headers.GetHeader("X-Client-Data", &variations); - - std::set<VariationID> variation_ids; - std::set<VariationID> trigger_ids; - ASSERT_TRUE(ExtractVariationIds(variations, &variation_ids, &trigger_ids)); - EXPECT_TRUE(variation_ids.find(123) != variation_ids.end()); - EXPECT_TRUE(trigger_ids.find(456) != trigger_ids.end()); -} - -} // namespace variations diff --git a/components/variations/net/variations_http_headers.cc b/components/variations/net/variations_http_headers.cc new file mode 100644 index 0000000..632a0e0 --- /dev/null +++ b/components/variations/net/variations_http_headers.cc @@ -0,0 +1,98 @@ +// Copyright 2015 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "components/variations/net/variations_http_headers.h" + +#include "base/strings/string_util.h" +#include "components/google/core/browser/google_util.h" +#include "components/variations/variations_http_header_provider.h" +#include "net/http/http_request_headers.h" +#include "url/gurl.h" + +namespace variations { + +namespace { + +const char* kSuffixesToSetHeadersFor[] = { + ".android.com", + ".doubleclick.com", + ".doubleclick.net", + ".ggpht.com", + ".googleadservices.com", + ".googleapis.com", + ".googlesyndication.com", + ".googleusercontent.com", + ".googlevideo.com", + ".gstatic.com", + ".ytimg.com", +}; + +const char kChromeUMAEnabled[] = "X-Chrome-UMA-Enabled"; +const char kClientData[] = "X-Client-Data"; + +} // namespace + +void AppendVariationHeaders(const GURL& url, + bool incognito, + bool uma_enabled, + net::HttpRequestHeaders* headers) { + // Note the criteria for attaching client experiment headers: + // 1. We only transmit to Google owned domains which can evaluate experiments. + // 1a. These include hosts which have a standard postfix such as: + // *.doubleclick.net or *.googlesyndication.com or + // exactly www.googleadservices.com or + // international TLD domains *.google.<TLD> or *.youtube.<TLD>. + // 2. Only transmit for non-Incognito profiles. + // 3. For the X-Chrome-UMA-Enabled bit, only set it if UMA is in fact enabled + // for this install of Chrome. + // 4. For the X-Client-Data header, only include non-empty variation IDs. + if (incognito || !internal::ShouldAppendVariationHeaders(url)) + return; + + if (uma_enabled) + headers->SetHeaderIfMissing(kChromeUMAEnabled, "1"); + + const std::string variation_ids_header = + VariationsHttpHeaderProvider::GetInstance()->GetClientDataHeader(); + if (!variation_ids_header.empty()) { + // Note that prior to M33 this header was named X-Chrome-Variations. + headers->SetHeaderIfMissing(kClientData, variation_ids_header); + } +} + +std::set<std::string> GetVariationHeaderNames() { + std::set<std::string> headers; + headers.insert(kChromeUMAEnabled); + headers.insert(kClientData); + return headers; +} + +namespace internal { + +// static +bool ShouldAppendVariationHeaders(const GURL& url) { + if (google_util::IsGoogleDomainUrl(url, google_util::ALLOW_SUBDOMAIN, + google_util::ALLOW_NON_STANDARD_PORTS)) { + return true; + } + + if (!url.is_valid() || !url.SchemeIsHTTPOrHTTPS()) + return false; + + // Some domains don't have international TLD extensions, so testing for them + // is very straight forward. + const std::string host = url.host(); + for (size_t i = 0; i < arraysize(kSuffixesToSetHeadersFor); ++i) { + if (base::EndsWith(host, kSuffixesToSetHeadersFor[i], + base::CompareCase::INSENSITIVE_ASCII)) + return true; + } + + return google_util::IsYoutubeDomainUrl(url, google_util::ALLOW_SUBDOMAIN, + google_util::ALLOW_NON_STANDARD_PORTS); +} + +} // namespace internal + +} // namespace variations diff --git a/components/variations/net/variations_http_headers.h b/components/variations/net/variations_http_headers.h new file mode 100644 index 0000000..d6280e5 --- /dev/null +++ b/components/variations/net/variations_http_headers.h @@ -0,0 +1,41 @@ +// Copyright 2015 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef COMPONENTS_VARIATIONS_NET_VARIATIONS_HTTP_HEADERS_H_ +#define COMPONENTS_VARIATIONS_NET_VARIATIONS_HTTP_HEADERS_H_ + +#include <set> +#include <string> + +namespace net { +class HttpRequestHeaders; +} + +class GURL; + +namespace variations { + +// Adds Chrome experiment and metrics state as custom headers to |headers|. +// Some headers may not be set given the |incognito| mode or whether +// the user has |uma_enabled|. Also, we never transmit headers to non-Google +// sites, which is checked based on the destination |url|. +void AppendVariationHeaders(const GURL& url, + bool incognito, + bool uma_enabled, + net::HttpRequestHeaders* headers); + +// Returns the HTTP header names which are added by AppendVariationHeaders(). +std::set<std::string> GetVariationHeaderNames(); + +namespace internal { + +// Checks whether variation headers should be appended to requests to the +// specified |url|. Returns true for google.<TLD> and youtube.<TLD> URLs. +bool ShouldAppendVariationHeaders(const GURL& url); + +} // namespace internal + +} // namespace variations + +#endif // COMPONENTS_VARIATIONS_NET_VARIATIONS_HTTP_HEADERS_H_ diff --git a/components/variations/net/variations_http_headers_unittest.cc b/components/variations/net/variations_http_headers_unittest.cc new file mode 100644 index 0000000..0f04c44 --- /dev/null +++ b/components/variations/net/variations_http_headers_unittest.cc @@ -0,0 +1,111 @@ +// Copyright 2014 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "components/variations/net/variations_http_headers.h" + +#include "base/macros.h" +#include "testing/gtest/include/gtest/gtest.h" +#include "url/gurl.h" + +namespace variations { + +TEST(VariationsHttpHeadersTest, ShouldAppendHeaders) { + struct { + const char* url; + bool should_append_headers; + } cases[] = { + {"http://google.com", true}, + {"http://www.google.com", true}, + {"http://m.google.com", true}, + {"http://google.ca", true}, + {"https://google.ca", true}, + {"http://google.co.uk", true}, + {"http://google.co.uk:8080/", true}, + {"http://www.google.co.uk:8080/", true}, + {"http://google", false}, + + {"http://youtube.com", true}, + {"http://www.youtube.com", true}, + {"http://www.youtube.ca", true}, + {"http://www.youtube.co.uk:8080/", true}, + {"https://www.youtube.com", true}, + {"http://youtube", false}, + + {"http://www.yahoo.com", false}, + + {"http://ad.doubleclick.net", true}, + {"https://a.b.c.doubleclick.net", true}, + {"https://a.b.c.doubleclick.net:8081", true}, + {"http://www.doubleclick.com", true}, + {"http://www.doubleclick.org", false}, + {"http://www.doubleclick.net.com", false}, + {"https://www.doubleclick.net.com", false}, + + {"http://ad.googlesyndication.com", true}, + {"https://a.b.c.googlesyndication.com", true}, + {"https://a.b.c.googlesyndication.com:8080", true}, + {"http://www.doubleclick.edu", false}, + {"http://www.googlesyndication.com.edu", false}, + {"https://www.googlesyndication.com.com", false}, + + {"http://www.googleadservices.com", true}, + {"http://www.googleadservices.com:8080", true}, + {"https://www.googleadservices.com", true}, + {"https://www.internal.googleadservices.com", true}, + {"https://www2.googleadservices.com", true}, + {"https://www.googleadservices.org", false}, + {"https://www.googleadservices.com.co.uk", false}, + + {"http://WWW.ANDROID.COM", true}, + {"http://www.android.com", true}, + {"http://www.doubleclick.com", true}, + {"http://www.doubleclick.net", true}, + {"http://www.ggpht.com", true}, + {"http://www.googleadservices.com", true}, + {"http://www.googleapis.com", true}, + {"http://www.googlesyndication.com", true}, + {"http://www.googleusercontent.com", true}, + {"http://www.googlevideo.com", true}, + {"http://ssl.gstatic.com", true}, + {"http://www.gstatic.com", true}, + {"http://www.ytimg.com", true}, + {"http://wwwytimg.com", false}, + {"http://ytimg.com", false}, + + {"http://www.android.org", false}, + {"http://www.doubleclick.org", false}, + {"http://www.doubleclick.net", true}, + {"http://www.ggpht.org", false}, + {"http://www.googleadservices.org", false}, + {"http://www.googleapis.org", false}, + {"http://www.googlesyndication.org", false}, + {"http://www.googleusercontent.org", false}, + {"http://www.googlevideo.org", false}, + {"http://ssl.gstatic.org", false}, + {"http://www.gstatic.org", false}, + {"http://www.ytimg.org", false}, + + {"http://a.b.android.com", true}, + {"http://a.b.doubleclick.com", true}, + {"http://a.b.doubleclick.net", true}, + {"http://a.b.ggpht.com", true}, + {"http://a.b.googleadservices.com", true}, + {"http://a.b.googleapis.com", true}, + {"http://a.b.googlesyndication.com", true}, + {"http://a.b.googleusercontent.com", true}, + {"http://a.b.googlevideo.com", true}, + {"http://ssl.gstatic.com", true}, + {"http://a.b.gstatic.com", true}, + {"http://a.b.ytimg.com", true}, + }; + + for (size_t i = 0; i < arraysize(cases); ++i) { + const GURL url(cases[i].url); + EXPECT_EQ(cases[i].should_append_headers, + internal::ShouldAppendVariationHeaders(url)) + << url; + } +} + +} // namespace variations diff --git a/components/variations/synthetic_trials.cc b/components/variations/synthetic_trials.cc new file mode 100644 index 0000000..228ca44 --- /dev/null +++ b/components/variations/synthetic_trials.cc @@ -0,0 +1,16 @@ +// Copyright 2015 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "components/variations/synthetic_trials.h" + +namespace variations { + +SyntheticTrialGroup::SyntheticTrialGroup(uint32_t trial, uint32_t group) { + id.name = trial; + id.group = group; +} + +SyntheticTrialGroup::~SyntheticTrialGroup() {} + +} // namespace variations diff --git a/components/variations/synthetic_trials.h b/components/variations/synthetic_trials.h new file mode 100644 index 0000000..b5b4240 --- /dev/null +++ b/components/variations/synthetic_trials.h @@ -0,0 +1,41 @@ +// Copyright 2015 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef COMPONENTS_VARIATIONS_SYNTHETIC_TRIALS_H_ +#define COMPONENTS_VARIATIONS_SYNTHETIC_TRIALS_H_ + +#include <vector> + +#include "base/compiler_specific.h" +#include "base/time/time.h" +#include "components/variations/active_field_trials.h" + +namespace variations { + +// A Field Trial and its selected group, which represent a particular +// Chrome configuration state. For example, the trial name could map to +// a preference name, and the group name could map to a preference value. +struct SyntheticTrialGroup { + public: + SyntheticTrialGroup(uint32_t trial, uint32_t group); + ~SyntheticTrialGroup(); + + ActiveGroupId id; + base::TimeTicks start_time; +}; + +// Interface class to observe changes to synthetic trials in MetricsService. +class SyntheticTrialObserver { + public: + // Called when the list of synthetic field trial groups has changed. + virtual void OnSyntheticTrialsChanged( + const std::vector<SyntheticTrialGroup>& groups) = 0; + + protected: + virtual ~SyntheticTrialObserver() {} +}; + +} // namespace variations + +#endif // COMPONENTS_VARIATIONS_SYNTHETIC_TRIALS_H_ diff --git a/components/variations/net/variations_http_header_provider.cc b/components/variations/variations_http_header_provider.cc index 6754cef..316fe52 100644 --- a/components/variations/net/variations_http_header_provider.cc +++ b/components/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 "components/variations/net/variations_http_header_provider.h" +#include "components/variations/variations_http_header_provider.h" #include <set> #include <string> @@ -14,60 +14,16 @@ #include "base/strings/string_number_conversions.h" #include "base/strings/string_split.h" #include "base/strings/string_util.h" -#include "components/google/core/browser/google_util.h" #include "components/variations/proto/client_variations.pb.h" -#include "net/base/registry_controlled_domains/registry_controlled_domain.h" -#include "net/http/http_request_headers.h" -#include "url/gurl.h" namespace variations { -namespace { - -const char* kSuffixesToSetHeadersFor[] = { - ".android.com", - ".doubleclick.com", - ".doubleclick.net", - ".ggpht.com", - ".googleadservices.com", - ".googleapis.com", - ".googlesyndication.com", - ".googleusercontent.com", - ".googlevideo.com", - ".gstatic.com", - ".ytimg.com", -}; - -const char kChromeUMAEnabled[] = "X-Chrome-UMA-Enabled"; -const char kClientData[] = "X-Client-Data"; - -} // namespace - +// static VariationsHttpHeaderProvider* VariationsHttpHeaderProvider::GetInstance() { return base::Singleton<VariationsHttpHeaderProvider>::get(); } -void VariationsHttpHeaderProvider::AppendHeaders( - const GURL& url, - bool incognito, - bool uma_enabled, - net::HttpRequestHeaders* headers) { - // Note the criteria for attaching client experiment headers: - // 1. We only transmit to Google owned domains which can evaluate experiments. - // 1a. These include hosts which have a standard postfix such as: - // *.doubleclick.net or *.googlesyndication.com or - // exactly www.googleadservices.com or - // international TLD domains *.google.<TLD> or *.youtube.<TLD>. - // 2. Only transmit for non-Incognito profiles. - // 3. For the X-Chrome-UMA-Enabled bit, only set it if UMA is in fact enabled - // for this install of Chrome. - // 4. For the X-Client-Data header, only include non-empty variation IDs. - if (incognito || !ShouldAppendHeaders(url)) - return; - - if (uma_enabled) - headers->SetHeaderIfMissing(kChromeUMAEnabled, "1"); - +std::string VariationsHttpHeaderProvider::GetClientDataHeader() { // Lazily initialize the header, if not already done, before attempting to // transmit it. InitVariationIDsCacheIfNeeded(); @@ -77,11 +33,7 @@ void VariationsHttpHeaderProvider::AppendHeaders( base::AutoLock scoped_lock(lock_); variation_ids_header_copy = variation_ids_header_; } - - if (!variation_ids_header_copy.empty()) { - // Note that prior to M33 this header was named X-Chrome-Variations. - headers->SetHeaderIfMissing(kClientData, variation_ids_header_copy); - } + return variation_ids_header_copy; } bool VariationsHttpHeaderProvider::SetDefaultVariationIds( @@ -98,7 +50,7 @@ bool VariationsHttpHeaderProvider::SetDefaultVariationIds( bool trigger_id = base::StartsWith(entry, "t", base::CompareCase::SENSITIVE); // Remove the "t" prefix if it's there. - base::StringPiece trimmed_entry = trigger_id ? entry.substr(1) : entry; + base::StringPiece trimmed_entry = trigger_id ? entry.substr(1) : entry; int variation_id = 0; if (!base::StringToInt(trimmed_entry, &variation_id)) { @@ -114,14 +66,6 @@ bool VariationsHttpHeaderProvider::SetDefaultVariationIds( return true; } -std::set<std::string> VariationsHttpHeaderProvider::GetVariationHeaderNames() - const { - std::set<std::string> headers; - headers.insert(kChromeUMAEnabled); - headers.insert(kClientData); - return headers; -} - void VariationsHttpHeaderProvider::ResetForTesting() { base::AutoLock scoped_lock(lock_); @@ -132,11 +76,9 @@ void VariationsHttpHeaderProvider::ResetForTesting() { } VariationsHttpHeaderProvider::VariationsHttpHeaderProvider() - : variation_ids_cache_initialized_(false) { -} + : variation_ids_cache_initialized_(false) {} -VariationsHttpHeaderProvider::~VariationsHttpHeaderProvider() { -} +VariationsHttpHeaderProvider::~VariationsHttpHeaderProvider() {} void VariationsHttpHeaderProvider::OnFieldTrialGroupFinalized( const std::string& trial_name, @@ -158,11 +100,11 @@ void VariationsHttpHeaderProvider::OnFieldTrialGroupFinalized( } void VariationsHttpHeaderProvider::OnSyntheticTrialsChanged( - const std::vector<metrics::SyntheticTrialGroup>& groups) { + const std::vector<SyntheticTrialGroup>& groups) { base::AutoLock scoped_lock(lock_); synthetic_variation_ids_set_.clear(); - for (const metrics::SyntheticTrialGroup& group : groups) { + for (const SyntheticTrialGroup& group : groups) { const VariationID id = GetGoogleVariationIDFromHashes(GOOGLE_WEB_PROPERTIES, group.id); if (id != EMPTY_ID) @@ -188,15 +130,13 @@ void VariationsHttpHeaderProvider::InitVariationIDsCacheIfNeeded() { for (base::FieldTrial::ActiveGroups::const_iterator it = initial_groups.begin(); it != initial_groups.end(); ++it) { - const VariationID id = - GetGoogleVariationID(GOOGLE_WEB_PROPERTIES, it->trial_name, - it->group_name); + const VariationID id = GetGoogleVariationID(GOOGLE_WEB_PROPERTIES, + it->trial_name, it->group_name); if (id != EMPTY_ID) variation_ids_set_.insert(id); - const VariationID trigger_id = - GetGoogleVariationID(GOOGLE_WEB_PROPERTIES_TRIGGER, it->trial_name, - it->group_name); + const VariationID trigger_id = GetGoogleVariationID( + GOOGLE_WEB_PROPERTIES_TRIGGER, it->trial_name, it->group_name); if (trigger_id != EMPTY_ID) variation_trigger_ids_set_.insert(trigger_id); } @@ -204,10 +144,8 @@ void VariationsHttpHeaderProvider::InitVariationIDsCacheIfNeeded() { UMA_HISTOGRAM_CUSTOM_COUNTS( "Variations.HeaderConstructionTime", - (base::TimeTicks::Now() - before_time).InMicroseconds(), - 0, - base::TimeDelta::FromSeconds(1).InMicroseconds(), - 50); + (base::TimeTicks::Now() - before_time).InMicroseconds(), 0, + base::TimeDelta::FromSeconds(1).InMicroseconds(), 50); variation_ids_cache_initialized_ = true; } @@ -265,27 +203,4 @@ void VariationsHttpHeaderProvider::UpdateVariationIDsHeaderValue() { variation_ids_header_ = hashed; } -// static -bool VariationsHttpHeaderProvider::ShouldAppendHeaders(const GURL& url) { - if (google_util::IsGoogleDomainUrl(url, google_util::ALLOW_SUBDOMAIN, - google_util::ALLOW_NON_STANDARD_PORTS)) { - return true; - } - - if (!url.is_valid() || !url.SchemeIsHTTPOrHTTPS()) - return false; - - // Some domains don't have international TLD extensions, so testing for them - // is very straight forward. - const std::string host = url.host(); - for (size_t i = 0; i < arraysize(kSuffixesToSetHeadersFor); ++i) { - if (base::EndsWith(host, kSuffixesToSetHeadersFor[i], - base::CompareCase::INSENSITIVE_ASCII)) - return true; - } - - return google_util::IsYoutubeDomainUrl(url, google_util::ALLOW_SUBDOMAIN, - google_util::ALLOW_NON_STANDARD_PORTS); -} - } // namespace variations diff --git a/components/variations/net/variations_http_header_provider.h b/components/variations/variations_http_header_provider.h index 21f8659..18ff19d 100644 --- a/components/variations/net/variations_http_header_provider.h +++ b/components/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 COMPONENTS_VARIATIONS_NET_VARIATIONS_HTTP_HEADER_PROVIDER_H_ -#define COMPONENTS_VARIATIONS_NET_VARIATIONS_HTTP_HEADER_PROVIDER_H_ +#ifndef COMPONENTS_VARIATIONS_VARIATIONS_HTTP_HEADER_PROVIDER_H_ +#define COMPONENTS_VARIATIONS_VARIATIONS_HTTP_HEADER_PROVIDER_H_ #include <set> #include <string> @@ -13,21 +13,12 @@ #include "base/gtest_prod_util.h" #include "base/metrics/field_trial.h" #include "base/synchronization/lock.h" -#include "components/metrics/metrics_service.h" +#include "components/variations/synthetic_trials.h" #include "components/variations/variations_associated_data.h" -namespace content { -class ResourceContext; -} - -namespace net { -class HttpRequestHeaders; -} - -class GURL; - namespace base { -template <typename T> struct DefaultSingletonTraits; +template <typename T> +struct DefaultSingletonTraits; } namespace variations { @@ -36,18 +27,13 @@ namespace variations { // transmitted in custom HTTP request headers. // This class is a thread-safe singleton. class VariationsHttpHeaderProvider : public base::FieldTrialList::Observer, - public metrics::SyntheticTrialObserver { + public SyntheticTrialObserver { public: 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 - // the user has |uma_enabled|. Also, we never transmit headers to non-Google - // sites, which is checked based on the destination |url|. - void AppendHeaders(const GURL& url, - bool incognito, - bool uma_enabled, - net::HttpRequestHeaders* headers); + // Returns the value of the client data header, computing and caching it if + // necessary. + std::string GetClientDataHeader(); // Sets *additional* variation ids and trigger variation ids to be encoded in // the X-Client-Data request header. This is intended for development use to @@ -56,9 +42,6 @@ class VariationsHttpHeaderProvider : public base::FieldTrialList::Observer, // with "t" it will be treated as a trigger experiment id. bool SetDefaultVariationIds(const std::string& variation_ids); - // Returns the HTTP header names which are added in this class. - std::set<std::string> GetVariationHeaderNames() const; - // Resets any cached state for tests. void ResetForTesting(); @@ -66,8 +49,6 @@ class VariationsHttpHeaderProvider : public base::FieldTrialList::Observer, friend struct base::DefaultSingletonTraits<VariationsHttpHeaderProvider>; FRIEND_TEST_ALL_PREFIXES(VariationsHttpHeaderProviderTest, - ShouldAppendHeaders); - FRIEND_TEST_ALL_PREFIXES(VariationsHttpHeaderProviderTest, SetDefaultVariationIds_Valid); FRIEND_TEST_ALL_PREFIXES(VariationsHttpHeaderProviderTest, SetDefaultVariationIds_Invalid); @@ -85,7 +66,7 @@ class VariationsHttpHeaderProvider : public base::FieldTrialList::Observer, // metrics::SyntheticTrialObserver: void OnSyntheticTrialsChanged( - const std::vector<metrics::SyntheticTrialGroup>& groups) override; + const std::vector<SyntheticTrialGroup>& groups) override; // Prepares the variation IDs cache with initial values if not already done. // This method also registers the caller with the FieldTrialList to receive @@ -97,10 +78,6 @@ class VariationsHttpHeaderProvider : public base::FieldTrialList::Observer, // held. void UpdateVariationIDsHeaderValue(); - // Checks whether variation headers should be appended to requests to the - // specified |url|. Returns true for google.<TLD> and youtube.<TLD> URLs. - static bool ShouldAppendHeaders(const GURL& url); - // Guards |variation_ids_cache_initialized_|, |variation_ids_set_| and // |variation_ids_header_|. base::Lock lock_; @@ -127,4 +104,4 @@ class VariationsHttpHeaderProvider : public base::FieldTrialList::Observer, } // namespace variations -#endif // COMPONENTS_VARIATIONS_NET_VARIATIONS_HTTP_HEADER_PROVIDER_H_ +#endif // COMPONENTS_VARIATIONS_VARIATIONS_HTTP_HEADER_PROVIDER_H_ diff --git a/components/variations/variations_http_header_provider_unittest.cc b/components/variations/variations_http_header_provider_unittest.cc new file mode 100644 index 0000000..22d5173 --- /dev/null +++ b/components/variations/variations_http_header_provider_unittest.cc @@ -0,0 +1,126 @@ +// Copyright 2014 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "components/variations/variations_http_header_provider.h" + +#include <string> + +#include "base/base64.h" +#include "base/message_loop/message_loop.h" +#include "base/metrics/field_trial.h" +#include "base/run_loop.h" +#include "components/variations/entropy_provider.h" +#include "components/variations/proto/client_variations.pb.h" +#include "components/variations/variations_associated_data.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace variations { + +namespace { + +// Decodes the variations header and extracts the variation ids. +bool ExtractVariationIds(const std::string& variations, + std::set<VariationID>* variation_ids, + std::set<VariationID>* trigger_ids) { + std::string serialized_proto; + if (!base::Base64Decode(variations, &serialized_proto)) + return false; + ClientVariations proto; + if (!proto.ParseFromString(serialized_proto)) + return false; + for (int i = 0; i < proto.variation_id_size(); ++i) + variation_ids->insert(proto.variation_id(i)); + for (int i = 0; i < proto.trigger_variation_id_size(); ++i) + trigger_ids->insert(proto.trigger_variation_id(i)); + return true; +} + +scoped_refptr<base::FieldTrial> CreateTrialAndAssociateId( + const std::string& trial_name, + const std::string& default_group_name, + IDCollectionKey key, + VariationID id) { + scoped_refptr<base::FieldTrial> trial( + base::FieldTrialList::CreateFieldTrial(trial_name, default_group_name)); + + AssociateGoogleVariationID(key, trial->trial_name(), trial->group_name(), id); + + return trial; +} + +} // namespace + +class VariationsHttpHeaderProviderTest : public ::testing::Test { + public: + VariationsHttpHeaderProviderTest() {} + + ~VariationsHttpHeaderProviderTest() override {} + + void TearDown() override { testing::ClearAllVariationIDs(); } +}; + +TEST_F(VariationsHttpHeaderProviderTest, SetDefaultVariationIds_Valid) { + base::MessageLoop loop; + VariationsHttpHeaderProvider provider; + + // Valid experiment ids. + EXPECT_TRUE(provider.SetDefaultVariationIds("12,456,t789")); + provider.InitVariationIDsCacheIfNeeded(); + std::string variations = provider.GetClientDataHeader(); + EXPECT_FALSE(variations.empty()); + std::set<VariationID> variation_ids; + std::set<VariationID> trigger_ids; + ASSERT_TRUE(ExtractVariationIds(variations, &variation_ids, &trigger_ids)); + EXPECT_TRUE(variation_ids.find(12) != variation_ids.end()); + EXPECT_TRUE(variation_ids.find(456) != variation_ids.end()); + EXPECT_TRUE(trigger_ids.find(789) != trigger_ids.end()); + EXPECT_FALSE(variation_ids.find(789) != variation_ids.end()); +} + +TEST_F(VariationsHttpHeaderProviderTest, SetDefaultVariationIds_Invalid) { + base::MessageLoop loop; + VariationsHttpHeaderProvider provider; + + // Invalid experiment ids. + EXPECT_FALSE(provider.SetDefaultVariationIds("abcd12,456")); + provider.InitVariationIDsCacheIfNeeded(); + EXPECT_TRUE(provider.GetClientDataHeader().empty()); + + // Invalid trigger experiment id + EXPECT_FALSE(provider.SetDefaultVariationIds("12,tabc456")); + provider.InitVariationIDsCacheIfNeeded(); + EXPECT_TRUE(provider.GetClientDataHeader().empty()); +} + +TEST_F(VariationsHttpHeaderProviderTest, OnFieldTrialGroupFinalized) { + base::MessageLoop loop; + base::FieldTrialList field_trial_list(nullptr); + VariationsHttpHeaderProvider provider; + provider.InitVariationIDsCacheIfNeeded(); + + const std::string default_name = "default"; + scoped_refptr<base::FieldTrial> trial_1(CreateTrialAndAssociateId( + "t1", default_name, GOOGLE_WEB_PROPERTIES, 123)); + + ASSERT_EQ(default_name, trial_1->group_name()); + + scoped_refptr<base::FieldTrial> trial_2(CreateTrialAndAssociateId( + "t2", default_name, GOOGLE_WEB_PROPERTIES_TRIGGER, 456)); + + ASSERT_EQ(default_name, trial_2->group_name()); + + // Run the message loop to make sure OnFieldTrialGroupFinalized is called for + // the two field trials. + base::RunLoop().RunUntilIdle(); + + std::string variations = provider.GetClientDataHeader(); + + std::set<VariationID> variation_ids; + std::set<VariationID> trigger_ids; + ASSERT_TRUE(ExtractVariationIds(variations, &variation_ids, &trigger_ids)); + EXPECT_TRUE(variation_ids.find(123) != variation_ids.end()); + EXPECT_TRUE(trigger_ids.find(456) != trigger_ids.end()); +} + +} // namespace variations |