Commit a230d7d2 authored by kkania@chromium.org's avatar kkania@chromium.org

Revert 134625 - Transmit a X-Chrome-UMA-Enabled bit to Google domains from...

Revert 134625 - Transmit a X-Chrome-UMA-Enabled bit to Google domains from clients that have UMA enabled. Causing startup crash on Linux chromeos bot.

This requires a change to the ChromeNetworkDelegate where we feed the incognito state (a bool) into the object at creation time, so we can check that field when doing our header setting.

BUG=123609
TEST=With UMA enabled (not Chromium), visit www.google.com and ensure that the request header includes X-Chrome-UMA-Enabled with value "1". Ensure that disabling UMA also disables the transmission of this header entirely.  Also, ensure that unit_tests GoogleUtilTests all pass.
TBR=ivankr@chromium.org

Review URL: http://codereview.chromium.org/10108026

TBR=stevet@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10264023

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@134641 0039d316-1c4b-4281-b951-d872f2087c98
parent 80afb7d6
......@@ -44,6 +44,13 @@ bool HasQueryParameter(const std::string& str) {
return false;
}
// True if |url| is an HTTP[S] request with host "[www.]google.<TLD>" and no
// explicit port.
bool IsGoogleDomainUrl(const GURL& url) {
return url.is_valid() && (url.SchemeIs("http") || url.SchemeIs("https")) &&
url.port().empty() && google_util::IsGoogleHostname(url.host());
}
} // anonymous namespace
namespace google_util {
......@@ -135,32 +142,21 @@ bool GetReactivationBrand(std::string* brand) {
#endif
bool IsGoogleDomainUrl(const std::string& url, SubdomainPermission permission) {
GURL original_url(url);
return original_url.is_valid() && original_url.port().empty() &&
(original_url.SchemeIs("http") || original_url.SchemeIs("https")) &&
google_util::IsGoogleHostname(original_url.host(), permission);
}
bool IsGoogleHostname(const std::string& host,
SubdomainPermission permission) {
bool IsGoogleHostname(const std::string& host) {
size_t tld_length =
net::RegistryControlledDomainService::GetRegistryLength(host, false);
if ((tld_length == 0) || (tld_length == std::string::npos))
return false;
std::string host_minus_tld(host, 0, host.length() - tld_length);
if (LowerCaseEqualsASCII(host_minus_tld, "google."))
return true;
if (permission == ALLOW_SUBDOMAIN)
return EndsWith(host_minus_tld, ".google.", false);
return LowerCaseEqualsASCII(host_minus_tld, "www.google.");
return LowerCaseEqualsASCII(host_minus_tld, "www.google.") ||
LowerCaseEqualsASCII(host_minus_tld, "google.");
}
bool IsGoogleHomePageUrl(const std::string& url) {
GURL original_url(url);
// First check to see if this has a Google domain.
if (!IsGoogleDomainUrl(url, DISALLOW_SUBDOMAIN))
if (!IsGoogleDomainUrl(original_url))
return false;
// Make sure the path is a known home page path.
......@@ -177,7 +173,7 @@ bool IsGoogleSearchUrl(const std::string& url) {
GURL original_url(url);
// First check to see if this has a Google domain.
if (!IsGoogleDomainUrl(url, DISALLOW_SUBDOMAIN))
if (!IsGoogleDomainUrl(original_url))
return false;
// Make sure the path is a known search path.
......
......@@ -46,21 +46,8 @@ bool GetReactivationBrand(std::string* brand);
// need to restrict some behavior to only happen on Google's officially-owned
// domains, use TransportSecurityState::IsGooglePinnedProperty() instead.
// Designate whether or not a URL checking function also checks for specific
// subdomains, or only "www" and empty subdomains.
enum SubdomainPermission {
ALLOW_SUBDOMAIN,
DISALLOW_SUBDOMAIN,
};
// True if |url| is an HTTP[S] request with host "[www.]google.<TLD>" and no
// explicit port. If |permission| is ALLOW_SUBDOMAIN, we check against host
// "*.google.<TLD>" instead.
bool IsGoogleDomainUrl(const std::string& url, SubdomainPermission permission);
// True if |host| is "[www.]google.<TLD>" with a valid TLD. If
// |permission| is ALLOW_SUBDOMAIN, we check against host "*.google.<TLD>"
// instead.
bool IsGoogleHostname(const std::string& host, SubdomainPermission permission);
// True if |host| is "[www.]google.<TLD>" with a valid TLD.
bool IsGoogleHostname(const std::string& host);
// True if |url| represents a valid Google home page URL.
bool IsGoogleHomePageUrl(const std::string& url);
// True if |url| represents a valid Google search URL.
......
......@@ -6,7 +6,6 @@
#include "chrome/browser/google/google_util.h"
#include "testing/gtest/include/gtest/gtest.h"
using google_util::IsGoogleDomainUrl;
using google_util::IsGoogleHomePageUrl;
using google_util::IsGoogleSearchUrl;
......@@ -242,51 +241,3 @@ TEST(GoogleUtilTest, BadSearches) {
EXPECT_FALSE(IsGoogleSearchUrl(
"http://www.google.com/WEBHP#q=something"));
}
TEST(GoogleUtilTest, GoogleDomains) {
// Test some good Google domains (valid TLDs).
EXPECT_TRUE(IsGoogleDomainUrl("http://www.google.com",
google_util::ALLOW_SUBDOMAIN));
EXPECT_TRUE(IsGoogleDomainUrl("http://google.com",
google_util::ALLOW_SUBDOMAIN));
EXPECT_TRUE(IsGoogleDomainUrl("http://www.google.ca",
google_util::ALLOW_SUBDOMAIN));
EXPECT_TRUE(IsGoogleDomainUrl("http://www.google.biz.tj",
google_util::ALLOW_SUBDOMAIN));
EXPECT_TRUE(IsGoogleDomainUrl("http://www.google.com/search?q=something",
google_util::ALLOW_SUBDOMAIN));
EXPECT_TRUE(IsGoogleDomainUrl("http://www.google.com/webhp",
google_util::ALLOW_SUBDOMAIN));
// Test some bad Google domains (invalid TLDs).
EXPECT_FALSE(IsGoogleDomainUrl("http://www.google.notrealtld",
google_util::ALLOW_SUBDOMAIN));
EXPECT_FALSE(IsGoogleDomainUrl("http://www.google.faketld/search?q=something",
google_util::ALLOW_SUBDOMAIN));
EXPECT_FALSE(IsGoogleDomainUrl("http://www.yahoo.com",
google_util::ALLOW_SUBDOMAIN));
// Test subdomain checks.
EXPECT_TRUE(IsGoogleDomainUrl("http://images.google.com",
google_util::ALLOW_SUBDOMAIN));
EXPECT_FALSE(IsGoogleDomainUrl("http://images.google.com",
google_util::DISALLOW_SUBDOMAIN));
EXPECT_TRUE(IsGoogleDomainUrl("http://google.com",
google_util::DISALLOW_SUBDOMAIN));
EXPECT_TRUE(IsGoogleDomainUrl("http://www.google.com",
google_util::DISALLOW_SUBDOMAIN));
// Port and scheme checks.
EXPECT_TRUE(IsGoogleDomainUrl("http://www.google.com:80",
google_util::DISALLOW_SUBDOMAIN));
EXPECT_FALSE(IsGoogleDomainUrl("http://www.google.com:123",
google_util::DISALLOW_SUBDOMAIN));
EXPECT_TRUE(IsGoogleDomainUrl("https://www.google.com:443",
google_util::DISALLOW_SUBDOMAIN));
EXPECT_FALSE(IsGoogleDomainUrl("https://www.google.com:123",
google_util::DISALLOW_SUBDOMAIN));
EXPECT_FALSE(IsGoogleDomainUrl("file://www.google.com",
google_util::DISALLOW_SUBDOMAIN));
EXPECT_FALSE(IsGoogleDomainUrl("doesnotexist://www.google.com",
google_util::DISALLOW_SUBDOMAIN));
}
......@@ -113,10 +113,6 @@ void ProfileImplIOData::Handle::Init(
local_state,
io_thread,
main_request_context_getter_);
io_data_->enable_metrics()->Init(prefs::kMetricsReportingEnabled,
local_state, NULL);
io_data_->enable_metrics()->MoveToThread(BrowserThread::IO);
}
base::Callback<ChromeURLDataManagerBackend*(void)>
......
......@@ -160,6 +160,7 @@ void ProfileIOData::InitializeOnUIThread(Profile* profile) {
scoped_ptr<ProfileParams> params(new ProfileParams);
params->path = profile->GetPath();
params->is_incognito = profile->IsOffTheRecord();
params->clear_local_state_on_exit =
pref_service->GetBoolean(prefs::kClearSiteDataOnExit);
......@@ -246,7 +247,8 @@ void ProfileIOData::AppRequestContext::SetHttpTransactionFactory(
ProfileIOData::AppRequestContext::~AppRequestContext() {}
ProfileIOData::ProfileParams::ProfileParams()
: clear_local_state_on_exit(false),
: is_incognito(false),
clear_local_state_on_exit(false),
io_thread(NULL),
#if defined(ENABLE_NOTIFICATIONS)
notification_service(NULL),
......@@ -260,8 +262,7 @@ ProfileIOData::ProfileIOData(bool is_incognito)
: initialized_(false),
ALLOW_THIS_IN_INITIALIZER_LIST(
resource_context_(new ResourceContext(this))),
initialized_on_UI_thread_(false),
is_incognito_(is_incognito) {
initialized_on_UI_thread_(false) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
}
......@@ -464,7 +465,7 @@ void ProfileIOData::LazyInitialize() const {
transport_security_persister_.reset(
new TransportSecurityPersister(transport_security_state_.get(),
profile_params_->path,
is_incognito()));
profile_params_->is_incognito));
// NOTE(willchan): Keep these protocol handlers in sync with
// ProfileIOData::IsHandledProtocol().
......@@ -476,7 +477,7 @@ void ProfileIOData::LazyInitialize() const {
}
bool set_protocol = job_factory_->SetProtocolHandler(
chrome::kExtensionScheme,
CreateExtensionProtocolHandler(is_incognito(),
CreateExtensionProtocolHandler(profile_params_->is_incognito,
profile_params_->extension_info_map));
DCHECK(set_protocol);
set_protocol = job_factory_->SetProtocolHandler(
......@@ -514,7 +515,7 @@ void ProfileIOData::LazyInitialize() const {
void ProfileIOData::ApplyProfileParamsToContext(
ChromeURLRequestContext* context) const {
context->set_is_incognito(is_incognito());
context->set_is_incognito(profile_params_->is_incognito);
context->set_accept_language(profile_params_->accept_language);
context->set_accept_charset(profile_params_->accept_charset);
context->set_referrer_charset(profile_params_->referrer_charset);
......@@ -524,7 +525,6 @@ void ProfileIOData::ApplyProfileParamsToContext(
void ProfileIOData::ShutdownOnUIThread() {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
enable_referrers_.Destroy();
enable_metrics_.Destroy();
clear_local_state_on_exit_.Destroy();
safe_browsing_enabled_.Destroy();
session_startup_pref_.Destroy();
......
......@@ -107,10 +107,6 @@ class ProfileIOData {
return &safe_browsing_enabled_;
}
BooleanPrefMember* enable_metrics() const {
return &enable_metrics_;
}
net::TransportSecurityState* transport_security_state() const {
return transport_security_state_.get();
}
......@@ -118,10 +114,6 @@ class ProfileIOData {
chrome_browser_net::HttpServerPropertiesManager*
http_server_properties_manager() const;
bool is_incognito() const {
return is_incognito_;
}
protected:
class AppRequestContext : public ChromeURLRequestContext {
public:
......@@ -144,6 +136,7 @@ class ProfileIOData {
~ProfileParams();
FilePath path;
bool is_incognito;
bool clear_local_state_on_exit;
std::string accept_language;
std::string accept_charset;
......@@ -297,7 +290,6 @@ class ProfileIOData {
// Member variables which are pointed to by the various context objects.
mutable BooleanPrefMember enable_referrers_;
mutable BooleanPrefMember enable_metrics_;
mutable BooleanPrefMember clear_local_state_on_exit_;
mutable BooleanPrefMember safe_browsing_enabled_;
// TODO(marja): Remove session_startup_pref_ if no longer needed.
......@@ -341,8 +333,6 @@ class ProfileIOData {
// TODO(jhawkins): Remove once crbug.com/102004 is fixed.
bool initialized_on_UI_thread_;
bool is_incognito_;
DISALLOW_COPY_AND_ASSIGN(ProfileIOData);
};
......
......@@ -26,11 +26,8 @@ namespace {
bool CanMerge(const GURL& url1, const GURL& url2) {
VLOG(1) << "Checking if can merge " << url1.spec() << " with " << url2.spec();
// All Google URLs are considered the same one.
if (google_util::IsGoogleHostname(url1.host(),
google_util::DISALLOW_SUBDOMAIN)) {
return google_util::IsGoogleHostname(url2.host(),
google_util::DISALLOW_SUBDOMAIN);
}
if (google_util::IsGoogleHostname(url1.host()))
return google_util::IsGoogleHostname(url2.host());
// Otherwise URLs must have the same domain.
return net::RegistryControlledDomainService::SameDomainOrHost(url1, url2);
}
......
......@@ -12,7 +12,6 @@
#include "chrome/browser/download/download_util.h"
#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/instant/instant_loader.h"
#include "chrome/browser/net/load_timing_observer.h"
#include "chrome/browser/prerender/prerender_manager.h"
......@@ -164,8 +163,6 @@ void ChromeResourceDispatcherHostDelegate::RequestBeginning(
#endif
}
AppendChromeMetricsHeaders(request, resource_context, resource_type);
AppendStandardResourceThrottles(request,
resource_context,
child_id,
......@@ -296,28 +293,6 @@ void ChromeResourceDispatcherHostDelegate::AppendStandardResourceThrottles(
throttles->push_back(throttle);
}
void ChromeResourceDispatcherHostDelegate::AppendChromeMetricsHeaders(
net::URLRequest* request,
content::ResourceContext* resource_context,
ResourceType::Type resource_type) {
// Note our 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
// Google-owned, only that it is of the form *.google.<TLD>. In the future
// we may choose to reinforce this check.
// 2. We only transmit for non-Incognito profiles.
// 3. For the X-Chrome-UMA-Enabled bit, we only set it if UMA is in fact
// enabled for this install of Chrome.
ProfileIOData* io_data = ProfileIOData::FromResourceContext(resource_context);
if (google_util::IsGoogleDomainUrl(request->url().spec(),
google_util::ALLOW_SUBDOMAIN) &&
!io_data->is_incognito() && io_data->enable_metrics()->GetValue()) {
request->SetExtraRequestHeaderByName("X-Chrome-UMA-Enabled",
"1",
false);
}
}
bool ChromeResourceDispatcherHostDelegate::ShouldForceDownloadResource(
const GURL& url, const std::string& mime_type) {
// Special-case user scripts to get downloaded instead of viewed.
......
......@@ -87,14 +87,6 @@ class ChromeResourceDispatcherHostDelegate
ResourceType::Type resource_type,
ScopedVector<content::ResourceThrottle>* throttles);
// Adds Chrome experiment and metrics state as custom headers to |request|.
// This is a best-effort attempt, and does not interrupt the request if the
// headers can not be appended.
void AppendChromeMetricsHeaders(
net::URLRequest* request,
content::ResourceContext* resource_context,
ResourceType::Type resource_type);
scoped_refptr<DownloadRequestLimiter> download_request_limiter_;
scoped_refptr<SafeBrowsingService> safe_browsing_;
scoped_refptr<UserScriptListener> user_script_listener_;
......
......@@ -3284,8 +3284,7 @@ static const PrepopulatedEngine* GetEngineForURL(const std::string& url) {
// First special-case Google, because the prepopulate URL for it will not
// convert to a GURL and thus won't have an origin. Instead see if the
// incoming URL's host is "[*.]google.<TLD>".
if (google_util::IsGoogleHostname(as_gurl.host(),
google_util::DISALLOW_SUBDOMAIN))
if (google_util::IsGoogleHostname(as_gurl.host()))
return &google;
// Now check the rest of the prepopulate data.
......
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