Migrates legacy packaged app data when it's upgraded to a platform app

content/browser/indexed_db/indexed_db_context_impl.cc

Index: content/browser/indexed_db/indexed_db_context_impl.cc
diff --git a/content/browser/indexed_db/indexed_db_context_impl.cc b/content/browser/indexed_db/indexed_db_context_impl.cc
index c7e13263677bc3aa093aa0d7ff52426d861b3c78..e3f650e67b969c2ef4340f3f00cd4ecbc7674638 100644
--- a/content/browser/indexed_db/indexed_db_context_impl.cc
+++ b/content/browser/indexed_db/indexed_db_context_impl.cc
@@ -322,6 +322,29 @@ void IndexedDBContextImpl::DeleteForOrigin(const GURL& origin_url) {
   }
 }
 
+void IndexedDBContextImpl::CopyOriginData(const GURL& origin_url,
+                                          IndexedDBContext* dest_context) {
+  DCHECK(TaskRunner()->RunsTasksOnCurrentThread());
+  ForceClose(origin_url, FORCE_CLOSE_COPY_ORIGIN);
+  if (data_path_.empty() || !IsInOriginSet(origin_url))
+    return;
+
+  base::FilePath idb_directory =
+      GetFilePathForTesting(origin_url.possibly_invalid_spec());

[jsbell, 2014/12/12 17:46:09]

The "ForTesting" methods should only be called from unit tests.

This should instead call GetStoragePaths() and iterate over the vector of
returned paths, copying each so that both the .leveldb and .blob directories are
copied.

+  base::FilePath dest_directory =

[jsbell, 2014/12/12 17:46:09]

This is basically just computing <context>/IndexedDB/<origin>.indexeddb.leveldb
then throwing away everything but <context>/IndexedDB - I think it would be
cleaner to expose that as a method on the context. Then this whole thing
basically becomes:

base::FilePath dest_dir = dest_context->GetContextPath(); // name?
if (!base::PathExists(dest_dir))
  base::CreateDirectory(dest_dir);
for (const auto& path : GetFilePaths())
  base::CopyDirectory(path, dest_dir);

... although you may want to do an early loop over the paths and exit early if
one exists, which is what this CL's line 337 does (although it doesn't return a
failure code?)

[ryanackley, 2014/12/12 19:03:04]

Should I expose the methods I need on IndexedDBContext or just do a static cast
to IndexedDBContextImpl on dest_context? If I'm using IndexedDBContextImpl,
there is already a data_path() method that will give me the <context>/IndexedDB
path. 

With regard to the failure condition, I was thinking of just changing it to a
DCHECK instead of a silent return. For my purposes, there isn't much I can do
with an error. Most of the time extension upgrades happen automatically in the
background. If the migration were to fail, I couldn't easily bubble this up to
the user to action it. i.e. "To continue please delete your
c:\users\bob\AppData\Local\Chrome\Storage\ext\sasdjkasdhkjhasdjkd\IndexedDB
directory. Oh and btw this folder may be hidden depending on your shell
settings."  It's more along the lines that if this fails, something is broken
with the state of our data model which would signify a bug or extraordinary
condition. 

+      dest_context->GetFilePathForTesting(origin_url.possibly_invalid_spec());
+
+  if (base::PathExists(dest_directory)) {
+    return;
+  }
+
+  if (!base::PathExists(dest_directory.DirName())) {
+    base::CreateDirectory(dest_directory.DirName());
+  }
+
+  base::CopyDirectory(idb_directory, dest_directory.DirName(), true);
+}
+
 void IndexedDBContextImpl::ForceClose(const GURL origin_url,
                                       ForceCloseReason reason) {
   DCHECK(TaskRunner()->RunsTasksOnCurrentThread());