Keep track of ChannelIDService in CookieStore

net/url_request/url_request_http_job.cc

Index: net/url_request/url_request_http_job.cc
diff --git a/net/url_request/url_request_http_job.cc b/net/url_request/url_request_http_job.cc
index ccc38d544ead9009417fdda7e4b7595c53defc7e..8c2f7f708d92f9b6116553a2e12a1dd67681b931 100644
--- a/net/url_request/url_request_http_job.cc
+++ b/net/url_request/url_request_http_job.cc
@@ -82,6 +82,8 @@ void LogChannelIDAndCookieStores(const net::URLRequestContext* context,
     EPHEMERAL_MISMATCH = 8,
     PERSISTENT_MATCH = 9,
     PERSISTENT_MISMATCH = 10,
+    EPHEMERAL_UNKNOWN = 11,
+    PERSISTENT_UNKNOWN = 12,
     EPHEMERALITY_MAX
   } ephemerality;
   const net::HttpNetworkSession::Params* params =
@@ -93,9 +95,10 @@ void LogChannelIDAndCookieStores(const net::URLRequestContext* context,
     ephemerality = NO_COOKIE_STORE;
   } else if (params->channel_id_service->GetChannelIDStore()->IsEphemeral()) {
     if (cookie_store->IsEphemeral()) {
-      if (context->channel_id_service() &&
-          params->channel_id_service->GetUniqueID() ==
-              context->channel_id_service()->GetUniqueID()) {
+      if (cookie_store->GetChannelIDServiceID() == -1) {
+        ephemerality = EPHEMERAL_UNKNOWN;
+      } else if (cookie_store->GetChannelIDServiceID() ==
+                 params->channel_id_service->GetUniqueID()) {
         ephemerality = EPHEMERAL_MATCH;
       } else {
         ephemerality = EPHEMERAL_MISMATCH;

[mmenke, 2016/03/28 17:54:00]

If this happens, it is a bug.  Should we really be logging bugs in histograms
instead of, say, DCHECKing when this happens?  If we can't DCHECK on
URLRequestContext initialization (Though I'd really prefer if we figure out a
way to do that), maybe DCHECK in URLRequestContext::CreateRequest?

I'm fine with doing both, but I really don't think we want to keep a histogram
about whether a URLRequestContext was created correctly, long term it's just
weird, and puts the burden of ensuring proper URLRequestContext creation on the
wrong team.

[nharper, 2016/03/29 00:30:20]

I don't see any reason why we can't DCHECK.
I'm assuming that DCHECKs don't fire on canary/dev builds, so I've added them to
the 4 cases that should never happen. I think doing both would be a good
short-term step, but I think long-term this histogram should go away. (Ideally
this whole method would go, but then there's no place to leave the DCHECKs.)

@@ -107,9 +110,10 @@ void LogChannelIDAndCookieStores(const net::URLRequestContext* context,
     }
   } else if (cookie_store->IsEphemeral()) {
     ephemerality = CID_PERSISTENT_COOKIE_EPHEMERAL;
-  } else if (context->channel_id_service() &&
-             params->channel_id_service->GetUniqueID() ==
-                 context->channel_id_service()->GetUniqueID()) {
+  } else if (cookie_store->GetChannelIDServiceID() == -1) {
+    ephemerality = PERSISTENT_UNKNOWN;
+  } else if (cookie_store->GetChannelIDServiceID() ==
+             params->channel_id_service->GetUniqueID()) {
     ephemerality = PERSISTENT_MATCH;
   } else {
     ephemerality = PERSISTENT_MISMATCH;