Add cookie prefix metrics

net/cookies/canonical_cookie.cc

Index: net/cookies/canonical_cookie.cc
diff --git a/net/cookies/canonical_cookie.cc b/net/cookies/canonical_cookie.cc
index ddf35e0531d48dd08db1624b647e0bbc720b391e..57ab39604fc10360127ae6c95699d9202e2832fc 100644
--- a/net/cookies/canonical_cookie.cc
+++ b/net/cookies/canonical_cookie.cc
@@ -47,6 +47,7 @@
 #include "base/basictypes.h"
 #include "base/format_macros.h"
 #include "base/logging.h"
+#include "base/metrics/histogram_macros.h"
 #include "base/strings/stringprintf.h"
 #include "net/cookies/cookie_util.h"
 #include "net/cookies/parsed_cookie.h"
@@ -116,21 +117,6 @@ int PartialCookieOrdering(const CanonicalCookie& a, const CanonicalCookie& b) {
   return a.Path().compare(b.Path());
 }
 
-// Returns true if the cookie does not violate any constraints imposed
-// by the cookie name's prefix, as described in
-// https://tools.ietf.org/html/draft-west-cookie-prefixes
-bool IsCookiePrefixValid(const GURL& url, const ParsedCookie& parsed_cookie) {
-  const char kSecurePrefix[] = "$Secure-";
-  const char kHostPrefix[] = "$Host-";
-  if (parsed_cookie.Name().find(kSecurePrefix) == 0)
-    return parsed_cookie.IsSecure() && url.SchemeIsCryptographic();
-  if (parsed_cookie.Name().find(kHostPrefix) == 0) {
-    return parsed_cookie.IsSecure() && url.SchemeIsCryptographic() &&
-           !parsed_cookie.HasDomain() && parsed_cookie.Path() == "/";
-  }
-  return true;
-}
-
 }  // namespace
 
 CanonicalCookie::CanonicalCookie()
@@ -263,7 +249,11 @@ CanonicalCookie* CanonicalCookie::Create(const GURL& url,
                                                          creation_time,
                                                          server_time);
 
-  if (options.enforce_prefixes() && !IsCookiePrefixValid(url, parsed_cookie)) {
+  CookiePrefix prefix = CanonicalCookie::GetCookiePrefix(parsed_cookie.Name());
+  bool is_cookie_valid =
+      CanonicalCookie::IsCookiePrefixValid(prefix, url, parsed_cookie);
+  CanonicalCookie::RecordCookiePrefixMetrics(prefix, is_cookie_valid);
+  if (options.enforce_prefixes() && !is_cookie_valid) {
     VLOG(kVlogSetCookies)
         << "Create() failed because the cookie violated prefix rules.";
     return nullptr;
@@ -469,4 +459,47 @@ bool CanonicalCookie::FullCompare(const CanonicalCookie& other) const {
   return Priority() < other.Priority();
 }
 
+// static
+CanonicalCookie::CookiePrefix CanonicalCookie::GetCookiePrefix(
+    const std::string& name) {
+  const char kSecurePrefix[] = "$Secure-";
+  const char kHostPrefix[] = "$Host-";
+  if (name.find(kSecurePrefix) == 0)
+    return CanonicalCookie::COOKIE_PREFIX_SECURE;
+  if (name.find(kHostPrefix) == 0)
+    return CanonicalCookie::COOKIE_PREFIX_HOST;
+  return CanonicalCookie::COOKIE_PREFIX_NONE;
+}
+
+// static
+void CanonicalCookie::RecordCookiePrefixMetrics(
+    CanonicalCookie::CookiePrefix prefix,
+    bool is_cookie_valid) {
+  const char kCookiePrefixHistogram[] = "Cookie.CookiePrefix";
+  const char kCookiePrefixBlockedHistogram[] = "Cookie.CookiePrefixBlocked";
+  UMA_HISTOGRAM_ENUMERATION(kCookiePrefixHistogram, prefix,
+                            CanonicalCookie::COOKIE_PREFIX_LAST);
+  if (!is_cookie_valid) {
+    UMA_HISTOGRAM_ENUMERATION(kCookiePrefixBlockedHistogram, prefix,
+                              CanonicalCookie::COOKIE_PREFIX_LAST);

[Mike West, 2015/11/20 14:43:58]

It might be easier to compare numbers if these were all in one histogram. That
is, "Of all the cookies we saw, X had this prefix and would have been blocked. Y
had this prefix and would have been allowed. Z had no prefix."

I'm totally fine with gathering that information out of two histograms; it's
easy, just marginally more annoying than having all the data in one place.

[estark, 2015/11/20 18:19:32]

It might be because I started trying to write code at 6am but doing it in the
same histogram seemed like it led to slightly uglier code, so I decided to go
for cleaner code at the expense of slightly more annoying histogram.

+  }
+}
+
+// Returns true if the cookie does not violate any constraints imposed
+// by the cookie name's prefix, as described in
+// https://tools.ietf.org/html/draft-west-cookie-prefixes
+//
+// static
+bool CanonicalCookie::IsCookiePrefixValid(CanonicalCookie::CookiePrefix prefix,
+                                          const GURL& url,
+                                          const ParsedCookie& parsed_cookie) {
+  if (prefix == CanonicalCookie::COOKIE_PREFIX_SECURE)
+    return parsed_cookie.IsSecure() && url.SchemeIsCryptographic();
+  if (prefix == CanonicalCookie::COOKIE_PREFIX_HOST) {
+    return parsed_cookie.IsSecure() && url.SchemeIsCryptographic() &&
+           !parsed_cookie.HasDomain() && parsed_cookie.Path() == "/";
+  }
+  return true;
+}
+
 }  // namespace net