Install platform apps into a separate data directory

chrome/browser/extensions/crx_installer.cc

Index: chrome/browser/extensions/crx_installer.cc
diff --git a/chrome/browser/extensions/crx_installer.cc b/chrome/browser/extensions/crx_installer.cc
index 41b97602f92bb4dbb76244c9b3d403e3f3cda27a..7b3f1159e21d36ac573887222921ceb68beff0ea 100644
--- a/chrome/browser/extensions/crx_installer.cc
+++ b/chrome/browser/extensions/crx_installer.cc
@@ -27,8 +27,10 @@
 #include "chrome/browser/extensions/extension_error_reporter.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/permissions_updater.h"
+#include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/shell_integration.h"
 #include "chrome/browser/web_applications/web_app.h"
+#include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/extensions/extension_constants.h"
@@ -138,6 +140,7 @@ CrxInstaller::CrxInstaller(base::WeakPtr<ExtensionService> frontend_weak,
       create_app_shortcut_(false),
       frontend_weak_(frontend_weak),
       profile_(frontend_weak->profile()),
+      profile_path_(profile_->GetPath()),

[benwells, 2012/02/13 02:55:44]

Do you need this new field? Can you just call profile_->GetPath() in the one
place it is used?

       client_(client),
       apps_require_extension_mime_type_(false),
       allow_silent_install_(false),
@@ -488,11 +491,15 @@ void CrxInstaller::CompleteInstall() {
     "Extensions.CrxInstallDirPathLength",
         install_directory_.value().length(), 0, 500, 100);
 
+  bool separate_data_dir_required =

[jeremy, 2012/02/12 09:40:28]

Could you add a comment explaining when this case kicks in.  It should be clear
to someone reading this function, under what conditions the flag will be true.

+      extension_->RequiresSeparateUserDataDirectory();
   FilePath version_dir = extension_file_util::InstallExtension(
       unpacked_extension_root_,
       extension_->id(),
       extension_->VersionString(),
-      install_directory_);
+      install_directory_,
+      separate_data_dir_required ? extension_file_util::DELETE_SOURCE_NO :
+                                   extension_file_util::DELETE_SOURCE_YES);
   if (version_dir.empty()) {
     ReportFailureFromFileThread(
         l10n_util::GetStringUTF16(
@@ -500,6 +507,22 @@ void CrxInstaller::CompleteInstall() {
     return;
   }
 
+  // If the platform app requires its own data directory then we'll need to
+  // copy the extension into that data directory too.
+  if (separate_data_dir_required) {

[benwells, 2012/02/13 02:55:44]

What about unpacked extensions? Can this move into
extension_file_util::InstallExtension?

+    FilePath data_dir =
+        web_app::GetWebAppDataDirectory(profile_path_, *extension_);
+    FilePath profile_path = data_dir.AppendASCII(chrome::kInitialProfile);
+    FilePath extension_path =
+        profile_path.AppendASCII(ExtensionService::kInstallDirectoryName);
+    extension_file_util::InstallExtension(

[jeremy, 2012/02/12 09:40:28]

What if this call fails?

+        unpacked_extension_root_,
+        extension_->id(),
+        extension_->VersionString(),
+        extension_path,
+        extension_file_util::DELETE_SOURCE_YES);
+  }
+
   // This is lame, but we must reload the extension because absolute paths
   // inside the content scripts are established inside InitFromValue() and we
   // just moved the extension.