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

chrome/browser/extensions/extension_service.cc

Index: chrome/browser/extensions/extension_service.cc
diff --git a/chrome/browser/extensions/extension_service.cc b/chrome/browser/extensions/extension_service.cc
index 7636dfe7101bd3f8bead57445336aa37760f9fe9..6b415753171cceeb5979911fb64521cf544a216c 100644
--- a/chrome/browser/extensions/extension_service.cc
+++ b/chrome/browser/extensions/extension_service.cc
@@ -9,6 +9,7 @@
 #include <set>
 
 #include "base/command_line.h"
+#include "base/files/file_util.h"
 #include "base/metrics/histogram.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/stringprintf.h"
@@ -56,6 +57,7 @@
 #include "components/crx_file/id_util.h"
 #include "components/startup_metric_utils/startup_metric_utils.h"
 #include "content/public/browser/devtools_agent_host.h"
+#include "content/public/browser/indexed_db_context.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/storage_partition.h"
@@ -81,6 +83,9 @@
 #include "extensions/common/one_shot_event.h"
 #include "extensions/common/permissions/permission_message_provider.h"
 #include "extensions/common/permissions/permissions_data.h"
+#include "storage/browser/fileapi/sandbox_file_system_backend_delegate.h"
+#include "storage/common/database/database_identifier.h"
+#include "storage/common/fileapi/file_system_types.h"
 
 #if defined(OS_CHROMEOS)
 #include "chrome/browser/chromeos/extensions/install_limiter.h"
@@ -115,6 +120,82 @@ namespace {
 // Wait this many seconds after an extensions becomes idle before updating it.
 const int kUpdateIdleDelay = 5;
 
+void CopyFileSystem(
+    storage::SandboxFileSystemBackendDelegate* old_sandbox_delegate,
+    storage::SandboxFileSystemBackendDelegate* sandbox_delegate,
+    const GURL& origin,
+    storage::FileSystemType type){
+
+  base::FilePath old_base_path =
+      old_sandbox_delegate->GetBaseDirectoryForOriginAndType(
+          origin,
+          type,
+          false);
+
+  base::FilePath base_path =
+      sandbox_delegate->GetBaseDirectoryForOriginAndType(
+          origin,
+          type,
+          true);
+
+  base::CopyDirectory(old_base_path, base_path.DirName(), true);
+}
+
+void MigrateLegacyPartition(
+    content::StoragePartition* old_partition,
+    content::StoragePartition* current_partition,
+    const Extension* extension) {
+  GURL extension_url = Extension::GetBaseURLFromExtensionId(extension->id());

[benwells, 2014/11/03 00:08:41]

Could you add a check here that this is happening on the correct thread?

[ryanackley, 2014/11/04 22:51:54]

Acknowledged.

+
+  content::IndexedDBContext* indexed_db_context =
+      current_partition->GetIndexedDBContext();
+  content::IndexedDBContext* old_indexed_db_context =
+      old_partition->GetIndexedDBContext();
+
+  base::FilePath old_db_path =
+      old_indexed_db_context->GetFilePath(extension_url);
+  base::FilePath current_db_path =
+      indexed_db_context->GetFilePath(extension_url);
+
+  if (base::PathExists(old_db_path) && !base::PathExists(current_db_path)){
+    base::CopyDirectory(old_db_path, current_db_path.DirName(), true);
+  }
+
+  storage::FileSystemContext* old_fs_context =
+      old_partition->GetFileSystemContext();
+  storage::FileSystemContext* fs_context =
+      current_partition->GetFileSystemContext();
+
+  storage::SandboxFileSystemBackendDelegate* old_sandbox_delegate =
+      old_fs_context->sandbox_delegate();
+  storage::SandboxFileSystemBackendDelegate* sandbox_delegate =
+      fs_context->sandbox_delegate();
+
+  scoped_ptr<storage::SandboxFileSystemBackendDelegate::OriginEnumerator>
+      enumerator(old_sandbox_delegate->CreateOriginEnumerator());
+
+  GURL origin;
+  while (extension_url != (origin = enumerator->Next()) && !origin.is_empty()){

[benwells, 2014/11/03 00:08:41]

Nit: you need a space before the curly (here and elsewhere). But you also don't
need these curies at all.

Nit: having an assignment in a conditional is generally yuck. I'd redo this to
avoid that.

[ryanackley, 2014/11/04 22:51:53]

Acknowledged.

+    continue;
+  }
+  if (!origin.is_empty()){
+    if (enumerator->HasFileSystemType(storage::kFileSystemTypeTemporary)){
+      CopyFileSystem(
+          old_sandbox_delegate,
+          sandbox_delegate,
+          extension_url,
+          storage::kFileSystemTypeTemporary);
+    }
+    if (enumerator->HasFileSystemType(storage::kFileSystemTypePersistent)){
+      CopyFileSystem(
+          old_sandbox_delegate,
+          sandbox_delegate,
+          extension_url,
+          storage::kFileSystemTypePersistent);
+    }
+  }
+}
+
 }  // namespace
 
 // ExtensionService.
