Add new simple disk cache backend.

net/disk_cache/very_simple/very_simple_backend_impl.cc

+// Copyright (c) 2013 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 "net/disk_cache/very_simple/very_simple_backend_impl.h"
+
+#include "base/bind.h"
+#include "base/callback.h"
+#include "base/file_util.h"
+#include "base/message_loop_proxy.h"
+#include "base/threading/worker_pool.h"
+#include "net/base/net_errors.h"
+#include "net/disk_cache/very_simple/very_simple_entry_impl.h"

[rvargas (doing something else), 2013/02/06 03:28:40]

How about renaming everything to simple, instead of very_simple?

[gavinp, 2013/02/08 23:17:51]

Done.

+
+using base::Bind;

[pasko-google - do not use, 2013/02/05 21:20:33]

nit:

although allowed by our style, in net/ the uses are via base::Bind, should keep
it consistent

[gavinp, 2013/02/08 23:17:51]

Done, although with grumpiness.

[pasko-google - do not use, 2013/02/11 13:59:25]

I understand your pain and grumpiness, please also understand that Grep, that
poor thing, can be grumpier.

+using base::Callback;
+using base::MessageLoopProxy;
+using base::Time;
+using base::WorkerPool;
+
+namespace disk_cache {
+
+namespace {

[rvargas (doing something else), 2013/02/06 03:28:40]

nit: can this be outside of disk_cache namespace?

[gavinp, 2013/02/08 23:17:51]

Done.

+
+typedef Callback<VerySimpleBackendImpl*(void)> FactoryCallback;
+
+void EnsureCachePathExists(
+    const FilePath& path,
+    const scoped_refptr<base::TaskRunner> callback_runner,
+    const Backend::CompletionCallback& callback) {
+  int result = net::OK;
+  if (!file_util::DirectoryExists(path) && !file_util::CreateDirectory(path))
+    result = net::ERR_FAILED;
+  callback_runner->PostTask(FROM_HERE, Bind(callback, result));
+}
+
+void OnCachePathCreated(const FactoryCallback& factory_callback,
+                        disk_cache::Backend** backend,
+                        const Backend::CompletionCallback& callback,
+                        int result) {
+  if (result == net::OK)
+    *backend = factory_callback.Run();
+  else
+    *backend = NULL;
+  callback.Run(result);
+}
+
+}  // namespace
+
+VerySimpleBackendImpl::BackendFactory::BackendFactory(const FilePath& path)
+    : path_(path) {
+  VLOG(0) << "VerySimpleBackendImpl::BackendFactory::CreateBackend()";

[pasko-google - do not use, 2013/02/05 21:20:33]

identical message as below, high log level, please remove one

[rvargas (doing something else), 2013/02/06 03:28:40]

Don't add tracing logs

[gavinp, 2013/02/08 23:17:51]

Done.

+}
+
+VerySimpleBackendImpl::BackendFactory::~BackendFactory() {
+}
+
+int VerySimpleBackendImpl::BackendFactory::CreateBackend(
+    net::NetLog* net_log,
+    disk_cache::Backend** backend,
+    const CompletionCallback& callback) {
+  VLOG(0) << "VerySimpleBackendImpl::BackendFactory::CreateBackend()";
+
+  base::WorkerPool::PostTask(

[rvargas (doing something else), 2013/02/06 03:28:40]

nit: first argument here.

[gavinp, 2013/02/08 23:17:51]

Done.

+      FROM_HERE, Bind(&EnsureCachePathExists, path_,
+                      MessageLoopProxy::current(),
+                      Bind(&OnCachePathCreated,
+                           Bind(&VerySimpleBackendImpl::Create,

[rvargas (doing something else), 2013/02/06 03:28:40]

wow, three nested Binds :(

Can we just split this instead of trying to control the whole process from one
place (here)?

Hopefully removing BackendFactory makes things a little easier to follow.

[gavinp, 2013/02/08 23:17:51]

Goes to show that a dedicated programmer can write a LISP program in any
language. I'll make this chain more explicit and less front loaded, in the C++
style.

+                                path_),
+                           backend, callback)),
+      true);
+  return net::ERR_IO_PENDING;
+}
+
+VerySimpleBackendImpl::~VerySimpleBackendImpl() {
+  DCHECK(thread_checker_.CalledOnValidThread());

[rvargas (doing something else), 2013/02/06 03:28:40]

nit: seems overkill, considering that all methods are used on the same thread,
and the default for chrome code is that classes are single-thread (so it would
follow that all classes would have to do this)

[gavinp, 2013/02/08 23:17:51]

I agree it is overkill technically. I am including it for documentation
purposes.

Reading the cache code for threading in the past few weeks, I've found myself
spending a lot of time tracing paths so I could be completely certain which
thread we were on at each point.

With these DCHECKs, anyone reading the code has an easy reference they can
check. And it's far more reliable than a comment. If a future editor of this
code is as bad at remembering which method is used on which thread as I am, then
they will be happy these were here.

[pasko-google - do not use, 2013/02/11 13:59:25]

+1, these DCHECKs are good for documentation

[rvargas (doing something else), 2013/02/13 01:48:46]

I would agree with that if the class had a mixed thread use (last time I looked
at it that was not the case, maybe that changed). If the class is not used from
multiple threads, can't we just document the behavior with a comment on the
header file?

I'm seriously concerned with the logic saying that we should add a
thread_checker to document the default coding convention of Chrome because that
leads to most classes having a thread checker.

[gavinp, 2013/02/14 15:29:55]

Done.

+}
+
+net::CacheType VerySimpleBackendImpl::GetCacheType() const {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  return net::DISK_CACHE;
+}
+
+int32 VerySimpleBackendImpl::GetEntryCount() const {
+  return 0;

[rvargas (doing something else), 2013/02/06 03:28:40]

Todo?

[gavinp, 2013/02/08 23:17:51]

Todo!

+}
+
+int VerySimpleBackendImpl::OpenEntry(const std::string& key,
+                                     Entry** entry,
+                                     const CompletionCallback& callback) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  return VerySimpleEntryImpl::OpenEntry(path_, key, entry, callback);
+}
+
+int VerySimpleBackendImpl::CreateEntry(const std::string& key,
+                                       Entry** entry,
+                                       const CompletionCallback& callback) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  return VerySimpleEntryImpl::CreateEntry(path_, key, entry, callback);
+}
+
+int VerySimpleBackendImpl::DoomEntry(const std::string& key,
+                                     const net::CompletionCallback& callback) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  return VerySimpleEntryImpl::DoomEntry(path_, key, callback);
+}
+
+int VerySimpleBackendImpl::DoomAllEntries(const CompletionCallback& callback) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  return net::ERR_FAILED;

