Switch variations http headers to only be reported over https.

components/variations/net/variations_http_headers.cc

Index: components/variations/net/variations_http_headers.cc
diff --git a/components/variations/net/variations_http_headers.cc b/components/variations/net/variations_http_headers.cc
index 48e84eb190c96f0f8d1ea955ddda215676d9d206..bf9b7504ca4845c8ed7268e420ea096ec0d2e016 100644
--- a/components/variations/net/variations_http_headers.cc
+++ b/components/variations/net/variations_http_headers.cc
@@ -7,6 +7,7 @@
 #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"
@@ -39,6 +40,45 @@ const char* kHostsToSetHeadersFor[] = {
 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,
@@ -80,29 +120,25 @@ 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.is_valid() || !url.SchemeIsHTTPOrHTTPS())
+  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;
-
-  // 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;
+  if (!ShouldAppendForDomain(url)) {
+    UMA_HISTOGRAM_ENUMERATION(kShouldAppendResultHistogram, NOT_GOOGLE_DOMAIN,
+                              SHOULD_APPEND_HEADERS_RESULT_SIZE);
+    return false;
   }
 
-  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