Offline By Default

services/url_response_disk_cache/url_response_disk_cache_impl.cc

 // Copyright 2015 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 "services/url_response_disk_cache/url_response_disk_cache_impl.h"
 
 #include <dirent.h>
 #include <fcntl.h>
 #include <sys/types.h>
 
 #include <type_traits>
 
 #include "base/bind.h"
+#include "base/callback_helpers.h"
 #include "base/files/file.h"
 #include "base/files/file_util.h"
 #include "base/location.h"
 #include "base/logging.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/trace_event/trace_event.h"
+#include "crypto/random.h"
 #include "mojo/data_pipe_utils/data_pipe_utils.h"
 #include "mojo/public/cpp/application/application_connection.h"
 #include "mojo/public/cpp/bindings/lib/fixed_buffer.h"
+#include "mojo/public/interfaces/network/http_header.mojom.h"
 #include "services/url_response_disk_cache/url_response_disk_cache_entry.mojom.h"
 #include "third_party/zlib/google/zip_reader.h"
 #include "url/gurl.h"
 
 namespace mojo {
 
 namespace {
 
 // The current version of the cache. This should only be incremented. When this
 // is incremented, all current cache entries will be invalidated.
-const uint32_t kCurrentVersion = 2;
+const uint32_t kCurrentVersion = 0;
+
+// The delay to wait before starting deleting data. This is delayed to not
+// interfere with the shell startup.
+const uint32_t kTrashDelayInSeconds = 60;
 
 const char kEtagHeader[] = "etag";
 
-const char kEntryName[] = "entry";
-const char kEntryTmpName[] = "entry.tmp";
-
-// TODO(darin): These Serialize / Deserialize methods should not live here.
-// They use private details of the bindings system. Instead, we should provide
-// these as helper functions under mojo/public/cpp/bindings/.
-
-template <typename T>
-void Serialize(T input, std::string* output) {
-  typedef typename mojo::internal::WrapperTraits<T>::DataType DataType;
-  size_t size = GetSerializedSize_(input);
-
-  output->clear();
-  output->resize(size);
-
-  mojo::internal::FixedBuffer buf;
-  buf.Initialize(&output->at(0), size);
-
-  DataType data_type;
-  Serialize_(input.Pass(), &buf, &data_type);
-  std::vector<Handle> handles;
-  data_type->EncodePointersAndHandles(&handles);
+std::string GetNewIdentifier() {

[ppi, 2015/09/15 15:21:23]

What is the identifier for? If this is to identify pending update requests, can
we say that in the comment? Should we call the function
GetNewUpdateIdentifier()? Or GetNewUpdateId()?

[qsr, 2015/09/16 11:46:38]

Added comment. This is not to identify a request -> it is an unique id that
serves to create the entry directory. I cannot use the safer method of creating
a directory because I need to create it in the staging directory and then move
it to the final one.

+  char bytes[32];
+  crypto::RandBytes(bytes, arraysize(bytes));
+  return base::HexEncode(bytes, arraysize(bytes));
 }
 
-template <typename T>
-bool Deserialize(std::string input, T* output) {
-  typedef typename mojo::internal::WrapperTraits<T>::DataType DataType;
-  mojo::internal::BoundsChecker bounds_checker(&input[0], input.size(), 0);
-  if (!std::remove_pointer<DataType>::type::Validate(&input[0],
-                                                     &bounds_checker)) {
-    return false;
+void DoNothing(const base::FilePath& fp1, const base::FilePath& fp2) {}
+
+void MovePathIntoDir(const base::FilePath& source,
+                     const base::FilePath& destination) {
+  if (!PathExists(source))
+    return;
+
+  base::FilePath tmp_dir;
+  base::CreateTemporaryDirInDir(destination, "", &tmp_dir);
+  base::File::Error error;
+  if (!base::ReplaceFile(source, tmp_dir, &error)) {
+    LOG(ERROR) << "Failed to clear dir content: " << error;
   }
-  DataType data_type = reinterpret_cast<DataType>(&input[0]);
-  std::vector<Handle> handles;
-  data_type->DecodePointersAndHandles(&handles);
-  Deserialize_(data_type, output);
-  return true;
 }
 
-void SaveEntry(CacheEntryPtr entry, base::ScopedFD dir) {
-  TRACE_EVENT0("url_response_disk_cache", "SaveEntry");
-
-  std::string serialized_entry;
-  Serialize(entry.Pass(), &serialized_entry);
-  DCHECK_LT(serialized_entry.size(),
-            static_cast<size_t>(std::numeric_limits<int>::max()));
-
-  int file_fd =
-      HANDLE_EINTR(openat(dir.get(), kEntryTmpName, O_WRONLY | O_CREAT, 0600));
-  if (file_fd < 0)
-    return;
-  base::File file(file_fd);
-  if (file.WriteAtCurrentPos(serialized_entry.data(),
-                             serialized_entry.size()) !=
-      static_cast<int>(serialized_entry.size()))
-    return;
-  // The file must be closed before the file is moved.
-  file.Close();
-  renameat(dir.get(), kEntryTmpName, dir.get(), kEntryName);
+void ClearTrashDir(scoped_refptr<base::TaskRunner> task_runner,
+                   const base::FilePath& trash_dir) {
+  // Delete the trash directory.
+  task_runner->PostDelayedTask(
+      FROM_HERE,
+      base::Bind(base::IgnoreResult(&base::DeleteFile), trash_dir, true),
+      base::TimeDelta::FromSeconds(kTrashDelayInSeconds));
 }
 
 Array<uint8_t> PathToArray(const base::FilePath& path) {
   if (path.empty())
     return Array<uint8_t>();
   const std::string& string = path.value();
   Array<uint8_t> result(string.size());
   memcpy(&result.front(), string.data(), string.size());
   return result.Pass();
 }
 
 // This method remove the query string of an url if one is present. It does
 // match the behavior of the application manager, which connects to the same app
 // if requested twice with different query parameters.
 std::string CanonicalizeURL(const std::string& url) {
   GURL gurl(url);
 
   if (!gurl.has_query()) {
     return gurl.spec();
   }
 
   GURL::Replacements repl;
   repl.SetQueryStr("");
   std::string result = gurl.ReplaceComponents(repl).spec();
   // Remove the dangling '?' because it's ugly.
   base::ReplaceChars(result, "?", "", &result);
   return result;
 }
 
-// Encode a string in ascii. This uses _ as an escape character. It also escapes
-// ':' because it is an usual path separator, and '#' because dart refuses it in
-// URLs.
-std::string EncodeString(const std::string& string) {
-  std::string result = "";
-  for (size_t i = 0; i < string.size(); ++i) {
-    unsigned char c = string[i];
-    if (c >= 32 && c < 128 && c != '_' && c != ':' && c != '#') {
-      result += c;
-    } else {
-      result += base::StringPrintf("_%02x", c);
-    }
-  }
-  return result;
-}
-
 // This service use a directory under HOME to store all of its data,
 base::FilePath GetBaseDirectory() {
   return base::FilePath(getenv("HOME")).Append(".mojo_url_response_disk_cache");
 }
 
+// Returns the directory containing live data for the cache.
 base::FilePath GetCacheDirectory() {
   return GetBaseDirectory().Append("cache");
 }
 
+// Returns a temporary that will be deleted after startup. This is used to have
+// a consistent directory for outdated files in case the trash process doesn't
+// finish.
 base::FilePath GetTrashDirectory() {
   return GetBaseDirectory().Append("trash");
 }
 
-// Returns the directory used store cached data for the given |url|, under
-// |base_directory|.
-base::FilePath GetDirName(base::FilePath base_directory,
-                          const std::string& url) {
-  // TODO(qsr): If the speed of directory traversal is problematic, this might
-  // need to change to use less directories.
-  return base_directory.Append(EncodeString(CanonicalizeURL(url)));
+// Returns a staging directory to save file before an entry can be saved in the
+// database. This directory is deleted when the service is started. This is used
+// to prevent leaking files if there is an interuption while downloading a

[ppi, 2015/09/15 15:21:23]

s/interuption/interruption/

[qsr, 2015/09/16 11:46:38]

Done.

+// response.
+base::FilePath GetStagingDirectory() {
+  return GetBaseDirectory().Append("staging");
 }
 
-// Returns the directory that the consumer can use to cache its own data.
-base::FilePath GetConsumerCacheDirectory(const base::FilePath& main_cache) {
-  return main_cache.Append("consumer_cache");
+// Returns path of the directory that the consumer can use to cache its own
+// data.
+base::FilePath GetConsumerCacheDirectory(
+    const base::FilePath& cache_directory) {
+  return cache_directory.Append("consumer_cache");
 }
 
 // Returns the path of the sentinel used to keep track of a zipped response has

[ppi, 2015/09/15 15:21:22]

s/used to keep track of/used to mark that/

[qsr, 2015/09/16 11:46:38]

Done.

 // already been extracted.
-base::FilePath GetExtractedSentinel(const base::FilePath& main_cache) {
-  return main_cache.Append("extracted_sentinel");
+base::FilePath GetExtractedSentinel(const base::FilePath& cache_directory) {

[ppi, 2015/09/15 15:21:23]

nit: wdyt about GetExtractionSentinel?

[qsr, 2015/09/16 11:46:38]

Done.

+  return cache_directory.Append("extracted_sentinel");
+}
+
+// Returns the path of the directory when a zipped content is extracted.

[ppi, 2015/09/15 15:21:23]

s/when/where/

[qsr, 2015/09/16 11:46:38]

Done.

+base::FilePath GetExtractedDirectory(const base::FilePath& cache_directory) {

[ppi, 2015/09/15 15:21:23]

if "cache_directory" here is per-entry, I'd suggest calling it
"entry_directory". "cache_directory" sounds global.

[ppi, 2015/09/15 15:21:23]

nit: GetExtractionDirectiory?

[qsr, 2015/09/16 11:46:38]

Done.

[qsr, 2015/09/16 11:46:38]

Done.

+  return cache_directory.Append("extracted");
 }
 
 // Runs the given callback. If |success| is false, call back with an error.
-// Otherwise, store |entry| in |entry_path|, then call back with the given
-// paths.
+// Otherwise, store a new entry in the databse, then call back with the content
+// path and the consumer cache path.
 void RunCallbackWithSuccess(
     const URLResponseDiskCacheImpl::FilePathPairCallback& callback,
-    const base::FilePath& content_path,
-    const base::FilePath& cache_dir,
-    const base::FilePath& entry_path,
-    CacheEntryPtr entry,
-    base::TaskRunner* task_runner,
+    const std::string& identifier,
+    const std::string& request_origin,
+    const std::string& url,
+    URLResponsePtr response,
+    scoped_refptr<URLResponseDiskCacheDB> db,
+    scoped_refptr<base::TaskRunner> task_runner,
     bool success) {
-  TRACE_EVENT1("url_response_disk_cache", "RunCallbackWithSuccess",
-               "content_path", content_path.value());
+  TRACE_EVENT2("url_response_disk_cache", "RunCallbackWithSuccess", "url", url,
+               "identifier", identifier);
   if (!success) {
     callback.Run(base::FilePath(), base::FilePath());
     return;
   }
 
-  // Save the entry in a background thread. The entry directory is opened and
-  // passed because if this service wants to replace the content later on, it
-  // will start by moving the current directory.
-  base::ScopedFD dir(HANDLE_EINTR(
-      open(entry_path.DirName().value().c_str(), O_RDONLY | O_DIRECTORY)));
-  task_runner->PostTask(FROM_HERE,
-                        base::Bind(&SaveEntry, base::Passed(entry.Pass()),
-                                   base::Passed(dir.Pass())));
+  base::FilePath staging_content = GetStagingDirectory().Append(identifier);

[ppi, 2015/09/15 15:21:22]

nit: how about "staged_content_path"?

[qsr, 2015/09/16 11:46:38]

Used staged_response_body_path

+  base::FilePath cache_directory = GetCacheDirectory().Append(identifier);

[ppi, 2015/09/15 15:21:23]

nit: how about "entry_directory"?

[qsr, 2015/09/16 11:46:38]

Done.

+  base::FilePath content_path = cache_directory.Append(identifier);

[ppi, 2015/09/15 15:21:23]

nit: how about "target_content_path"?

[qsr, 2015/09/16 11:46:38]

Used response_body_path

+  base::FilePath consumer_cache_directory =
+      GetConsumerCacheDirectory(cache_directory);
 
-  callback.Run(content_path, cache_dir);
+  CacheEntryPtr entry = CacheEntry::New();
+  entry->response = response.Pass();
+  entry->cache_directory = cache_directory.value();
+  entry->content_path = content_path.value();
+
+  db->PutNewest(request_origin, url, entry.Pass());
+
+  if (!base::CreateDirectoryAndGetError(cache_directory, nullptr) ||
+      !base::CreateDirectoryAndGetError(consumer_cache_directory, nullptr) ||
+      !base::ReplaceFile(staging_content, content_path, nullptr)) {
+    MovePathIntoDir(cache_directory, GetStagingDirectory());
+    callback.Run(base::FilePath(), base::FilePath());
+    return;
+  }
+
+  callback.Run(content_path, consumer_cache_directory);
 }
 
 // Run the given mojo callback with the given paths.
 void RunMojoCallback(
     const Callback<void(Array<uint8_t>, Array<uint8_t>)>& callback,
     const base::FilePath& path1,
     const base::FilePath& path2) {
   callback.Run(PathToArray(path1), PathToArray(path2));
 }
 
 // Returns the list of values for the given |header_name| in the given list of
 // headers.
-template <typename HeaderType>
 std::vector<std::string> GetHeaderValues(const std::string& header_name,
-                                         const Array<HeaderType>& headers) {
+                                         const Array<HttpHeaderPtr>& headers) {
   std::vector<std::string> result;
   for (size_t i = 0u; i < headers.size(); ++i) {
-    std::string name = headers[i]->name;
+    const std::string& name = headers[i]->name.get();
     if (base::LowerCaseEqualsASCII(name, header_name.c_str()))
       result.push_back(headers[i]->value);
   }
   return result;
 }
 
-// Returns whether the given directory |dir| constains a valid entry file for
-// the given |response|. If this is the case and |output| is not |nullptr|, then
-// the deserialized entry is returned in |*output|.
-bool IsCacheEntryValid(const base::FilePath& dir,
-                       URLResponse* response,
-                       CacheEntryPtr* output) {
-  // Find the entry file, and deserialize it.
-  base::FilePath entry_path = dir.Append(kEntryName);
-  if (!base::PathExists(entry_path))
-    return false;
-  std::string serialized_entry;
-  if (!ReadFileToString(entry_path, &serialized_entry))
-    return false;
-  CacheEntryPtr entry;
-  if (!Deserialize(serialized_entry, &entry))
-    return false;
+// Returns whether the given |entry| is valid.
+bool IsCacheEntryValid(const CacheEntryPtr& entry) {
+  return entry && PathExists(base::FilePath(entry->content_path));
+}
 
-  // Obsolete entries are invalidated.
-  if (entry->version != kCurrentVersion)
+// Returns whether the given directory |entry| is valid and its content can be
+// used for the given |response|.
+bool IsCacheEntryFresh(const URLResponsePtr& response,
+                       const CacheEntryPtr& entry) {
+  DCHECK(response);
+  if (!IsCacheEntryValid(entry))
     return false;
 
   // If |entry| or |response| has not headers, it is not possible to check if
   // the entry is valid, so returns |false|.
-  if (entry->headers.is_null() || response->headers.is_null())
+  if (entry->response->headers.is_null() || response->headers.is_null())
     return false;
 
   // Only handle etag for the moment.
   std::string etag_header_name = kEtagHeader;
   std::vector<std::string> entry_etags =
-      GetHeaderValues(etag_header_name, entry->headers);
+      GetHeaderValues(etag_header_name, entry->response->headers);
   if (entry_etags.size() == 0)
     return false;
   std::vector<std::string> response_etags =
       GetHeaderValues(etag_header_name, response->headers);
   if (response_etags.size() == 0)
     return false;
 
   // Looking for the first etag header.
-  bool result = entry_etags[0] == response_etags[0];
-
-  // Returns |entry| if requested.
-  if (output)
-    *output = entry.Pass();
-
-  return result;
+  return entry_etags[0] == response_etags[0];
+}
+
+void PruneCache(scoped_refptr<URLResponseDiskCacheDB> db,

[ppi, 2015/09/15 15:21:22]

"Prune" usually means reduce to meet some limit. Should we call this "EvictAll"
?

[qsr, 2015/09/16 11:46:38]

Which is what we do. We keep an entry per key.

+                const scoped_ptr<URLResponseDiskCacheDB::Iterator>& iterator) {
+  CacheKeyPtr last_key;
+  CacheKeyPtr key;
+  CacheEntryPtr entry;
+  while (iterator->HasNext()) {
+    iterator->GetNext(&key, &entry);
+    if (last_key && last_key->request_origin == key->request_origin &&
+        last_key->url == key->url) {
+      base::FilePath cache_directory = base::FilePath(entry->cache_directory);
+      if (base::DeleteFile(cache_directory, true))
+        db->Delete(key.Clone());
+    }
+    last_key = key.Pass();
+  }
 }
 
 }  // namespace
 
 // static
-void URLResponseDiskCacheImpl::ClearCache(base::TaskRunner* task_runner) {
-  // Create a unique subdirectory in trash.
+scoped_refptr<URLResponseDiskCacheDB> URLResponseDiskCacheImpl::CreateDB(
+    scoped_refptr<base::TaskRunner> task_runner,
+    bool force_clean) {
+  // Create the trash directory if needed.
   base::FilePath trash_dir = GetTrashDirectory();
   base::CreateDirectory(trash_dir);
-  base::FilePath dest_dir;
-  base::CreateTemporaryDirInDir(trash_dir, "", &dest_dir);
-
-  // Move the current cache directory, if present, into trash.
-  base::FilePath cache_dir = GetCacheDirectory();
-  if (PathExists(cache_dir)) {
-    base::File::Error error;
-    if (!base::ReplaceFile(cache_dir, dest_dir, &error)) {
-      LOG(ERROR) << "Failed to clear cache content: " << error;
+
+  // Clean the trash directory when exiting this method.
+  base::ScopedClosureRunner trash_cleanup(
+      base::Bind(&ClearTrashDir, task_runner, trash_dir));
+
+  // Move the staging directory to trash.
+  MovePathIntoDir(GetStagingDirectory(), trash_dir);
+  // And recreate it.
+  base::CreateDirectory(GetStagingDirectory());
+
+  base::FilePath db_path = GetBaseDirectory().Append("db");
+
+  if (!force_clean && PathExists(db_path)) {
+    scoped_refptr<URLResponseDiskCacheDB> db =
+        new URLResponseDiskCacheDB(db_path);
+    if (db->GetVersion() == kCurrentVersion) {
+      task_runner->PostDelayedTask(
+          FROM_HERE,
+          base::Bind(&PruneCache, db, base::Passed(db->Iterate())),
+          base::TimeDelta::FromSeconds(kTrashDelayInSeconds));
+      return db;
     }
-  }
-
-  // Delete the trash directory.
-  task_runner->PostTask(
-      FROM_HERE,
-      base::Bind(base::IgnoreResult(&base::DeleteFile), trash_dir, true));
+    db = nullptr;
+  }
+
+  // Move the database to trash.

[ppi, 2015/09/15 15:21:23]

should these two be in 

 if (force_clean) {

?

[qsr, 2015/09/16 11:46:38]

No. If we do not want to clean the db, we would have returned on line 289. At
this point we need to clean the database, whether because it was forced, or
because it was of an other version.

+  MovePathIntoDir(db_path, trash_dir);
+  // Move the current cache content to trash.
+  MovePathIntoDir(GetCacheDirectory(), trash_dir);
+
+  scoped_refptr<URLResponseDiskCacheDB> result =
+      new URLResponseDiskCacheDB(db_path);
+  result->SetVersion(kCurrentVersion);
+  return result;
 }
 
 URLResponseDiskCacheImpl::URLResponseDiskCacheImpl(
-    base::TaskRunner* task_runner,
+    scoped_refptr<base::TaskRunner> task_runner,
+    scoped_refptr<URLResponseDiskCacheDB> db,
     const std::string& remote_application_url,
     InterfaceRequest<URLResponseDiskCache> request)
-    : task_runner_(task_runner), binding_(this, request.Pass()) {
-  base_directory_ = GetCacheDirectory();
-  // The cached files are shared only for application of the same origin.
-  if (remote_application_url != "") {
-    base_directory_ = base_directory_.Append(
-        EncodeString(GURL(remote_application_url).GetOrigin().spec()));
-  }
+    : task_runner_(task_runner), db_(db), binding_(this, request.Pass()) {
+  request_origin_ = GURL(remote_application_url).GetOrigin().spec();
+  cache_directory_ = GetCacheDirectory();
 }
 
 URLResponseDiskCacheImpl::~URLResponseDiskCacheImpl() {
 }
 
-void URLResponseDiskCacheImpl::GetFile(URLResponsePtr response,
-                                       const GetFileCallback& callback) {
-  return GetFileInternal(response.Pass(),
-                         base::Bind(&RunMojoCallback, callback));
-}
-
-void URLResponseDiskCacheImpl::GetExtractedContent(
+void URLResponseDiskCacheImpl::Get(const String& url,
+                                   const GetCallback& callback) {
+  CacheEntryPtr entry = db_->GetNewest(request_origin_, CanonicalizeURL(url));
+  if (!IsCacheEntryValid(entry)) {
+    callback.Run(URLResponsePtr(), Array<uint8_t>(), Array<uint8_t>());
+    return;
+  }
+  callback.Run(
+      entry->response.Pass(), PathToArray(base::FilePath(entry->content_path)),
+      PathToArray(
+          GetConsumerCacheDirectory(base::FilePath(entry->cache_directory))));
+}
+
+void URLResponseDiskCacheImpl::Update(URLResponsePtr response) {
+  UpdateAndGetInternal(response.Pass(), base::Bind(&DoNothing));
+}
+
+void URLResponseDiskCacheImpl::UpdateAndGet(
     URLResponsePtr response,
-    const GetExtractedContentCallback& callback) {
-  base::FilePath dir = GetDirName(base_directory_, response->url);
-  base::FilePath extracted_dir = dir.Append("extracted");
-  if (IsCacheEntryValid(dir, response.get(), nullptr) &&
-      PathExists(GetExtractedSentinel(dir))) {
-    callback.Run(PathToArray(extracted_dir),
-                 PathToArray(GetConsumerCacheDirectory(dir)));
-    return;
-  }
-
-  GetFileInternal(
-      response.Pass(),
-      base::Bind(&URLResponseDiskCacheImpl::GetExtractedContentInternal,
-                 base::Unretained(this), base::Bind(&RunMojoCallback, callback),
-                 dir, extracted_dir));
-}
-
-void URLResponseDiskCacheImpl::GetFileInternal(
+    const UpdateAndGetCallback& callback) {
+  UpdateAndGetInternal(response.Pass(), base::Bind(&RunMojoCallback, callback));
+}
+
+void URLResponseDiskCacheImpl::UpdateAndGetExtracted(
+    URLResponsePtr response,
+    const UpdateAndGetExtractedCallback& callback) {
+  if (response->error ||
+      (response->status_code >= 400 && response->status_code < 600)) {
+    callback.Run(Array<uint8_t>(), Array<uint8_t>());
+    return;
+  }
+
+  std::string url = CanonicalizeURL(response->url);
+
+  // Check if the response is cached and valid. If that's the case, returns the
+  // cached value.
+  CacheEntryPtr entry = db_->GetNewest(request_origin_, url);
+
+  if (!IsCacheEntryFresh(response, entry)) {
+    UpdateAndGetInternal(
+        response.Pass(),
+        base::Bind(&URLResponseDiskCacheImpl::UpdateAndGetExtractedInternal,
+                   base::Unretained(this),
+                   base::Bind(&RunMojoCallback, callback)));
+    return;
+  }
+
+  base::FilePath cache_directory = base::FilePath(entry->cache_directory);
+  base::FilePath extracted_directory = GetExtractedDirectory(cache_directory);
+  if (PathExists(GetExtractedSentinel(cache_directory))) {

[ppi, 2015/09/15 15:21:23]

Should we invert the flow to match the other if above?

if (there is an issue) {
  UpdateAndGetInternal(...)
}

callback.Run(...)

[qsr, 2015/09/16 11:46:38]

Done.

+    callback.Run(PathToArray(extracted_directory),
+                 PathToArray(GetConsumerCacheDirectory(cache_directory)));
+    return;
+  }
+
+  UpdateAndGetExtractedInternal(base::Bind(&RunMojoCallback, callback),
+                                base::FilePath(entry->content_path),
+                                GetConsumerCacheDirectory(cache_directory));
+}
+
+void URLResponseDiskCacheImpl::UpdateAndGetInternal(
     URLResponsePtr response,
     const FilePathPairCallback& callback) {
-  base::FilePath dir = GetDirName(base_directory_, response->url);
-
-  // Check if the response is cached and valid. If that's the case, returns the
-  // cached value.
-  CacheEntryPtr entry;
-  if (IsCacheEntryValid(dir, response.get(), &entry)) {
-    callback.Run(base::FilePath(entry->content_path),
-                 GetConsumerCacheDirectory(dir));
-    return;
-  }
-
-  // As the response was either not cached or the cached value is not valid, if
-  // the cache directory for the response exists, it needs to be cleaned.
-  if (base::PathExists(dir)) {
-    base::FilePath to_delete;
-    CHECK(CreateTemporaryDirInDir(base_directory_, "to_delete", &to_delete));
-    CHECK(Move(dir, to_delete));
-    task_runner_->PostTask(
-        FROM_HERE,
-        base::Bind(base::IgnoreResult(&base::DeleteFile), to_delete, true));
-  }
-
-  // If the response has not a valid body, and it is not possible to create
-  // either the cache directory or the consumer cache directory, returns an
-  // error.
-  if (!response->body.is_valid() ||
-      !base::CreateDirectoryAndGetError(dir, nullptr) ||
-      !base::CreateDirectoryAndGetError(GetConsumerCacheDirectory(dir),
-                                        nullptr)) {
-    callback.Run(base::FilePath(), base::FilePath());
-    return;
-  }
-
-  // Fill the entry values for the request.
-  base::FilePath entry_path = dir.Append(kEntryName);
-  base::FilePath content;
-  CHECK(CreateTemporaryFileInDir(dir, &content));
-  entry = CacheEntry::New();
-  entry->version = kCurrentVersion;
-  entry->url = response->url;
-  entry->content_path = content.value();
-  for (size_t i = 0u; i < response->headers.size(); ++i) {
-    auto cache_header = CacheHeaders::New();
-    cache_header->name = response->headers[i]->name;
-    cache_header->value = response->headers[i]->value;
-    entry->headers.push_back(cache_header.Pass());
-  }
-  // Asynchronously copy the response body to the cached file. The entry is send
-  // to the callback so that it is saved on disk only if the copy of the body
-  // succeded.
+  if (response->error ||
+      (response->status_code >= 400 && response->status_code < 600)) {
+    callback.Run(base::FilePath(), base::FilePath());
+    return;
+  }
+
+  std::string url = CanonicalizeURL(response->url);
+
+  // Check if the response is cached and valid. If that's the case, returns
+  // the cached value.
+  CacheEntryPtr entry = db_->GetNewest(request_origin_, url);
+  if (IsCacheEntryFresh(response, entry)) {
+    callback.Run(
+        base::FilePath(entry->content_path),
+        GetConsumerCacheDirectory(base::FilePath(entry->cache_directory)));
+    return;
+  }
+
+  if (!response->body.is_valid()) {
+    callback.Run(base::FilePath(), base::FilePath());
+    return;
+  }
+
+  std::string identifier = GetNewIdentifier();
+  // The content is copied to the staging directory so that files are not leaked
+  // if the shell terminates before an entry is saved to the database.
+  base::FilePath staging_content = GetStagingDirectory().Append(identifier);

[ppi, 2015/09/15 15:21:23]

s/staging_content/stage_at_path/ maybe?

[qsr, 2015/09/16 11:46:38]

Used staged_response_body_path

+
+  ScopedDataPipeConsumerHandle body = response->body.Pass();
+
+  // Asynchronously copy the response body to the staging directory. The
+  // callback will move it to the cache directory and save an entry in the
+  // database only if  the copy of the body succeded.

[ppi, 2015/09/15 15:21:23]

nit: remove one of the two spaces after if:)

[qsr, 2015/09/16 11:46:38]

Done.

   common::CopyToFile(
-      response->body.Pass(), content, task_runner_,
-      base::Bind(&RunCallbackWithSuccess, callback, content,
-                 GetConsumerCacheDirectory(dir), entry_path,
-                 base::Passed(entry.Pass()), base::Unretained(task_runner_)));
-}
-
-void URLResponseDiskCacheImpl::GetExtractedContentInternal(
+      body.Pass(), staging_content, task_runner_.get(),
+      base::Bind(&RunCallbackWithSuccess, callback, identifier, request_origin_,
+                 url, base::Passed(response.Pass()), db_, task_runner_));
+}
+
+void URLResponseDiskCacheImpl::UpdateAndGetExtractedInternal(
     const FilePathPairCallback& callback,
-    const base::FilePath& base_dir,
-    const base::FilePath& extracted_dir,
-    const base::FilePath& content,
-    const base::FilePath& cache_dir) {
-  TRACE_EVENT1("url_response_disk_cache", "GetExtractedContentInternal",
-               "extracted_dir", extracted_dir.value());
+    const base::FilePath& content_path,
+    const base::FilePath& consumer_cache_directory) {
+  TRACE_EVENT1("url_response_disk_cache", "UpdateAndGetExtractedInternal",
+               "content_path", content_path.value());
   // If it is not possible to get the cached file, returns an error.
-  if (content.empty()) {
-    callback.Run(base::FilePath(), base::FilePath());
-    return;
+  if (content_path.empty()) {
+    callback.Run(base::FilePath(), base::FilePath());
+    return;
+  }
+
+  base::FilePath cache_directory = consumer_cache_directory.DirName();

[ppi, 2015/09/15 15:21:23]

I'd also call this entry_directory here.

[qsr, 2015/09/16 11:46:38]

Done.

+  base::FilePath extracted_directory = GetExtractedDirectory(cache_directory);

[ppi, 2015/09/15 15:21:23]

I'd s/extracted/extraction/ here and below.

[qsr, 2015/09/16 11:46:38]

Done.

+  base::FilePath extracted_sentinel = GetExtractedSentinel(cache_directory);
+
+  if (PathExists(extracted_sentinel)) {
+    callback.Run(extracted_directory, consumer_cache_directory);
+    return;
+  }
+
+  if (PathExists(extracted_directory)) {
+    if (!base::DeleteFile(extracted_directory, true)) {
+      callback.Run(base::FilePath(), base::FilePath());
+      return;
+    }
   }
 
   // Unzip the content to the extracted directory. In case of any error, returns
   // an error.
   zip::ZipReader reader;
-  if (!reader.Open(content)) {
+  if (!reader.Open(content_path)) {
     callback.Run(base::FilePath(), base::FilePath());
     return;
   }
   while (reader.HasMore()) {
     bool success = reader.OpenCurrentEntryInZip();
-    success = success && reader.ExtractCurrentEntryIntoDirectory(extracted_dir);
+    success =
+        success && reader.ExtractCurrentEntryIntoDirectory(extracted_directory);
     success = success && reader.AdvanceToNextEntry();
     if (!success) {
       callback.Run(base::FilePath(), base::FilePath());
       return;
     }
   }
   // We can ignore write error, as it will just force to clear the cache on the
   // next request.
-  WriteFile(GetExtractedSentinel(base_dir), nullptr, 0);
-  callback.Run(extracted_dir, cache_dir);
+  WriteFile(GetExtractedSentinel(cache_directory), nullptr, 0);
+  callback.Run(extracted_directory, consumer_cache_directory);
 }
 
 }  // namespace mojo