Switch variations http headers to only be reported over https.

components/variations/net/variations_http_headers.cc

 // 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 <stddef.h>
 
 #include "base/macros.h"
+#include "base/metrics/histogram_macros.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 {
 
@@ skipping 12 matching lines @@
 };
 
 // Exact hostnames in lowercase to set headers for.
 const char* kHostsToSetHeadersFor[] = {
     "googleweblight.com",
 };
 
 const char kChromeUMAEnabled[] = "X-Chrome-UMA-Enabled";
 const char kClientData[] = "X-Client-Data";
 
+// The result of checking if a URL should have variations headers appended.
+// This enum is used to record UMA histogram values, and should not be
+// reordered.
+enum ShouldAppendHeadersResult {
+  INVALID_URL,
+  NOT_HTTPS,
+  NOT_GOOGLE_DOMAIN,
+  SHOULD_APPEND,
+  SHOULD_APPEND_HEADERS_RESULT_SIZE,
+};
+
+// Checks whether headers should be appended to the |url|, based on the domain
+// of |url|. |url| is assumed to be valid, and to have the https scheme.
+bool ShouldAppendForDomain(const GURL& url) {

[rkaplow, 2016/12/12 19:31:17]

actually don't super like this name, kind of prefer IsGoogleDomain or something.
up to you

[jwd, 2016/12/12 19:52:54]

Done.

+  if (google_util::IsGoogleDomainUrl(url, google_util::ALLOW_SUBDOMAIN,
+                                     google_util::ALLOW_NON_STANDARD_PORTS)) {
+    return true;
+  }
+  if (google_util::IsYoutubeDomainUrl(url, google_util::ALLOW_SUBDOMAIN,
+                                      google_util::ALLOW_NON_STANDARD_PORTS)) {
+    return true;
+  }
+
+  // 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;
+  }
+  for (size_t i = 0; i < arraysize(kHostsToSetHeadersFor); ++i) {
+    if (base::LowerCaseEqualsASCII(host, kHostsToSetHeadersFor[i]))
+      return true;
+  }
+
+  return false;
+}
+
 }  // 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
@@ skipping 21 matching lines @@
   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;
+  const char kShouldAppendResultHistogram[] =

[rkaplow, 2016/12/12 19:31:18]

nit - could rename this

[jwd, 2016/12/12 19:52:54]

Done.

+      "Variations.Headers.URLValidationResult";
+  if (!url.is_valid()) {
+    UMA_HISTOGRAM_ENUMERATION(kShouldAppendResultHistogram, INVALID_URL);
+    return false;
+  }
+  if (url.SchemeIs("https")) {

[rkaplow, 2016/12/12 19:31:18]

!

[jwd, 2016/12/12 19:52:54]

Done.

+    UMA_HISTOGRAM_ENUMERATION(kShouldAppendResultHistogram, NOT_HTTPS);
+    return false;
+  }
+  if (!ShouldAppendForDomain(url)) {
+    UMA_HISTOGRAM_ENUMERATION(kShouldAppendResultHistogram, NOT_GOOGLE_DOMAIN,
+                              SHOULD_APPEND_HEADERS_RESULT_SIZE);
+    return false;
   }
 
-  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;
-  }
-  for (size_t i = 0; i < arraysize(kHostsToSetHeadersFor); ++i) {
-    if (base::LowerCaseEqualsASCII(host, kHostsToSetHeadersFor[i]))
-      return true;
-  }
-
-  return google_util::IsYoutubeDomainUrl(url, google_util::ALLOW_SUBDOMAIN,
-                                         google_util::ALLOW_NON_STANDARD_PORTS);
+  UMA_HISTOGRAM_ENUMERATION(kShouldAppendResultHistogram, SHOULD_APPEND,
+                            SHOULD_APPEND_HEADERS_RESULT_SIZE);
+  return true;
 }
 
 }  // namespace internal
 
 }  // namespace variations