[rvargas (doing something else), 2013/02/06 03:28:40]

Todo?  etc

[gavinp, 2013/02/08 23:17:51]

Done.

+}
+
+int VerySimpleBackendImpl::DoomEntriesBetween(
+    const Time initial_time,
+    const Time end_time,
+    const CompletionCallback& callback) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  return net::ERR_FAILED;
+}
+
+int VerySimpleBackendImpl::DoomEntriesSince(
+    const Time initial_time,
+    const CompletionCallback& callback) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  return net::ERR_FAILED;
+}
+
+int VerySimpleBackendImpl::OpenNextEntry(void** iter,
+                                         Entry** next_entry,
+                                         const CompletionCallback& callback) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  return net::ERR_FAILED;

[pasko-google - do not use, 2013/02/05 21:20:33]

Just curious, do the callers always workaround such kind of failures gracefully?
If they sometimes misbehave in such situations, better put a deadly DCHECK here.

[gavinp, 2013/02/08 23:17:51]

I'll go deadly DCHECK, since I don't know.

+}
+
+void VerySimpleBackendImpl::EndEnumeration(void** iter) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+}
+
+void VerySimpleBackendImpl::GetStats(
+    std::vector<std::pair<std::string, std::string> >* stats) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+}
+
+void VerySimpleBackendImpl::OnExternalCacheHit(const std::string& key) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+}
+
+VerySimpleBackendImpl::VerySimpleBackendImpl(
+    const FilePath& path) : path_(path) {

[rvargas (doing something else), 2013/02/06 03:28:40]

The general idea is that at this point contents of the cache are read and any
access issue should be resolved/reported.

[gavinp, 2013/02/08 23:17:51]

Yes. That was true in the patch you were reviewing, and that's true in the new
upload too.

[rvargas (doing something else), 2013/02/13 01:48:46]

What I meant is that the structure of SimpleBackendImpl::OnCachePathCreated()
breaks as soon as you add the meat to this method, so it would be better to have
the structure correct from the get go. But I'm OK with adding code that works
today but not later on if that's what you want.

[gavinp, 2013/02/14 15:29:55]

Yeah. Currently Egor plans to chase this and get it right.

+}
+
+// static
+VerySimpleBackendImpl* VerySimpleBackendImpl::Create(const FilePath& path) {
+  return new VerySimpleBackendImpl(path);

[pasko-google - do not use, 2013/02/05 21:20:33]

simpler would be to do this one liner in OnCachePathCreated. I am having a
constant buzz in my head with various kinds of Create, Factory, Callback etc.
Reducing the number of them would help reducing the buzz.

[gavinp, 2013/02/08 23:17:51]

Happy to. I think I was being too averse to making the header file complex; but
adding a one line factory method just to avoid putting OnCachePathCreated in the
class is... me being silly. Thanks. The new upload looks a lot more like C++.

+}
+
+}  // namespace disk_cache