Add histograms for network activity, and total/cumulative

net/http/http_cache_transaction.cc

Index: net/http/http_cache_transaction.cc
===================================================================
--- net/http/http_cache_transaction.cc	(revision 150909)
+++ net/http/http_cache_transaction.cc	(working copy)
@@ -745,12 +745,15 @@
   if (rv != OK)
     return rv;
 
+  ReportNetworkActionStart();

[mmenke, 2012/08/15 15:09:53]

I'm a bit concerned about mismatched Report*() calls, especially as the code
gets modified (Not that this is a particularly often modified file).

Have two suggestions for alleviating these concerns:

1)  Have HttpTransactionDelegateImpl DCHECK on unexpected FINISH events.
2)  Or have a HttpCache::Transaction::SetNextState(State) function, which may
calls a Finish() function based on the old value of next_state_, and/or call a
Start() function based on the new value.

[tburkard, 2012/08/15 16:50:46]

I can add that to the HttpTransactionDelegateImpl, however, it will need to keep
some extra state to keep track of it.  Last time, you asked me to remove such
extra state.. So are you ok with that extra state being added to the delegate
this time?  Please advise.

[mmenke, 2012/08/15 18:45:41]

When the state was solely for use elsewhere, I thought it belonged where it was
being used.  When it's being used to ensure correctness, I'm fine with it.  In
case I was being unclear, those were intended to be either-or options.  Doing
both would be overkill.

I'm open to other ideas, or just leaving it as-is, but I do think regressions
here are a real concern, and could easily go unnoticed.

[mmenke, 2012/08/15 19:08:34]

Actually, my real objection is that it was in net/ and assumed the cache
behavior was completely different from what it really was - which seemed to me
like it might be a reasonable assumption in chrome/, but not net/.

[tburkard, 2012/08/15 20:42:24]

Done.

   next_state_ = STATE_SEND_REQUEST_COMPLETE;
   rv = network_trans_->Start(request_, io_callback_, net_log_);
   return rv;
 }
 
 int HttpCache::Transaction::DoSendRequestComplete(int result) {
+  ReportNetworkActionFinish();
+
   if (!cache_)
     return ERR_UNEXPECTED;
 
@@ -839,6 +842,7 @@
 }
 
 int HttpCache::Transaction::DoNetworkRead() {
+  ReportNetworkActionStart();
   next_state_ = STATE_NETWORK_READ_COMPLETE;
   return network_trans_->Read(read_buf_, io_buf_len_, io_callback_);
 }
@@ -846,6 +850,8 @@
 int HttpCache::Transaction::DoNetworkReadComplete(int result) {
   DCHECK(mode_ & WRITE || mode_ == NONE);
 
+  ReportNetworkActionFinish();
+
   if (!cache_)
     return ERR_UNEXPECTED;
 
@@ -1319,7 +1325,7 @@
 
 int HttpCache::Transaction::DoCacheWriteTruncatedResponse() {
   if (entry_) {
-    if (net_log_.IsLoggingAllEvents() && entry_)
+    if (net_log_.IsLoggingAllEvents())
       net_log_.BeginEvent(NetLog::TYPE_HTTP_CACHE_WRITE_INFO);
     ReportCacheActionStart();
   }
@@ -1773,6 +1779,7 @@
   DCHECK(network_trans_.get());
   DCHECK_EQ(STATE_NONE, next_state_);
 
+  ReportNetworkActionStart();
   next_state_ = STATE_SEND_REQUEST_COMPLETE;
   int rv = network_trans_->RestartIgnoringLastError(io_callback_);
   if (rv != ERR_IO_PENDING)
@@ -1786,6 +1793,7 @@
   DCHECK(network_trans_.get());
   DCHECK_EQ(STATE_NONE, next_state_);
 
+  ReportNetworkActionStart();
   next_state_ = STATE_SEND_REQUEST_COMPLETE;
   int rv = network_trans_->RestartWithCertificate(client_cert, io_callback_);
   if (rv != ERR_IO_PENDING)
@@ -1799,6 +1807,7 @@
   DCHECK(network_trans_.get());
   DCHECK_EQ(STATE_NONE, next_state_);
 
+  ReportNetworkActionStart();
   next_state_ = STATE_SEND_REQUEST_COMPLETE;
   int rv = network_trans_->RestartWithAuth(credentials, io_callback_);
   if (rv != ERR_IO_PENDING)
@@ -2083,6 +2092,11 @@
        response_.headers->HasHeaderValue("cache-control", "no-store")) ||
       net::IsCertStatusError(response_.ssl_info.cert_status)) {
     DoneWritingToEntry(false);
+    ReportCacheActionFinish();
+    if (net_log_.IsLoggingAllEvents()) {
+      net_log_.EndEventWithNetErrorCode(NetLog::TYPE_HTTP_CACHE_WRITE_INFO,
+                                        OK);

[mmenke, 2012/08/15 15:09:53]

optional nit:  Can just use EndEvent.  "OK" is ignored, to avoid spamming it all
over the log.

[tburkard, 2012/08/15 16:50:46]

Done.

+    }
     return OK;
   }
 
@@ -2232,6 +2246,16 @@
     transaction_delegate_->OnCacheActionFinish();
 }
 
+void HttpCache::Transaction::ReportNetworkActionStart() {
+  if (transaction_delegate_)
+    transaction_delegate_->OnNetworkActionStart();
+}
+
+void HttpCache::Transaction::ReportNetworkActionFinish() {
+  if (transaction_delegate_)
+    transaction_delegate_->OnNetworkActionFinish();
+}
+
 void HttpCache::Transaction::UpdateTransactionPattern(
     TransactionPattern new_transaction_pattern) {
   if (transaction_pattern_ == PATTERN_NOT_COVERED)