Http cache: Add a test to make sure that the cache

works as expected with synchronous responses.

As an added bonus, now changing the default test mode
of kRangeGET_TransactionOK doesn't end up with some tests
hanging, so it is easy to manually verify that all tests
work as expected.

BUG=26729
TEST=unittests

Review URL: http://codereview.chromium.org/501099

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@34914 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 83 insertions, 45 deletions

diff --git a/net/base/test_completion_callback.h b/net/base/test_completion_callback.h
index 82680aa..5fa14ae 100644
--- a/net/base/test_completion_callback.h
+++ b/net/base/test_completion_callback.h
@@ -7,6 +7,7 @@
 
 #include "base/message_loop.h"
 #include "net/base/completion_callback.h"
+#include "net/base/net_errors.h"
 
 //-----------------------------------------------------------------------------
 // completion callback helper
@@ -38,6 +39,12 @@ class TestCompletionCallback : public CallbackRunner< Tuple1<int> > {
     return result_;
   }
 
+  int GetResult(int result) {
+    if (net::ERR_IO_PENDING != result)
+      return result;
+    return WaitForResult();
+  }
+
   bool have_result() const { return have_result_; }
 
   virtual void RunWithParams(const Tuple1<int>& params) {
diff --git a/net/disk_cache/backend_unittest.cc b/net/disk_cache/backend_unittest.cc
index 6609d3a..9d4194b 100644
--- a/net/disk_cache/backend_unittest.cc
+++ b/net/disk_cache/backend_unittest.cc
@@ -9,6 +9,7 @@
 #include "base/string_util.h"
 #include "net/base/io_buffer.h"
 #include "net/base/net_errors.h"
+#include "net/base/test_completion_callback.h"
 #include "net/disk_cache/backend_impl.h"
 #include "net/disk_cache/disk_cache_test_base.h"
 #include "net/disk_cache/disk_cache_test_util.h"
@@ -208,7 +209,7 @@ TEST_F(DiskCacheBackendTest, ExternalFiles) {
 }
 
 TEST_F(DiskCacheTest, ShutdownWithPendingIO) {
-  SimpleCallbackTest callback;
+  TestCompletionCallback callback;
 
   {
     FilePath path = GetCacheFilePath();
diff --git a/net/disk_cache/disk_cache_test_util.cc b/net/disk_cache/disk_cache_test_util.cc
index 2293a5e..36d28d2 100644
--- a/net/disk_cache/disk_cache_test_util.cc
+++ b/net/disk_cache/disk_cache_test_util.cc
@@ -7,7 +7,6 @@
 #include "base/logging.h"
 #include "base/file_util.h"
 #include "base/path_service.h"
-#include "net/base/net_errors.h"
 #include "net/disk_cache/backend_impl.h"
 #include "net/disk_cache/cache_util.h"
 #include "net/disk_cache/file.h"
@@ -125,14 +124,6 @@ void CallbackTest::RunWithParams(const Tuple1<int>& params) {
 
 // -----------------------------------------------------------------------
 
-int SimpleCallbackTest::GetResult(int result) {
-  if (net::ERR_IO_PENDING != result)
-    return result;
-  return WaitForResult();
-}
-
-// -----------------------------------------------------------------------
-
 MessageLoopHelper::MessageLoopHelper()
     : num_callbacks_(0),
       num_iterations_(0),
diff --git a/net/disk_cache/disk_cache_test_util.h b/net/disk_cache/disk_cache_test_util.h
index 8e980ab..623810b 100644
--- a/net/disk_cache/disk_cache_test_util.h
+++ b/net/disk_cache/disk_cache_test_util.h
@@ -12,7 +12,6 @@
 #include "base/task.h"
 #include "base/timer.h"
 #include "build/build_config.h"
-#include "net/base/test_completion_callback.h"
 
 class FilePath;
 
@@ -70,24 +69,6 @@ class CallbackTest : public CallbackRunner< Tuple1<int> >  {
 
 // -----------------------------------------------------------------------
 
-// Simple callback to process IO completions from the cache. This object is not
-// intended to be used when multiple IO operations are in-flight at the same
-// time.
-class SimpleCallbackTest : public TestCompletionCallback  {
- public:
-  SimpleCallbackTest() {}
-  ~SimpleCallbackTest() {}
-
-  // Returns the final result of the IO operation. If |result| is
-  // net::ERR_IO_PENDING, it waits for the callback be invoked.
-  int GetResult(int result);
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(SimpleCallbackTest);
-};
-
-// -----------------------------------------------------------------------
-
 // Simple helper to deal with the message loop on a test.
 class MessageLoopHelper {
  public:
diff --git a/net/disk_cache/entry_unittest.cc b/net/disk_cache/entry_unittest.cc
index ca4d6ed..e60a3da 100644
--- a/net/disk_cache/entry_unittest.cc
+++ b/net/disk_cache/entry_unittest.cc
@@ -8,6 +8,7 @@
 #include "base/string_util.h"
 #include "net/base/io_buffer.h"
 #include "net/base/net_errors.h"
+#include "net/base/test_completion_callback.h"
 #include "net/disk_cache/disk_cache_test_base.h"
 #include "net/disk_cache/disk_cache_test_util.h"
 #include "net/disk_cache/entry_impl.h"
@@ -880,8 +881,8 @@ TEST_F(DiskCacheEntryTest, MemoryOnlyEnumerationWithSparseEntries) {
 void VerifySparseIO(disk_cache::Entry* entry, int64 offset,
                     net::IOBuffer* buf_1, int size, bool async,
                     net::IOBuffer* buf_2) {
-  SimpleCallbackTest callback;
-  SimpleCallbackTest* cb = async ? &callback : NULL;
+  TestCompletionCallback callback;
+  TestCompletionCallback* cb = async ? &callback : NULL;
 
   memset(buf_2->data(), 0, size);
   int ret = entry->ReadSparseData(offset, buf_2, size, cb);
@@ -903,8 +904,8 @@ void VerifySparseIO(disk_cache::Entry* entry, int64 offset,
 // same as the content of the provided |buffer|.
 void VerifyContentSparseIO(disk_cache::Entry* entry, int64 offset, char* buffer,
                            int size, bool async) {
-  SimpleCallbackTest callback;
-  SimpleCallbackTest* cb = async ? &callback : NULL;
+  TestCompletionCallback callback;
+  TestCompletionCallback* cb = async ? &callback : NULL;
 
   scoped_refptr<net::IOBuffer> buf_1 = new net::IOBuffer(size);
   memset(buf_1->data(), 0, size);
@@ -1354,7 +1355,7 @@ TEST_F(DiskCacheEntryTest, CancelSparseIO) {
   scoped_refptr<net::IOBuffer> buf = new net::IOBuffer(kSize);
   CacheTestFillBuffer(buf->data(), kSize, false);
 
-  SimpleCallbackTest cb1, cb2, cb3, cb4;
+  TestCompletionCallback cb1, cb2, cb3, cb4;
   int64 offset = 0;
   int tries = 0;
   const int maxtries = 100;   // Avoid hang on infinitely fast disks
diff --git a/net/http/http_cache_transaction.cc b/net/http/http_cache_transaction.cc
index a32efc3..918ebad 100644
--- a/net/http/http_cache_transaction.cc
+++ b/net/http/http_cache_transaction.cc
@@ -1604,7 +1604,7 @@ int HttpCache::Transaction::DoOverwriteCachedResponse() {
 }
 
 int HttpCache::Transaction::DoTruncateCachedDataComplete(int result) {
-  // If this response is a redirect, then we can stop writing now.  (We  don't
+  // If this response is a redirect, then we can stop writing now.  (We don't
   // need to cache the response body of a redirect.)
   if (response_.headers->IsRedirect(NULL))
     DoneWritingToEntry(true);
diff --git a/net/http/http_cache_unittest.cc b/net/http/http_cache_unittest.cc
index 3a207ab..2a30bf0 100644
--- a/net/http/http_cache_unittest.cc
+++ b/net/http/http_cache_unittest.cc
@@ -2287,6 +2287,62 @@ TEST(HttpCache, RangeGET_OK) {
   RemoveMockTransaction(&kRangeGET_TransactionOK);
 }
 
+// Tests that we can cache range requests and fetch random blocks from the
+// cache and the network, with synchronous responses.
+TEST(HttpCache, RangeGET_SyncOK) {
+  MockHttpCache cache;
+  cache.http_cache()->set_enable_range_support(true);
+
+  MockTransaction transaction(kRangeGET_TransactionOK);
+  transaction.test_mode = TEST_MODE_SYNC_ALL;
+  AddMockTransaction(&transaction);
+
+  // Write to the cache (40-49).
+  std::string headers;
+  RunTransactionTestWithResponse(cache.http_cache(), transaction, &headers);
+
+  EXPECT_TRUE(Verify206Response(headers, 40, 49));
+  EXPECT_EQ(1, cache.network_layer()->transaction_count());
+  EXPECT_EQ(0, cache.disk_cache()->open_count());
+  EXPECT_EQ(1, cache.disk_cache()->create_count());
+
+  // Read from the cache (40-49).
+  RunTransactionTestWithResponse(cache.http_cache(), transaction, &headers);
+
+  EXPECT_TRUE(Verify206Response(headers, 40, 49));
+  EXPECT_EQ(2, cache.network_layer()->transaction_count());
+  EXPECT_EQ(0, cache.disk_cache()->open_count());
+  EXPECT_EQ(1, cache.disk_cache()->create_count());
+
+  // Make sure we are done with the previous transaction.
+  MessageLoop::current()->RunAllPending();
+
+  // Write to the cache (30-39).
+  transaction.request_headers = "Range: bytes = 30-39\r\n" EXTRA_HEADER;
+  transaction.data = "rg: 30-39 ";
+  RunTransactionTestWithResponse(cache.http_cache(), transaction, &headers);
+
+  EXPECT_TRUE(Verify206Response(headers, 30, 39));
+  EXPECT_EQ(3, cache.network_layer()->transaction_count());
+  EXPECT_EQ(1, cache.disk_cache()->open_count());
+  EXPECT_EQ(1, cache.disk_cache()->create_count());
+
+  // Make sure we are done with the previous transaction.
+  MessageLoop::current()->RunAllPending();
+
+  // Write and read from the cache (20-59).
+  transaction.request_headers = "Range: bytes = 20-59\r\n" EXTRA_HEADER;
+  transaction.data = "rg: 20-29 rg: 30-39 rg: 40-49 rg: 50-59 ";
+  RunTransactionTestWithResponse(cache.http_cache(), transaction, &headers);
+
+  EXPECT_TRUE(Verify206Response(headers, 20, 59));
+  EXPECT_EQ(5, cache.network_layer()->transaction_count());
+  EXPECT_EQ(2, cache.disk_cache()->open_count());
+  EXPECT_EQ(1, cache.disk_cache()->create_count());
+
+  RemoveMockTransaction(&transaction);
+}
+
 // Tests that we deal with 304s for range requests.
 TEST(HttpCache, RangeGET_304) {
   MockHttpCache cache;
@@ -2837,8 +2893,7 @@ TEST(HttpCache, RangeGET_Cancel2) {
   // read will return while waiting for the network).
   scoped_refptr<net::IOBufferWithSize> buf = new net::IOBufferWithSize(5);
   rv = c->trans->Read(buf, buf->size(), &c->callback);
-  EXPECT_EQ(net::ERR_IO_PENDING, rv);
-  rv = c->callback.WaitForResult();
+  EXPECT_EQ(5, c->callback.GetResult(rv));
   rv = c->trans->Read(buf, buf->size(), &c->callback);
   EXPECT_EQ(net::ERR_IO_PENDING, rv);
 
@@ -2883,8 +2938,7 @@ TEST(HttpCache, RangeGET_Cancel3) {
   // read will return while waiting for the network).
   scoped_refptr<net::IOBufferWithSize> buf = new net::IOBufferWithSize(5);
   rv = c->trans->Read(buf, buf->size(), &c->callback);
-  EXPECT_EQ(net::ERR_IO_PENDING, rv);
-  rv = c->callback.WaitForResult();
+  EXPECT_EQ(5, c->callback.GetResult(rv));
   rv = c->trans->Read(buf, buf->size(), &c->callback);
   EXPECT_EQ(net::ERR_IO_PENDING, rv);
 
@@ -3353,14 +3407,14 @@ TEST(HttpCache, GET_CancelIncompleteResource) {
   Context* c = new Context();
   EXPECT_EQ(net::OK, cache.http_cache()->CreateTransaction(&c->trans));
 
-  EXPECT_EQ(net::ERR_IO_PENDING, c->trans->Start(&request, &c->callback, NULL));
-  EXPECT_EQ(net::OK, c->callback.WaitForResult());
+  int rv = c->trans->Start(&request, &c->callback, NULL);
+  EXPECT_EQ(net::OK, c->callback.GetResult(rv));
 
   // Read 20 bytes from the cache, and 10 from the net.
-  EXPECT_EQ(net::ERR_IO_PENDING, c->trans->Read(buf, len, &c->callback));
-  EXPECT_EQ(len, c->callback.WaitForResult());
-  EXPECT_EQ(net::ERR_IO_PENDING, c->trans->Read(buf, 10, &c->callback));
-  EXPECT_EQ(10, c->callback.WaitForResult());
+  rv = c->trans->Read(buf, len, &c->callback);
+  EXPECT_EQ(len, c->callback.GetResult(rv));
+  rv = c->trans->Read(buf, 10, &c->callback);
+  EXPECT_EQ(10, c->callback.GetResult(rv));
 
   // At this point, we are already reading so canceling the request should leave
   // a truncated one.
diff --git a/net/http/http_transaction_unittest.h b/net/http/http_transaction_unittest.h
index 2a93237..cffde7a 100644
--- a/net/http/http_transaction_unittest.h
+++ b/net/http/http_transaction_unittest.h
@@ -34,6 +34,9 @@ enum {
   TEST_MODE_SYNC_CACHE_START = 1 << 2,
   TEST_MODE_SYNC_CACHE_READ  = 1 << 3,
   TEST_MODE_SYNC_CACHE_WRITE  = 1 << 4,
+  TEST_MODE_SYNC_ALL = TEST_MODE_SYNC_NET_START | TEST_MODE_SYNC_NET_READ |
+                       TEST_MODE_SYNC_CACHE_START | TEST_MODE_SYNC_CACHE_READ |
+                       TEST_MODE_SYNC_CACHE_WRITE
 };
 
 typedef void (*MockTransactionHandler)(const net::HttpRequestInfo* request,