Initialize the simple cache backend at runtime.

net/disk_cache/disk_cache_test_base.cc

Index: net/disk_cache/disk_cache_test_base.cc
===================================================================
--- net/disk_cache/disk_cache_test_base.cc	(revision 190930)
+++ net/disk_cache/disk_cache_test_base.cc	(working copy)
@@ -11,8 +11,11 @@
 #include "net/base/net_errors.h"
 #include "net/base/test_completion_callback.h"
 #include "net/disk_cache/backend_impl.h"
+#include "net/disk_cache/cache_util.h"
+#include "net/disk_cache/disk_cache.h"
 #include "net/disk_cache/disk_cache_test_util.h"
 #include "net/disk_cache/mem_backend_impl.h"
+#include "net/disk_cache/simple/simple_backend_impl.h"
 
 DiskCacheTest::DiskCacheTest() {
   CHECK(temp_dir_.CreateUniqueTempDir());
@@ -53,7 +56,7 @@
       size_(0),
       type_(net::DISK_CACHE),
       memory_only_(false),
-      implementation_(false),
+      simple_cache_mode_(false),
       force_creation_(false),
       new_eviction_(false),
       first_cleanup_(true),
@@ -65,9 +68,6 @@
 DiskCacheTestWithCache::~DiskCacheTestWithCache() {}
 
 void DiskCacheTestWithCache::InitCache() {
-  if (mask_ || new_eviction_)
-    implementation_ = true;
-
   if (memory_only_)
     InitMemoryCache();
   else
@@ -80,7 +80,7 @@
 
 // We are expected to leak memory when simulating crashes.
 void DiskCacheTestWithCache::SimulateCrash() {
-  ASSERT_TRUE(implementation_ && !memory_only_);
+  ASSERT_TRUE(!memory_only_);
   net::TestCompletionCallback cb;
   int rv = cache_impl_->FlushQueueForTest(cb.callback());
   ASSERT_EQ(net::OK, cb.GetResult(rv));
@@ -89,11 +89,11 @@
   delete cache_impl_;
   EXPECT_TRUE(CheckCacheIntegrity(cache_path_, new_eviction_, mask_));
 
-  InitDiskCacheImpl();
+  CreateBackendAsModeSuggests(disk_cache::kNoRandom, &cache_thread_);
 }
 
 void DiskCacheTestWithCache::SetTestMode() {
-  ASSERT_TRUE(implementation_ && !memory_only_);
+  ASSERT_TRUE(!memory_only_);
   cache_impl_->SetUnitTestMode();
 }
 
@@ -236,17 +236,12 @@
 }
 
 void DiskCacheTestWithCache::InitMemoryCache() {
-  if (!implementation_) {
-    cache_ = disk_cache::MemBackendImpl::CreateBackend(size_, NULL);
-    return;
-  }
-
   mem_cache_ = new disk_cache::MemBackendImpl(NULL);
   cache_ = mem_cache_;
   ASSERT_TRUE(NULL != cache_);
 
   if (size_)
-    EXPECT_TRUE(mem_cache_->SetMaxSize(size_));
+    ASSERT_TRUE(mem_cache_->SetMaxSize(size_));

[rvargas (doing something else), 2013/03/28 03:12:45]

The idea was that failing to set the size is normnally not critical... yes, the
test will probably fail, but nothing will crash.

On the other hand, moving on without a cache_, or after Init() fails most likely
will crash net_unittests (yes, it is still broken 'cause we indeed move on, and
we indeed crash)

[pasko-google - do not use, 2013/03/28 21:41:42]

OK, I was assuming all standard fixture initialization should always succeed,
and if it does not succeed the problem is not in the test but rather in the
testing framework.

Reverted to the original since it is your choice about the importance of this
very specific failure.

 
   ASSERT_TRUE(mem_cache_->Init());
 }
@@ -256,46 +251,68 @@
     ASSERT_TRUE(CleanupCacheDir());
 
   if (!cache_thread_.IsRunning()) {
-    EXPECT_TRUE(cache_thread_.StartWithOptions(
+    ASSERT_TRUE(cache_thread_.StartWithOptions(
                     base::Thread::Options(MessageLoop::TYPE_IO, 0)));
   }
   ASSERT_TRUE(cache_thread_.message_loop() != NULL);
 
-  if (implementation_)
-    return InitDiskCacheImpl();
+  CreateBackendAsModeSuggests(disk_cache::kNoRandom, &cache_thread_);
+}
 
-  scoped_refptr<base::MessageLoopProxy> thread =
-      use_current_thread_ ? base::MessageLoopProxy::current() :
-                            cache_thread_.message_loop_proxy();
+// Testing backend creation retry logic is hard because the CacheCreator and
+// cache backend(s) are tightly coupled. So we take the default backend often.
+// Tests themselves need to be adjusted for platforms where the BackendImpl is
+// not the default backend.
+void DiskCacheTestWithCache::InitDefaultCacheViaCreator() {
+  if (!cache_thread_.IsRunning()) {
+    ASSERT_TRUE(cache_thread_.StartWithOptions(
+                    base::Thread::Options(MessageLoop::TYPE_IO, 0)));
+  }
+  ASSERT_TRUE(cache_thread_.message_loop() != NULL);
 
   net::TestCompletionCallback cb;
-  int rv = disk_cache::BackendImpl::CreateBackend(
-               cache_path_, force_creation_, size_, type_,
-               disk_cache::kNoRandom, thread, NULL, &cache_, cb.callback());
+  disk_cache::CacheCreator* creator = new disk_cache::CacheCreator(cache_path_,

[rvargas (doing something else), 2013/03/28 03:12:45]

nit: argument must go down

[rvargas (doing something else), 2013/03/28 03:12:45]

Use CreateCacheBackend instead.

[pasko-google - do not use, 2013/03/28 21:41:42]

Done.

[pasko-google - do not use, 2013/03/28 21:41:42]

OK, I'll try to do it in the next smaller CL, hopefully it won't make the tests
flaky.

+      true, 0, net::DISK_CACHE, disk_cache::kNoRandom,
+      cache_thread_.message_loop_proxy(), NULL, &cache_, cb.callback());
+  int rv = creator->Run();
   ASSERT_EQ(net::OK, cb.GetResult(rv));
 }
 
-void DiskCacheTestWithCache::InitDiskCacheImpl() {
-  scoped_refptr<base::MessageLoopProxy> thread =
-      use_current_thread_ ? base::MessageLoopProxy::current() :
-                            cache_thread_.message_loop_proxy();
+void DiskCacheTestWithCache::CreateBackendAsModeSuggests(
+    uint32 flags, base::Thread* thread) {

[rvargas (doing something else), 2013/03/28 03:12:45]

first arg on the previous line, second one indented under the first one.

[pasko-google - do not use, 2013/03/28 21:41:42]

How do you know it would fit the line? :)

[rvargas (doing something else), 2013/03/28 22:30:21]

Sometimes I try things before adding comments.

+  base::MessageLoopProxy* runner;
+  if (use_current_thread_)
+    runner = base::MessageLoopProxy::current();
+  else
+    runner = thread->message_loop_proxy();
+
+  if (simple_cache_mode_) {
+    net::TestCompletionCallback cb;
+    disk_cache::Backend* simple_backend;
+    // TODO(pasko): split Simple Backend construction from initialization.
+    int rv = disk_cache::SimpleBackendImpl::CreateBackend(cache_path_, size_,
+        type_, disk_cache::kNone, make_scoped_refptr(runner), NULL,
+        &simple_backend, cb.callback());
+    ASSERT_EQ(net::OK, cb.GetResult(rv));
+    cache_ = simple_backend;
+    return;
+  }
+
   if (mask_)
-    cache_impl_ = new disk_cache::BackendImpl(cache_path_, mask_, thread, NULL);
+    cache_impl_ = new disk_cache::BackendImpl(cache_path_, mask_, runner, NULL);
   else
-    cache_impl_ = new disk_cache::BackendImpl(cache_path_, thread, NULL);
-
+    cache_impl_ = new disk_cache::BackendImpl(cache_path_, runner, NULL);
   cache_ = cache_impl_;
   ASSERT_TRUE(NULL != cache_);
-
   if (size_)
-    EXPECT_TRUE(cache_impl_->SetMaxSize(size_));
-
+    ASSERT_TRUE(cache_impl_->SetMaxSize(size_));

[rvargas (doing something else), 2013/03/28 03:12:45]

same thing about SetMaxSize (non fatal)

[pasko-google - do not use, 2013/03/28 21:41:42]

Done.

   if (new_eviction_)
     cache_impl_->SetNewEviction();
-
   cache_impl_->SetType(type_);
-  cache_impl_->SetFlags(disk_cache::kNoRandom);
+  cache_impl_->SetFlags(flags);
   net::TestCompletionCallback cb;
   int rv = cache_impl_->Init(cb.callback());
   ASSERT_EQ(net::OK, cb.GetResult(rv));
+  cache_ = cache_impl_;
 }
+

[rvargas (doing something else), 2013/03/28 03:12:45]

nit: is this an extra line?

[pasko-google - do not use, 2013/03/28 21:41:42]

Done.