@@ -1341,6 +1422,7 @@ void ExtensionService::AddExtension(const Extension* extension) {
     int version_compare_result =
         extension->version()->CompareTo(*(old->version()));
     is_extension_upgrade = version_compare_result > 0;
+
     // Other than for unpacked extensions, CrxInstaller should have guaranteed
     // that we aren't downgrading.
     if (!Manifest::IsUnpackedLocation(extension->location()))
@@ -1414,10 +1496,12 @@ void ExtensionService::AddExtension(const Extension* extension) {
     }
 
     registry_->AddEnabled(extension);
+
     if (extension_sync_service_)
       extension_sync_service_->SyncExtensionChangeIfNeeded(*extension);
     NotifyExtensionLoaded(extension);
   }
+
   system_->runtime_data()->SetBeingUpgraded(extension, false);
 }
 
@@ -1712,6 +1796,48 @@ void ExtensionService::OnExtensionManagementSettingsChanged() {
   CheckManagementPolicy();
 }
 
+bool ExtensionService::NeedsMigration(const Extension* extension){

[benwells, 2014/11/03 00:08:41]

Could all the new logic you're adding go in a new file / class - Extension? It
would be on ExtensionService as a field. This file is kind of big and ugly and
this is adding more dependencies out onto other bits of code.

[benwells, 2014/11/03 00:08:42]

Can you add a check here that this is happening on the UI thread?

[ryanackley, 2014/11/04 22:51:54]

Acknowledged.

[ryanackley, 2014/11/04 22:51:54]

Acknowledged.

+  const Extension* old = GetInstalledExtension(extension->id());
+  return old && old->is_legacy_packaged_app() && extension->is_platform_app();
+}
+
+void ExtensionService::MigrateThenFinishInstallation(
+      const Extension* extension,
+      bool was_ephemeral){
+
+  const Extension* old = GetInstalledExtension(extension->id());
+
+  // juggle the enabled status of the new and old extensions so we can get
+  // the old and new storage partitions
+  content::StoragePartition* old_partition =
+      BrowserContext::GetStoragePartitionForSite(
+          profile_,
+          Extension::GetBaseURLFromExtensionId(extension->id()));
+  bool oldWasDisabled = registry_->AddEnabled(extension);
+  content::StoragePartition* new_partition =
+      BrowserContext::GetStoragePartitionForSite(
+          profile_,
+          Extension::GetBaseURLFromExtensionId(extension->id()));
+
+  if (!oldWasDisabled){
+    registry_->AddEnabled(old);
+  }else{
+    registry_->RemoveEnabled(extension->id());
+  }
+
+  BrowserThread::PostTaskAndReply(BrowserThread::FILE, FROM_HERE,

[benwells, 2014/11/03 00:08:41]

Could you use the blocking pool instead? See
https://code.google.com/p/chromium/codesearch#chromium/src/content/public/bro...
and callers for how to use it.

The blocking pool is a replacement of the FILE thread.

[ryanackley, 2014/11/04 22:51:54]

Acknowledged.

+      base::Bind(
+          &MigrateLegacyPartition,
+          old_partition,
+          new_partition,
+          make_scoped_refptr(extension)),
+      base::Bind(
+          &ExtensionService::FinishInstallation,
+          AsWeakPtr(),
+          make_scoped_refptr(extension),
+          was_ephemeral));
+}
+
 void ExtensionService::AddNewOrUpdatedExtension(
     const Extension* extension,
     Extension::State initial_state,
@@ -1725,7 +1851,13 @@ void ExtensionService::AddNewOrUpdatedExtension(
   delayed_installs_.Remove(extension->id());
   if (InstallVerifier::NeedsVerification(*extension))
     system_->install_verifier()->VerifyExtension(extension->id());
-  FinishInstallation(extension, was_ephemeral);
+
+  if (NeedsMigration(extension)){

[benwells, 2014/11/03 00:08:42]

The patter chrome tends to use here is more like:

  if (blah) {
    blab();
    return;
  }

  otherblab();
}

[ryanackley, 2014/11/04 22:51:54]

Acknowledged.

+    MigrateThenFinishInstallation(extension, was_ephemeral);
+  }else{
+    FinishInstallation(extension, was_ephemeral);
+  }
+
 }
 
 void ExtensionService::MaybeFinishDelayedInstallation(