Use string16 for cache_name in the back end of CacheStorage.

content/browser/cache_storage/cache_storage.cc

Index: content/browser/cache_storage/cache_storage.cc
diff --git a/content/browser/cache_storage/cache_storage.cc b/content/browser/cache_storage/cache_storage.cc
index 1a1f0517e48815acf9ecc5b003a8264e5661f42d..66261bcd4c86a711b6c2bd7438167f648b2d6f56 100644
--- a/content/browser/cache_storage/cache_storage.cc
+++ b/content/browser/cache_storage/cache_storage.cc
@@ -20,8 +20,10 @@
 #include "base/sha1.h"
 #include "base/single_thread_task_runner.h"
 #include "base/stl_util.h"
+#include "base/strings/string16.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
+#include "base/strings/utf_string_conversions.h"
 #include "base/thread_task_runner_handle.h"
 #include "content/browser/cache_storage/cache_storage.pb.h"
 #include "content/browser/cache_storage/cache_storage_cache.h"
@@ -70,7 +72,7 @@ class CacheStorage::CacheLoader {
   typedef base::Callback<void(const scoped_refptr<CacheStorageCache>&)>
       CacheCallback;
   typedef base::Callback<void(bool)> BoolCallback;
-  typedef base::Callback<void(scoped_ptr<std::vector<std::string>>)>
+  typedef base::Callback<void(scoped_ptr<std::vector<base::string16>>)>
       StringVectorCallback;
 
   CacheLoader(
@@ -92,15 +94,15 @@ class CacheStorage::CacheLoader {
   // Creates a CacheStorageCache with the given name. It does not attempt to
   // load the backend, that happens lazily when the cache is used.
   virtual scoped_refptr<CacheStorageCache> CreateCache(
-      const std::string& cache_name) = 0;
+      const base::string16& cache_name) = 0;
 
   // Deletes any pre-existing cache of the same name and then loads it.
-  virtual void PrepareNewCacheDestination(const std::string& cache_name,
+  virtual void PrepareNewCacheDestination(const base::string16& cache_name,
                                           const CacheCallback& callback) = 0;
 
   // After the backend has been deleted, do any extra house keeping such as
   // removing the cache's directory.
-  virtual void CleanUpDeletedCache(const std::string& key,
+  virtual void CleanUpDeletedCache(const base::string16& cache_name,
                                    const BoolCallback& callback) = 0;
 
   // Writes the cache names (and sizes) to disk if applicable.
@@ -108,7 +110,7 @@ class CacheStorage::CacheLoader {
                           const BoolCallback& callback) = 0;
 
   // Loads the cache names from disk if applicable.
-  virtual void LoadIndex(scoped_ptr<std::vector<std::string>> cache_names,
+  virtual void LoadIndex(scoped_ptr<std::vector<base::string16>> cache_names,
                          const StringVectorCallback& callback) = 0;
 
  protected:
@@ -138,19 +140,19 @@ class CacheStorage::MemoryLoader : public CacheStorage::CacheLoader {
                     origin) {}
 
   scoped_refptr<CacheStorageCache> CreateCache(
-      const std::string& cache_name) override {
+      const base::string16& cache_name) override {
     return CacheStorageCache::CreateMemoryCache(
         origin_, request_context_getter_, quota_manager_proxy_, blob_context_);
   }
 
-  void PrepareNewCacheDestination(const std::string& cache_name,
+  void PrepareNewCacheDestination(const base::string16& cache_name,
                                   const CacheCallback& callback) override {
     scoped_refptr<CacheStorageCache> cache = CreateCache(cache_name);
     cache_refs_.insert(std::make_pair(cache_name, cache));
     callback.Run(cache);
   }
 
-  void CleanUpDeletedCache(const std::string& cache_name,
+  void CleanUpDeletedCache(const base::string16& cache_name,
                            const BoolCallback& callback) override {
     CacheRefMap::iterator it = cache_refs_.find(cache_name);
     DCHECK(it != cache_refs_.end());
@@ -163,13 +165,14 @@ class CacheStorage::MemoryLoader : public CacheStorage::CacheLoader {
     callback.Run(false);
   }
 
-  void LoadIndex(scoped_ptr<std::vector<std::string>> cache_names,
+  void LoadIndex(scoped_ptr<std::vector<base::string16>> cache_names,
                  const StringVectorCallback& callback) override {
     callback.Run(std::move(cache_names));
   }
 
  private:
-  typedef std::map<std::string, scoped_refptr<CacheStorageCache>> CacheRefMap;
+  typedef std::map<base::string16, scoped_refptr<CacheStorageCache>>
+      CacheRefMap;
   ~MemoryLoader() override {}
 
   // Keep a reference to each cache to ensure that it's not freed before the
@@ -196,7 +199,7 @@ class CacheStorage::SimpleCacheLoader : public CacheStorage::CacheLoader {
         weak_ptr_factory_(this) {}
 
   scoped_refptr<CacheStorageCache> CreateCache(
-      const std::string& cache_name) override {
+      const base::string16& cache_name) override {
     DCHECK_CURRENTLY_ON(BrowserThread::IO);
     DCHECK(ContainsKey(cache_name_to_cache_dir_, cache_name));
 
@@ -207,7 +210,7 @@ class CacheStorage::SimpleCacheLoader : public CacheStorage::CacheLoader {
         blob_context_);
   }
 
-  void PrepareNewCacheDestination(const std::string& cache_name,
+  void PrepareNewCacheDestination(const base::string16& cache_name,
                                   const CacheCallback& callback) override {
     DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
@@ -232,7 +235,7 @@ class CacheStorage::SimpleCacheLoader : public CacheStorage::CacheLoader {
     return base::CreateDirectory(cache_path) ? cache_dir : "";
   }
 
-  void PrepareNewCacheCreateCache(const std::string& cache_name,
+  void PrepareNewCacheCreateCache(const base::string16& cache_name,
                                   const CacheCallback& callback,
                                   const std::string& cache_dir) {
     if (cache_dir.empty()) {
@@ -244,7 +247,7 @@ class CacheStorage::SimpleCacheLoader : public CacheStorage::CacheLoader {
     callback.Run(CreateCache(cache_name));
   }
 
-  void CleanUpDeletedCache(const std::string& cache_name,
+  void CleanUpDeletedCache(const base::string16& cache_name,
                            const BoolCallback& callback) override {
     DCHECK_CURRENTLY_ON(BrowserThread::IO);
     DCHECK(ContainsKey(cache_name_to_cache_dir_, cache_name));
@@ -281,8 +284,14 @@ class CacheStorage::SimpleCacheLoader : public CacheStorage::CacheLoader {
       DCHECK(ContainsKey(cache_name_to_cache_dir_, cache_names[i]));
 
       CacheStorageIndex::Cache* index_cache = index.add_cache();
-      index_cache->set_name(cache_names[i]);
+      // We use |name_utf16| to store the cache name. But we set |name| here for
+      // forward compatibility.
+      // TODO(horo): Depricate |name| when M51 become stable.

[nhiroki, 2016/03/02 05:43:13]

s/become/becomes/

[horo, 2016/03/02 08:35:27]

Done.

+      index_cache->set_name(base::UTF16ToUTF8(cache_names[i]));
       index_cache->set_cache_dir(cache_name_to_cache_dir_[cache_names[i]]);
+      index_cache->set_name_utf16(
+          cache_names[i].data(),
+          cache_names[i].length() * sizeof(base::char16));
     }
 
     std::string serialized;
@@ -316,7 +325,7 @@ class CacheStorage::SimpleCacheLoader : public CacheStorage::CacheLoader {
     original_task_runner->PostTask(FROM_HERE, base::Bind(callback, rv));
   }
 
-  void LoadIndex(scoped_ptr<std::vector<std::string>> names,
+  void LoadIndex(scoped_ptr<std::vector<base::string16>> names,
                  const StringVectorCallback& callback) override {
     DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
@@ -334,7 +343,7 @@ class CacheStorage::SimpleCacheLoader : public CacheStorage::CacheLoader {
                    callback));
   }
 
-  void LoadIndexDidReadFile(scoped_ptr<std::vector<std::string>> names,
+  void LoadIndexDidReadFile(scoped_ptr<std::vector<base::string16>> names,
                             const StringVectorCallback& callback,
                             const std::string& serialized) {
     DCHECK_CURRENTLY_ON(BrowserThread::IO);
@@ -346,8 +355,14 @@ class CacheStorage::SimpleCacheLoader : public CacheStorage::CacheLoader {
       for (int i = 0, max = index.cache_size(); i < max; ++i) {
         const CacheStorageIndex::Cache& cache = index.cache(i);
         DCHECK(cache.has_cache_dir());
-        names->push_back(cache.name());
-        cache_name_to_cache_dir_[cache.name()] = cache.cache_dir();
+        base::string16 cache_name(base::UTF8ToUTF16(cache.name()));
+        if (cache.has_name_utf16()) {

[nhiroki, 2016/03/02 05:43:13]

Should we have a UMA to track the number of attempts to update the protobuf
schema so that we can know when to drop these conversion code?

[horo, 2016/03/02 08:35:27]

We can't say when users return back to the sites which created the cache storage
in old Chrome.
So it is not easy to drop these conversion code.

Of course we can add codes which migrate the all cache storage indexes for all
origins.
But I prefer to keep this simple conversion code rather than to add complex
migration codes.

[nhiroki, 2016/03/02 09:22:58]

Hmm... I'm a bit confused. According to the TODO comment at line 289, you will
actually remove/deprecate the name field (and the conversion code), right? How
do you decide the timing to deprecate it w/o such the UMA and/or migration code?
If we don't have either the UMA or the migration code, I think we couldn't
estimate the number of users who may lose access to their contents...

[horo, 2016/03/04 03:49:13]

Line 290 is writing the name for FORWARD compatibility so that Chrome 50 and
older (or 51 if this patch will be reverted unfortunately) can read the
CacheStorage index which was written by Chrome 51 (with this patch).
We can stop writing the name when M51 becomes stable.

Line 358 is reading the name for BACKWARD compatibility so that Chrome with this
patch will be able to read the old CacheStorage index which was written by
Chrome 50 and older.
I think we should keep this line.

+          cache_name = base::string16(
+              reinterpret_cast<const base::char16*>(cache.name_utf16().data()),
+              cache.name_utf16().size() / sizeof(base::char16));

[nhiroki, 2016/03/02 05:43:13]

We may need to store the index if there is a cache that doesn't have a valid
|name_utf16| field because WriteIndex() would be called only when a Cache is
newly created or deleted, that is, the index is never updated until those
operations are issued.

+        }
+        names->push_back(cache_name);
+        cache_name_to_cache_dir_[cache_name] = cache.cache_dir();
         cache_dirs->insert(cache.cache_dir());
       }
     }
@@ -442,7 +457,7 @@ class CacheStorage::SimpleCacheLoader : public CacheStorage::CacheLoader {
   }
 
   const base::FilePath origin_path_;
-  std::map<std::string, std::string> cache_name_to_cache_dir_;
+  std::map<base::string16, std::string> cache_name_to_cache_dir_;
 
   base::WeakPtrFactory<SimpleCacheLoader> weak_ptr_factory_;
 };
@@ -477,7 +492,7 @@ CacheStorage::CacheStorage(
 CacheStorage::~CacheStorage() {
 }
 
-void CacheStorage::OpenCache(const std::string& cache_name,
+void CacheStorage::OpenCache(const base::string16& cache_name,
                              const CacheAndErrorCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
@@ -492,7 +507,7 @@ void CacheStorage::OpenCache(const std::string& cache_name,
                                            cache_name, pending_callback));
 }
 
-void CacheStorage::HasCache(const std::string& cache_name,
+void CacheStorage::HasCache(const base::string16& cache_name,
                             const BoolAndErrorCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
@@ -507,7 +522,7 @@ void CacheStorage::HasCache(const std::string& cache_name,
                                            cache_name, pending_callback));
 }
 
-void CacheStorage::DeleteCache(const std::string& cache_name,
+void CacheStorage::DeleteCache(const base::string16& cache_name,
                                const BoolAndErrorCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
@@ -537,7 +552,7 @@ void CacheStorage::EnumerateCaches(const StringsAndErrorCallback& callback) {
 }
 
 void CacheStorage::MatchCache(
-    const std::string& cache_name,
+    const base::string16& cache_name,
     scoped_ptr<ServiceWorkerFetchRequest> request,
     const CacheStorageCache::ResponseCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
@@ -629,8 +644,8 @@ void CacheStorage::LazyInitImpl() {
   // 3. Once each load is complete, update the map variables.
   // 4. Call the list of waiting callbacks.
 
-  scoped_ptr<std::vector<std::string>> indexed_cache_names(
-      new std::vector<std::string>());
+  scoped_ptr<std::vector<base::string16>> indexed_cache_names(
+      new std::vector<base::string16>());
 
   cache_loader_->LoadIndex(std::move(indexed_cache_names),
                            base::Bind(&CacheStorage::LazyInitDidLoadIndex,
@@ -638,7 +653,7 @@ void CacheStorage::LazyInitImpl() {
 }
 
 void CacheStorage::LazyInitDidLoadIndex(
-    scoped_ptr<std::vector<std::string>> indexed_cache_names) {
+    scoped_ptr<std::vector<base::string16>> indexed_cache_names) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   for (size_t i = 0u, max = indexed_cache_names->size(); i < max; ++i) {
@@ -653,7 +668,7 @@ void CacheStorage::LazyInitDidLoadIndex(
   scheduler_->CompleteOperationAndRunNext();
 }
 
-void CacheStorage::OpenCacheImpl(const std::string& cache_name,
+void CacheStorage::OpenCacheImpl(const base::string16& cache_name,
                                  const CacheAndErrorCallback& callback) {
   scoped_refptr<CacheStorageCache> cache = GetLoadedCache(cache_name);
   if (cache.get()) {
@@ -667,7 +682,7 @@ void CacheStorage::OpenCacheImpl(const std::string& cache_name,
 }
 
 void CacheStorage::CreateCacheDidCreateCache(
-    const std::string& cache_name,
+    const base::string16& cache_name,
     const CacheAndErrorCallback& callback,
     const scoped_refptr<CacheStorageCache>& cache) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
@@ -704,13 +719,13 @@ void CacheStorage::CreateCacheDidWriteIndex(
   callback.Run(cache, CACHE_STORAGE_OK);
 }
 
-void CacheStorage::HasCacheImpl(const std::string& cache_name,
+void CacheStorage::HasCacheImpl(const base::string16& cache_name,
                                 const BoolAndErrorCallback& callback) {
   bool has_cache = ContainsKey(cache_map_, cache_name);
   callback.Run(has_cache, CACHE_STORAGE_OK);
 }
 
-void CacheStorage::DeleteCacheImpl(const std::string& cache_name,
+void CacheStorage::DeleteCacheImpl(const base::string16& cache_name,
                                    const BoolAndErrorCallback& callback) {
   scoped_refptr<CacheStorageCache> cache = GetLoadedCache(cache_name);
   if (!cache.get()) {
@@ -734,7 +749,7 @@ void CacheStorage::DeleteCacheImpl(const std::string& cache_name,
 }
 
 void CacheStorage::DeleteCacheDidClose(
-    const std::string& cache_name,
+    const base::string16& cache_name,
     const BoolAndErrorCallback& callback,
     const StringVector& ordered_cache_names,
     const scoped_refptr<CacheStorageCache>& cache,
@@ -746,7 +761,7 @@ void CacheStorage::DeleteCacheDidClose(
 }
 
 void CacheStorage::DeleteCacheDidWriteIndex(
-    const std::string& cache_name,
+    const base::string16& cache_name,
     const BoolAndErrorCallback& callback,
     int cache_size,
     bool success) {
@@ -774,7 +789,7 @@ void CacheStorage::EnumerateCachesImpl(
 }
 
 void CacheStorage::MatchCacheImpl(
-    const std::string& cache_name,
+    const base::string16& cache_name,
     scoped_ptr<ServiceWorkerFetchRequest> request,
     const CacheStorageCache::ResponseCallback& callback) {
   scoped_refptr<CacheStorageCache> cache = GetLoadedCache(cache_name);
@@ -815,7 +830,7 @@ void CacheStorage::MatchAllCachesImpl(
                                       weak_factory_.GetWeakPtr(),
                                       base::Passed(std::move(callback_copy))));
 
-  for (const std::string& cache_name : ordered_cache_names_) {
+  for (const base::string16& cache_name : ordered_cache_names_) {
     scoped_refptr<CacheStorageCache> cache = GetLoadedCache(cache_name);
     DCHECK(cache.get());
 
@@ -853,7 +868,7 @@ void CacheStorage::MatchAllCachesDidMatchAll(
 }
 
 scoped_refptr<CacheStorageCache> CacheStorage::GetLoadedCache(
-    const std::string& cache_name) {
+    const base::string16& cache_name) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DCHECK(initialized_);
 
@@ -914,7 +929,7 @@ void CacheStorage::GetSizeThenCloseAllCachesImpl(const SizeCallback& callback) {
       base::Bind(&SizeRetrievedFromAllCaches,
                  base::Passed(std::move(accumulator)), callback));
 
-  for (const std::string& cache_name : ordered_cache_names_) {
+  for (const base::string16& cache_name : ordered_cache_names_) {
     scoped_refptr<CacheStorageCache> cache = GetLoadedCache(cache_name);
     cache->GetSizeThenClose(base::Bind(&SizeRetrievedFromCache, cache,
                                        barrier_closure, accumulator_ptr));
@@ -933,7 +948,7 @@ void CacheStorage::SizeImpl(const SizeCallback& callback) {
       base::Bind(&SizeRetrievedFromAllCaches,
                  base::Passed(std::move(accumulator)), callback));
 
-  for (const std::string& cache_name : ordered_cache_names_) {
+  for (const base::string16& cache_name : ordered_cache_names_) {
     scoped_refptr<CacheStorageCache> cache = GetLoadedCache(cache_name);
     cache->Size(base::Bind(&SizeRetrievedFromCache, cache, barrier_closure,
                            accumulator_ptr));