Add histograms for network activity, and total/cumulative

net/http/http_cache_unittest.cc

Index: net/http/http_cache_unittest.cc
===================================================================
--- net/http/http_cache_unittest.cc	(revision 150909)
+++ net/http/http_cache_unittest.cc	(working copy)
@@ -62,28 +62,48 @@
 
 class TestHttpTransactionDelegate : public net::HttpTransactionDelegate {
  public:
-  explicit TestHttpTransactionDelegate(int num_actions_to_observe)
-      : num_remaining_actions_to_observe_(num_actions_to_observe),
-        action_in_progress_(false) {
+  TestHttpTransactionDelegate(int num_cache_actions_to_observe,
+                              int num_network_actions_to_observe)
+      : num_remaining_cache_actions_to_observe_(num_cache_actions_to_observe),
+        num_remaining_network_actions_to_observe_(
+            num_network_actions_to_observe),
+        cache_action_in_progress_(false),
+        network_action_in_progress_(false) {
   }
   virtual ~TestHttpTransactionDelegate() {
-    EXPECT_EQ(0, num_remaining_actions_to_observe_);
-    EXPECT_FALSE(action_in_progress_);
+    EXPECT_EQ(0, num_remaining_cache_actions_to_observe_);
+    EXPECT_EQ(0, num_remaining_network_actions_to_observe_);
+    EXPECT_FALSE(cache_action_in_progress_);
+    EXPECT_FALSE(network_action_in_progress_);
   }
   virtual void OnCacheActionStart() {
-    EXPECT_FALSE(action_in_progress_);
-    EXPECT_GT(num_remaining_actions_to_observe_, 0);
-    num_remaining_actions_to_observe_--;
-    action_in_progress_ = true;
+    EXPECT_FALSE(cache_action_in_progress_);
+    EXPECT_FALSE(network_action_in_progress_);
+    EXPECT_GT(num_remaining_cache_actions_to_observe_, 0);
+    num_remaining_cache_actions_to_observe_--;
+    cache_action_in_progress_ = true;
   }
   virtual void OnCacheActionFinish() {
-    EXPECT_TRUE(action_in_progress_);
-    action_in_progress_ = false;
+    EXPECT_TRUE(cache_action_in_progress_);
+    cache_action_in_progress_ = false;
   }
+  virtual void OnNetworkActionStart() {
+    EXPECT_FALSE(cache_action_in_progress_);
+    EXPECT_FALSE(network_action_in_progress_);
+    EXPECT_GT(num_remaining_network_actions_to_observe_, 0);
+    num_remaining_network_actions_to_observe_--;
+    network_action_in_progress_ = true;
+  }
+  virtual void OnNetworkActionFinish() {
+    EXPECT_TRUE(network_action_in_progress_);
+    network_action_in_progress_ = false;
+  }
 
  private:
-  int num_remaining_actions_to_observe_;
-  bool action_in_progress_;
+  int num_remaining_cache_actions_to_observe_;
+  int num_remaining_network_actions_to_observe_;
+  bool cache_action_in_progress_;
+  bool network_action_in_progress_;
 };
 
 void ReadAndVerifyTransaction(net::HttpTransaction* trans,
@@ -104,14 +124,18 @@
     const MockHttpRequest& request,
     net::HttpResponseInfo* response_info,
     const net::BoundNetLog& net_log,
-    int num_delegate_actions) {
+    int num_cache_delegate_actions,
+    int num_network_delegate_actions) {
   net::TestCompletionCallback callback;
 
   // write to the cache
 
   scoped_ptr<TestHttpTransactionDelegate> delegate;
-  if (num_delegate_actions != kNoDelegateTransactionCheck) {
-    delegate.reset(new TestHttpTransactionDelegate(num_delegate_actions));
+  if (num_cache_delegate_actions != kNoDelegateTransactionCheck &&
+      num_network_delegate_actions != kNoDelegateTransactionCheck) {
+    delegate.reset(
+        new TestHttpTransactionDelegate(num_cache_delegate_actions,
+                                        num_network_delegate_actions));
   }
   scoped_ptr<net::HttpTransaction> trans;
   int rv = cache->CreateTransaction(&trans, delegate.get());
@@ -138,7 +162,7 @@
                                    net::HttpResponseInfo* response_info) {
   RunTransactionTestWithRequestAndLogAndDelegate(
       cache, trans_info, request, response_info, net::BoundNetLog(),
-      kNoDelegateTransactionCheck);
+      kNoDelegateTransactionCheck, kNoDelegateTransactionCheck);
 }
 
 void RunTransactionTestWithLog(net::HttpCache* cache,
@@ -146,15 +170,16 @@
                                const net::BoundNetLog& log) {
   RunTransactionTestWithRequestAndLogAndDelegate(
       cache, trans_info, MockHttpRequest(trans_info), NULL, log,
-      kNoDelegateTransactionCheck);
+      kNoDelegateTransactionCheck, kNoDelegateTransactionCheck);
 }
 
 void RunTransactionTestWithDelegate(net::HttpCache* cache,
                                     const MockTransaction& trans_info,
-                                    int num_delegate_actions) {
+                                    int num_cache_delegate_actions,
+                                    int num_network_delegate_actions) {
   RunTransactionTestWithRequestAndLogAndDelegate(
       cache, trans_info, MockHttpRequest(trans_info), NULL, net::BoundNetLog(),
-      num_delegate_actions);
+      num_cache_delegate_actions, num_network_delegate_actions);
 }
 
 void RunTransactionTest(net::HttpCache* cache,
@@ -3905,7 +3930,13 @@
   MockHttpRequest request(transaction);
 
   scoped_ptr<Context> c(new Context());
-  int rv = cache.http_cache()->CreateTransaction(&c->trans, NULL);
+  // We use a test delegate to ensure that after initiating destruction
+  // of the transaction, no further delegate callbacks happen.
+  // We initialize the TestHttpTransactionDelegate with the correct number of
+  // cache actions and network actions to be reported.
+  scoped_ptr<TestHttpTransactionDelegate> delegate(
+      new TestHttpTransactionDelegate(7, 3));
+  int rv = cache.http_cache()->CreateTransaction(&c->trans, delegate.get());
   EXPECT_EQ(net::OK, rv);
 
   rv = c->trans->Start(&request, c->callback.callback(), net::BoundNetLog());
@@ -3934,6 +3965,11 @@
   c->trans.reset();
   MockHttpCache::SetTestMode(0);
 
+  // Since the transaction was aborted in the middle of network I/O, we will
+  // manually call the delegate so that its pending I/O operation will be
+  // closed (which is what the test delegate is expecting).
+  delegate->OnNetworkActionFinish();

[rvargas (doing something else), 2012/08/17 22:05:00]

It's better to have a way to explicitly check that the number of callbacks
doesn't change across c->trans.reset():

delegate->MyPrivateMethod() == x

[tburkard, 2012/08/17 22:39:56]

Done.

+
   // Make sure that we don't invoke the callback. We may have an issue if the
   // UrlRequestJob is killed directly (without cancelling the UrlRequest) so we
   // could end up with the transaction being deleted twice if we send any
@@ -5023,7 +5059,8 @@
   // Write to the cache.
   RunTransactionTestWithDelegate(cache.http_cache(),
                                  kSimpleGET_Transaction,
-                                 8);
+                                 8,
+                                 3);
 
   // Force this transaction to read from the cache.
   MockTransaction transaction(kSimpleGET_Transaction);
@@ -5031,5 +5068,6 @@
 
   RunTransactionTestWithDelegate(cache.http_cache(),
                                  kSimpleGET_Transaction,
-                                 5);
+                                 5,
+                                 0);
 }