Cache failover to LOAD_PREFERRING_CACHE if network response suggests offline.

net/http/http_cache_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/http/http_cache.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/memory/scoped_vector.h"
 #include "base/message_loop.h"
@@ skipping 137 matching lines @@
                                         num_network_delegate_actions));
   }
   scoped_ptr<net::HttpTransaction> trans;
   int rv = cache->CreateTransaction(&trans, delegate.get());
   EXPECT_EQ(net::OK, rv);
   ASSERT_TRUE(trans.get());
 
   rv = trans->Start(&request, callback.callback(), net_log);
   if (rv == net::ERR_IO_PENDING)
     rv = callback.WaitForResult();
-  ASSERT_EQ(net::OK, rv);
+  ASSERT_EQ(trans_info.error_return, rv);
 
   const net::HttpResponseInfo* response = trans->GetResponseInfo();
   ASSERT_TRUE(response);
 
   if (response_info)
     *response_info = *response;
 
   ReadAndVerifyTransaction(trans.get(), trans_info);
 }
 

[rvargas (doing something else), 2013/03/05 02:58:19]

This file defines a bunch of transactions... which means that something is fishy
if the current test are passing (as in we need to find out all places that
define transactions).

[Randy Smith (Not in Mondays), 2013/03/05 23:16:14]

While I completely don't guarantee that I saw everything, I tried to look at all
defined transactions, and they all either copied from one of the template
transactions defined in http_transaction_unittest.* or initialized with 0, both
of which work with my new code.  So it doesn't surprise me that the current
tests pass.  Can you point me at a single test that you think should fail, and
I'll use that to refine my scanning filter?

[rvargas (doing something else), 2013/03/06 03:11:48]

lines 377, 240 

[Randy Smith (Not in Mondays), 2013/03/06 22:55:55]

Ah, got it.  Filter failure was looking at codesearch "instantiations" and not
scanning the references too (not sure why it classified those there, but hey). 
Got those two, as well as an extra one in url_request_job_unittest.cc.  I also
scanned the ScopedMockTransaction s, but they're all based off one of the
constant ones, and hence are ok.  The reasons the test were passing is, I
assume, zero fill; the last entry in the ones I replaced was zero, and that
presumably got extended into the return code.

I didn't modify the initializations that look like:

  MockTransaction mock_network_response = { 0 };

They're as good (or dodgy) as they always were.

Let me know if you think I've skipped anything.

 void RunTransactionTestWithRequest(net::HttpCache* cache,
                                    const MockTransaction& trans_info,
                                    const MockHttpRequest& request,
                                    net::HttpResponseInfo* response_info) {
   RunTransactionTestWithRequestAndLogAndDelegate(
       cache, trans_info, request, response_info, net::BoundNetLog(),
       kNoDelegateTransactionCheck, kNoDelegateTransactionCheck);
 }
 
 void RunTransactionTestWithLog(net::HttpCache* cache,
@@ skipping 1017 matching lines @@
   for (int i = 0; i < kNumTransactions; ++i) {
     if (i == 1)
       continue;
     Context* c = context_list[i];
     ASSERT_EQ(net::ERR_IO_PENDING, c->result);
     c->result = c->callback.WaitForResult();
     ReadAndVerifyTransaction(c->trans.get(), kSimpleGET_Transaction);
   }
 }
 
+// Tests that LOAD_CACHE_RETURN_IF_OFFLINE returns proper responses on
+// network success, offline failure, and non-offline failure.

[rvargas (doing something else), 2013/03/05 02:58:19]

sounds like three different tests.

[Randy Smith (Not in Mondays), 2013/03/05 23:16:14]

Done.

