Separate Simple Backend creation from initialization.

Also make both backends use the same directory for files, and the same name for
the index file. Using the index file each backend will detect if the index
belongs to it. Hence, easier to make sure that both cache trees on disk are
mutually exclusive.

BUG=229437


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

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

net/disk_cache/cache_creator.cc

@@ -45,7 +45,6 @@ class CacheCreator {
   net::CompletionCallback callback_;
   disk_cache::Backend* created_cache_;
   net::NetLog* net_log_;
-  bool use_simple_cache_backend_;
 
   DISALLOW_COPY_AND_ASSIGN(CacheCreator);
 };
@@ -66,8 +65,7 @@ CacheCreator::CacheCreator(
       backend_(backend),
       callback_(callback),
       created_cache_(NULL),
-      net_log_(net_log),
-      use_simple_cache_backend_(false) {
+      net_log_(net_log) {
   }
 
 CacheCreator::~CacheCreator() {
@@ -82,12 +80,12 @@ int CacheCreator::Run() {
     // testing it against net::DISK_CACHE. Turn it on for more cache types as
     // appropriate.
     if (type_ == net::DISK_CACHE) {
-      VLOG(1) << "Using the Simple Cache Backend.";
-      use_simple_cache_backend_ = true;
-      return disk_cache::SimpleBackendImpl::CreateBackend(
-          path_, max_bytes_, type_, disk_cache::kNone, thread_, net_log_,
-          backend_, base::Bind(&CacheCreator::OnIOComplete,
-                               base::Unretained(this)));
+      disk_cache::SimpleBackendImpl* simple_cache =
+          new disk_cache::SimpleBackendImpl(path_, max_bytes_, type_, thread_,
+                                            net_log_);
+      created_cache_ = simple_cache;
+      return simple_cache->Init(
+          base::Bind(&CacheCreator::OnIOComplete, base::Unretained(this)));
     }
   }
   disk_cache::BackendImpl* new_cache =
@@ -104,14 +102,9 @@ int CacheCreator::Run() {
 
 void CacheCreator::DoCallback(int result) {
   DCHECK_NE(net::ERR_IO_PENDING, result);
-  if (result == net::OK) {
-    // TODO(pasko): Separate creation of the Simple Backend from its
-    // initialization, eliminate unnecessary use_simple_cache_backend_.
-    if (use_simple_cache_backend_)
-      created_cache_ = *backend_;
-    else
-      *backend_ = created_cache_;
-  } else {
+  if (result == net::OK)
+    *backend_ = created_cache_;
+  else {
     LOG(ERROR) << "Unable to create cache";
     *backend_ = NULL;
     delete created_cache_;
@@ -160,5 +153,4 @@ int CreateCacheBackend(net::CacheType type, const base::FilePath& path,
   return creator->Run();
 }
 
-
 }  // namespace disk_cache

net/disk_cache/disk_cache_test_base.cc

@@ -268,11 +268,10 @@ void DiskCacheTestWithCache::CreateBackend(uint32 flags, base::Thread* thread) {
 
   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());
+    disk_cache::SimpleBackendImpl* simple_backend =
+        new disk_cache::SimpleBackendImpl(cache_path_, size_, type_,
+                                          make_scoped_refptr(runner), NULL);
+    int rv = simple_backend->Init(cb.callback());
     ASSERT_EQ(net::OK, cb.GetResult(rv));
     cache_ = simple_backend;
     return;

net/disk_cache/simple/simple_backend_impl.cc

@@ -23,8 +23,6 @@ using file_util::CreateDirectory;
 
 namespace {
 
-const char* kSimpleBackendSubdirectory = "Simple";
-
 // Must run on IO Thread.
 void DeleteBackendImpl(disk_cache::Backend** backend,
                        const net::CompletionCallback& callback,
@@ -39,32 +37,23 @@ void DeleteBackendImpl(disk_cache::Backend** backend,
 
 namespace disk_cache {
 
-// static
-int SimpleBackendImpl::CreateBackend(
-    const FilePath& full_path,
+SimpleBackendImpl::SimpleBackendImpl(
+    const FilePath& path,
     int max_bytes,
     net::CacheType type,
-    uint32 flags,
-    scoped_refptr<base::TaskRunner> cache_thread,
-    net::NetLog* net_log,
-    Backend** backend,
-    const CompletionCallback& callback) {
-  // TODO(gavinp): Use the |net_log|.
-  DCHECK_EQ(net::DISK_CACHE, type);
-  FilePath simple_cache_path =
-      full_path.AppendASCII(kSimpleBackendSubdirectory);
-
-  // In order to not leak when the EnsureCachePathExists fails, we need to
-  // delete this in DeleteBackendImpl on the IO Thread.
-  *backend = new SimpleBackendImpl(cache_thread, simple_cache_path);
-
-  cache_thread->PostTask(FROM_HERE,
-                         base::Bind(&SimpleBackendImpl::EnsureCachePathExists,
-                                    simple_cache_path,
-                                    cache_thread,
-                                    MessageLoopProxy::current(),
-                                    callback,
-                                    backend));
+    const scoped_refptr<base::TaskRunner>& cache_thread,
+    net::NetLog* net_log)
+  : path_(path),
+    cache_thread_(cache_thread) {
+  index_.reset(new SimpleIndex(cache_thread, path));
+}
+
+int SimpleBackendImpl::Init(const CompletionCallback& callback) {
+  cache_thread_->PostTask(FROM_HERE,
+                          base::Bind(&SimpleBackendImpl::InitializeIndex,
+                                     base::Unretained(this),
+                                     MessageLoopProxy::current(),
+                                     callback));
   return net::ERR_IO_PENDING;
 }
 
@@ -140,40 +129,15 @@ void SimpleBackendImpl::OnExternalCacheHit(const std::string& key) {
   NOTIMPLEMENTED();
 }
 
-SimpleBackendImpl::SimpleBackendImpl(
-    const scoped_refptr<base::TaskRunner>& cache_thread,
-    const FilePath& path) : path_(path) {
-  index_.reset(new SimpleIndex(cache_thread, path));
-}
-
-void SimpleBackendImpl::Initialize() {
-  index_->Initialize();
-}
-
-// static
-void SimpleBackendImpl::EnsureCachePathExists(
-    const FilePath& path,
-    const scoped_refptr<base::TaskRunner>& cache_thread,
-    const scoped_refptr<base::TaskRunner>& io_thread,
-    const CompletionCallback& callback,
-    Backend** backend) {
-  int result = net::OK;
-  if (!DirectoryExists(path) && !CreateDirectory(path))
-    result = net::ERR_FAILED;
-
-  if (result == net::OK) {
-    DCHECK(*backend);
-    // TODO(pasko): Move the object creation and initalization out of
-    // CreateBackend and fix this downcast.
-    static_cast<SimpleBackendImpl*>(*backend)->Initialize();
-    io_thread->PostTask(FROM_HERE,
-                        base::Bind(callback, result));
-  } else {
-    io_thread->PostTask(FROM_HERE,
-                        base::Bind(
-                            &DeleteBackendImpl,
-                            backend, callback, result));
-  }
+void SimpleBackendImpl::InitializeIndex(MessageLoopProxy* io_thread,
+                                        const CompletionCallback& callback) {
+  int rv = net::OK;
+  if (!file_util::PathExists(path_) && !file_util::CreateDirectory(path_)) {
+    LOG(ERROR) << "Simple Cache Backend: failed to create: " << path_.value();
+    rv = net::ERR_FAILED;
+  } else
+    rv = index_->Initialize() ? net::OK : net::ERR_FAILED;
+  io_thread->PostTask(FROM_HERE, base::Bind(callback, rv));
 }
 
 }  // namespace disk_cache

net/disk_cache/simple/simple_backend_impl.h

@@ -29,16 +29,14 @@ class SimpleIndex;
 
 class NET_EXPORT_PRIVATE SimpleBackendImpl : public Backend {
  public:
-  virtual ~SimpleBackendImpl();
+  SimpleBackendImpl(const base::FilePath& path, int max_bytes,
+                    net::CacheType type,
+                    const scoped_refptr<base::TaskRunner>& cache_thread,
+                    net::NetLog* net_log);
+
+  int Init(const CompletionCallback& callback);
 
-  static int CreateBackend(const base::FilePath& full_path,
-                           int max_bytes,
-                           net::CacheType type,
-                           uint32 flags,
-                           scoped_refptr<base::TaskRunner> cache_thread,
-                           net::NetLog* net_log,
-                           Backend** backend,
-                           const CompletionCallback& callback);
+  virtual ~SimpleBackendImpl();
 
   // From Backend:
   virtual net::CacheType GetCacheType() const OVERRIDE;
@@ -63,25 +61,14 @@ class NET_EXPORT_PRIVATE SimpleBackendImpl : public Backend {
   virtual void OnExternalCacheHit(const std::string& key) OVERRIDE;
 
  private:
-  SimpleBackendImpl(
-      const scoped_refptr<base::TaskRunner>& cache_thread,
-      const base::FilePath& path);
-
-  // Creates the Cache directory if needed. Performs blocking IO, so it cannot
-  // be called on IO thread.
-  static void EnsureCachePathExists(
-      const base::FilePath& path,
-      const scoped_refptr<base::TaskRunner>& cache_thread,
-      const scoped_refptr<base::TaskRunner>& io_thread,
-      const CompletionCallback& callback,
-      Backend** backend);
-
   // Must run on Cache Thread.
-  void Initialize();
+  void InitializeIndex(base::MessageLoopProxy* io_thread,
+                       const CompletionCallback& callback);
 
   const base::FilePath path_;
 
   scoped_ptr<SimpleIndex> index_;
+  const scoped_refptr<base::TaskRunner> cache_thread_;
 };
 
 }  // namespace disk_cache

net/disk_cache/simple/simple_index.cc

@@ -46,7 +46,7 @@ SimpleIndex::SimpleIndex(
     const base::FilePath& path)
     : path_(path),
       cache_thread_(cache_thread) {
-  index_filename_ = path_.AppendASCII("simple-index");
+  index_filename_ = path_.AppendASCII("index");
 }
 
 bool SimpleIndex::Initialize() {

net/disk_cache/simple/simple_synchronous_entry.cc

@@ -51,8 +51,7 @@ int32 DataSizeFromKeyAndFileSize(size_t key_size, int64 file_size) {
 }
 
 int64 FileOffsetFromDataOffset(size_t key_size, int data_offset) {
-  const int64 headers_size = sizeof(disk_cache::SimpleFileHeader) +
-                             key_size;
+  const int64 headers_size = sizeof(disk_cache::SimpleFileHeader) + key_size;
   return headers_size + data_offset;
 }