Test cache creation retry via public interface only.

This is a cleanup after:
https://codereview.chromium.org/12794003/

One test for backend force retry is enough, the other test
(CreateBackend_MissingFile) is converted to be backend implementation specific.

The CreateCacheViaPublicInterface() is called only once, but to eliminate it
there is a need to make the cache_thread_ protected. Also, I am sure the name is
not ideal, please suggest another.

The New Eviction allows not to be very strict about version comparison as stored
in the files on disk, this allows creating the BackendImpl with no flags and
then being able to pass the common integrity check.

BUG=173384


Review URL: https://chromiumcodereview.appspot.com/13517004

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

net/disk_cache/backend_unittest.cc

@@ -245,17 +245,25 @@ TEST_F(DiskCacheTest, CreateBackend) {
   MessageLoop::current()->RunUntilIdle();
 }
 
-// Testst that re-creating the cache performs the expected cleanup.
+// Tests that |BackendImpl| fails to initialize with a missing file.
 TEST_F(DiskCacheBackendTest, CreateBackend_MissingFile) {
   ASSERT_TRUE(CopyTestCache("bad_entry"));
   base::FilePath filename = cache_path_.AppendASCII("data_1");
   file_util::Delete(filename, false);
-  DisableFirstCleanup();
-  SetForceCreation();
+  base::Thread cache_thread("CacheThread");
+  ASSERT_TRUE(cache_thread.StartWithOptions(
+                  base::Thread::Options(MessageLoop::TYPE_IO, 0)));
+  net::TestCompletionCallback cb;
 
   bool prev = base::ThreadRestrictions::SetIOAllowed(false);
-  InitDefaultCacheViaCreator();
+  disk_cache::BackendImpl* cache = new disk_cache::BackendImpl(
+      cache_path_, cache_thread.message_loop_proxy(), NULL);
+  int rv = cache->Init(cb.callback());
+  ASSERT_EQ(net::ERR_FAILED, cb.GetResult(rv));
   base::ThreadRestrictions::SetIOAllowed(prev);
+
+  delete cache;
+  DisableIntegrityCheck();
 }
 
 TEST_F(DiskCacheBackendTest, ExternalFiles) {
@@ -1541,7 +1549,21 @@ TEST_F(DiskCacheTest, WrongVersion) {
 // Tests that the cache is properly restarted on recovery error.
 TEST_F(DiskCacheBackendTest, DeleteOld) {
   ASSERT_TRUE(CopyTestCache("wrong_version"));
-  InitDefaultCacheViaCreator();
+  SetNewEviction();
+  base::Thread cache_thread("CacheThread");
+  ASSERT_TRUE(cache_thread.StartWithOptions(
+                  base::Thread::Options(MessageLoop::TYPE_IO, 0)));
+
+  net::TestCompletionCallback cb;
+  bool prev = base::ThreadRestrictions::SetIOAllowed(false);
+  int rv = disk_cache::CreateCacheBackend(net::DISK_CACHE, cache_path_, 0, true,
+                                          cache_thread.message_loop_proxy(),
+                                          NULL, &cache_, cb.callback());
+  ASSERT_EQ(net::OK, cb.GetResult(rv));
+  base::ThreadRestrictions::SetIOAllowed(prev);
+  delete cache_;
+  cache_ = NULL;
+  EXPECT_TRUE(CheckCacheIntegrity(cache_path_, new_eviction_, mask_));
 }
 
 // We want to be able to deal with messed up entries on disk.

net/disk_cache/cache_creator.cc

@@ -11,7 +11,44 @@
 #include "net/disk_cache/mem_backend_impl.h"
 #include "net/disk_cache/simple/simple_backend_impl.h"
 
-namespace disk_cache {
+namespace {
+
+// Builds an instance of the backend depending on platform, type, experiments
+// etc. Takes care of the retry state. This object will self-destroy when
+// finished.
+class CacheCreator {
+ public:
+  CacheCreator(const base::FilePath& path, bool force, int max_bytes,
+               net::CacheType type, uint32 flags,
+               base::MessageLoopProxy* thread, net::NetLog* net_log,
+               disk_cache::Backend** backend,
+               const net::CompletionCallback& callback);
+
+  // Creates the backend.
+  int Run();
+
+ private:
+  ~CacheCreator();
+
+  void DoCallback(int result);
+
+  void OnIOComplete(int result);
+
+  const base::FilePath& path_;
+  bool force_;
+  bool retry_;
+  int max_bytes_;
+  net::CacheType type_;
+  uint32 flags_;
+  scoped_refptr<base::MessageLoopProxy> thread_;
+  disk_cache::Backend** backend_;
+  net::CompletionCallback callback_;
+  disk_cache::Backend* created_cache_;
+  net::NetLog* net_log_;
+  bool use_simple_cache_backend_;
+
+  DISALLOW_COPY_AND_ASSIGN(CacheCreator);
+};
 
 CacheCreator::CacheCreator(
     const base::FilePath& path, bool force, int max_bytes,
@@ -103,6 +140,10 @@ void CacheCreator::OnIOComplete(int result) {
   DCHECK_EQ(net::ERR_IO_PENDING, rv);
 }
 
+}  // namespace
+
+namespace disk_cache {
+
 int CreateCacheBackend(net::CacheType type, const base::FilePath& path,
                        int max_bytes,
                        bool force, base::MessageLoopProxy* thread,

net/disk_cache/cache_util.h

@@ -36,43 +36,6 @@ NET_EXPORT_PRIVATE bool DeleteCacheFile(const base::FilePath& name);
 // task. Used by cache creator itself or by backends for self-restart on error.
 bool DelayedCacheCleanup(const base::FilePath& full_path);
 
-// Builds an instance of the backend depending on platform, type, experiments
-// etc. Takes care of the retry state. This object will self-destroy when
-// finished.
-class NET_EXPORT_PRIVATE CacheCreator {
- public:
-  CacheCreator(const base::FilePath& path, bool force, int max_bytes,
-               net::CacheType type, uint32 flags,
-               base::MessageLoopProxy* thread, net::NetLog* net_log,
-               disk_cache::Backend** backend,
-               const net::CompletionCallback& callback);
-
-  // Creates the backend.
-  int Run();
-
- private:
-  ~CacheCreator();
-
-  void DoCallback(int result);
-
-  void OnIOComplete(int result);
-
-  const base::FilePath& path_;
-  bool force_;
-  bool retry_;
-  int max_bytes_;
-  net::CacheType type_;
-  uint32 flags_;
-  scoped_refptr<base::MessageLoopProxy> thread_;
-  disk_cache::Backend** backend_;
-  net::CompletionCallback callback_;
-  disk_cache::Backend* created_cache_;
-  net::NetLog* net_log_;
-  bool use_simple_cache_backend_;
-
-  DISALLOW_COPY_AND_ASSIGN(CacheCreator);
-};
-
 }  // namespace disk_cache
 
 #endif  // NET_DISK_CACHE_CACHE_UTIL_H_

net/disk_cache/disk_cache_test_base.cc

@@ -259,25 +259,6 @@ void DiskCacheTestWithCache::InitDiskCache() {
   CreateBackend(disk_cache::kNoRandom, &cache_thread_);
 }
 
-// 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;
-  disk_cache::CacheCreator* creator = new disk_cache::CacheCreator(
-      cache_path_, 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::CreateBackend(uint32 flags, base::Thread* thread) {
   base::MessageLoopProxy* runner;
   if (use_current_thread_)

net/disk_cache/disk_cache_test_base.h

@@ -62,7 +62,6 @@ class DiskCacheTestWithCache : public DiskCacheTest {
   void CreateBackend(uint32 flags, base::Thread* thread);
 
   void InitCache();
-  void InitDefaultCacheViaCreator();
   void SimulateCrash();
   void SetTestMode();