+TEST(HttpCache, SimpleGET_CacheOverride) {

[rvargas (doing something else), 2013/03/05 02:58:19]

could you move this next to a test that goes over behavior with load flags?
Around 840?

[Randy Smith (Not in Mondays), 2013/03/05 23:16:14]

Done.

+  MockHttpCache cache(net::HttpCache::DefaultBackend::InMemory(1024 * 1024));

[rvargas (doing something else), 2013/03/05 02:58:19]

why a real cache?

[Randy Smith (Not in Mondays), 2013/03/05 23:16:14]

Naivete': I copy-pasted from the wrong test.  Fixed.  (Presuming "real cache" ==
not using the null constructor.)

+
+  MockTransaction transaction(kSimpleGET_Transaction);
+  transaction.load_flags |= net::LOAD_CACHE_RETURN_IF_OFFLINE;
+  transaction.response_headers = "Cache-Control: no-cache\n";
+
+  // Regular network test; should behave as normal.
+  AddMockTransaction(&transaction);
+  net::HttpResponseInfo response_info;
+  RunTransactionTestWithResponseInfo(cache.http_cache(), transaction,
+                                     &response_info);
+
+  EXPECT_EQ(1, cache.network_layer()->transaction_count());
+  EXPECT_FALSE(response_info.was_cache_override);
+
+  RemoveMockTransaction(&transaction);
+
+  // Network failure with offline error; should return cache entry above +
+  // flag signalling stale data.
+  transaction.error_return = net::ERR_NAME_NOT_RESOLVED;
+  AddMockTransaction(&transaction);
+
+  MockHttpRequest request(transaction);
+  net::TestCompletionCallback callback;
+  scoped_ptr<net::HttpTransaction> trans;
+  int rv = cache.http_cache()->CreateTransaction(&trans, NULL);
+  EXPECT_EQ(net::OK, rv);
+  ASSERT_TRUE(trans.get());
+  rv = trans->Start(&request, callback.callback(), net::BoundNetLog());
+  if (rv == net::ERR_IO_PENDING)
+    rv = callback.WaitForResult();

[rvargas (doing something else), 2013/03/05 02:58:19]

GetResult(rv)

[Randy Smith (Not in Mondays), 2013/03/05 23:16:14]

Done.

+  EXPECT_EQ(net::OK, rv);
+
+  const net::HttpResponseInfo* response_info1 = trans->GetResponseInfo();
+  ASSERT_TRUE(response_info1);
+  EXPECT_TRUE(response_info1->was_cache_override);
+  ReadAndVerifyTransaction(trans.get(), transaction);
+  EXPECT_EQ(2, cache.network_layer()->transaction_count());
+
+  RemoveMockTransaction(&transaction);
+
+  // Network failure with non-offline error; should fail with that error.
+  transaction.error_return = net::ERR_PROXY_CONNECTION_FAILED;
+  AddMockTransaction(&transaction);
+
+  net::HttpResponseInfo response_info2;
+  RunTransactionTestWithResponseInfo(cache.http_cache(), transaction,
+                                     &response_info2);
+
+  EXPECT_EQ(3, cache.network_layer()->transaction_count());
+  EXPECT_FALSE(response_info2.was_cache_override);
+
+  RemoveMockTransaction(&transaction);
+}
+
 // This is a test for http://code.google.com/p/chromium/issues/detail?id=4731.
 // We may attempt to delete an entry synchronously with the act of adding a new
 // transaction to said entry.
 TEST(HttpCache, FastNoStoreGET_DoneWithPending) {
   MockHttpCache cache;
 
   // The headers will be served right from the call to Start() the request.
   MockHttpRequest request(kFastNoStoreGET_Transaction);
   FastTransactionServer request_handler;
   AddMockTransaction(&kFastNoStoreGET_Transaction);
@@ skipping 4297 matching lines @@
 
   // Force this transaction to read from the cache.
   MockTransaction transaction(kSimpleGET_Transaction);
   transaction.load_flags |= net::LOAD_ONLY_FROM_CACHE;
 
   RunTransactionTestWithDelegate(cache.http_cache(),
                                  kSimpleGET_Transaction,
                                  5,
                                  0);
 }