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..304b2eabe2cdea31f897f62f5437ee3f09d5ee53 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 URLValidationResult {
+  INVALID_URL,
+  NOT_HTTPS,
+  NOT_GOOGLE_DOMAIN,
+  SHOULD_APPEND,
+  URL_VALIDATION_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 IsGoogleDomain(const GURL& url) {
+  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,27 @@ 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 kURLValidationResultHistogram[] =
+      "Variations.Headers.URLValidationResult";
+  if (!url.is_valid()) {
+    UMA_HISTOGRAM_ENUMERATION(kURLValidationResultHistogram, INVALID_URL,
+                              URL_VALIDATION_RESULT_SIZE);
+    return false;
   }
-
-  if (!url.is_valid() || !url.SchemeIsHTTPOrHTTPS())
+  if (!url.SchemeIs("https")) {
+    UMA_HISTOGRAM_ENUMERATION(kURLValidationResultHistogram, NOT_HTTPS,
+                              URL_VALIDATION_RESULT_SIZE);
     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 (!IsGoogleDomain(url)) {
+    UMA_HISTOGRAM_ENUMERATION(kURLValidationResultHistogram, NOT_GOOGLE_DOMAIN,
+                              URL_VALIDATION_RESULT_SIZE);
+    return false;
   }
 
-  return google_util::IsYoutubeDomainUrl(url, google_util::ALLOW_SUBDOMAIN,
-                                         google_util::ALLOW_NON_STANDARD_PORTS);
+  UMA_HISTOGRAM_ENUMERATION(kURLValidationResultHistogram, SHOULD_APPEND,
+                            URL_VALIDATION_RESULT_SIZE);

[Alexei Svitkine (slow), 2016/12/12 20:41:16]

Nit: Make a helper function for logging this - since each macro expands to a lot
of code. Once you do that, you can inline the string constant into that helper.

[jwd, 2016/12/12 20:58:09]

Done.

+  return true;
 }
 
 }  // namespace internal