Separate Simple Backend creation from initialization.

net/disk_cache/cache_creator.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 "base/file_util.h"
 #include "base/metrics/field_trial.h"
 #include "base/stringprintf.h"
 #include "net/base/net_errors.h"
 #include "net/disk_cache/backend_impl.h"
 #include "net/disk_cache/cache_util.h"
@@ skipping 31 matching lines @@
   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,
     net::CacheType type, uint32 flags,
     base::MessageLoopProxy* thread, net::NetLog* net_log,
     disk_cache::Backend** backend,
     const net::CompletionCallback& callback)
     : path_(path),
       force_(force),
       retry_(false),
       max_bytes_(max_bytes),
       type_(type),
       flags_(flags),
       thread_(thread),
       backend_(backend),
       callback_(callback),
       created_cache_(NULL),
-      net_log_(net_log),
-      use_simple_cache_backend_(false) {
+      net_log_(net_log) {
   }
 
 CacheCreator::~CacheCreator() {
 }
 
 int CacheCreator::Run() {
   // TODO(pasko): The two caches should never coexist on disk. Each individual
   // cache backend should fail to initialize if it observes the index that does
   // not belong to it.
   if (base::FieldTrialList::FindFullName("SimpleCacheTrial") == "Yes") {
     // TODO(gavinp,pasko): While simple backend development proceeds, we're only
     // 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 =
       new disk_cache::BackendImpl(path_, thread_, net_log_);
   created_cache_ = new_cache;
   new_cache->SetMaxSize(max_bytes_);
   new_cache->SetType(type_);
   new_cache->SetFlags(flags_);
   int rv = new_cache->Init(
       base::Bind(&CacheCreator::OnIOComplete, base::Unretained(this)));
   DCHECK_EQ(net::ERR_IO_PENDING, rv);
   return rv;
 }
 
 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_;
+    *backend_ = created_cache_;
 #ifdef USE_TRACING_CACHE_BACKEND

[rvargas (doing something else), 2013/04/09 19:16:07]

I should not be seeing this!

[gavinp, 2013/04/10 10:04:39]

I'm confused. Since this CL is downstream from
https://codereview.chromium.org/13731002 , isn't the line you're commenting on
actually https://codereview.chromium.org/13731002#pair-118 ? Isn't CL 13731002
the place to comment on this line?

I just spoke to Egor, he says this CL rebases onto trunk fine.

[rvargas (doing something else), 2013/04/10 18:50:11]

No, I'm not saying that I don't understand what this line is about. I'm saying
that this CL doesn't depend on anything else to land, and a code review should
be performed against the _current_ state of the tree, not against the state of
the code on a developer's machine.

So I should not be seeing lines in the "old" code that don't exist on TOT, that
simple. It's just a request to follow regular code review practices.

-    *backend_ = new disk_cache::TracingCacheBackend(*backend_);
+    *backend_ = new disk_cache::TracingCacheBackend(created_cache_);
 #endif
   } else {
     LOG(ERROR) << "Unable to create cache";
     *backend_ = NULL;
     delete created_cache_;
   }
   callback_.Run(result);
   delete this;
 }
 
@@ skipping 31 matching lines @@
     *backend = disk_cache::MemBackendImpl::CreateBackend(max_bytes, net_log);
     return *backend ? net::OK : net::ERR_FAILED;
   }
   DCHECK(thread);
   CacheCreator* creator = new CacheCreator(path, force, max_bytes, type, kNone,
                                            thread, net_log, backend, callback);
   return creator->Run();
 }
 
 }  // namespace disk_cache