Install platform apps into a separate data directory

chrome/browser/extensions/extension_service.cc

 // Copyright (c) 2012 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 "chrome/browser/extensions/extension_service.h"
 
 #include <algorithm>
 #include <set>
 
 #include "base/basictypes.h"
@@ skipping 27 matching lines @@
 #include "chrome/browser/extensions/crx_installer.h"
 #include "chrome/browser/extensions/default_apps_trial.h"
 #include "chrome/browser/extensions/extension_browser_event_router.h"
 #include "chrome/browser/extensions/extension_cookies_api.h"
 #include "chrome/browser/extensions/extension_data_deleter.h"
 #include "chrome/browser/extensions/extension_error_reporter.h"
 #include "chrome/browser/extensions/extension_global_error.h"
 #include "chrome/browser/extensions/extension_host.h"
 #include "chrome/browser/extensions/extension_input_ime_api.h"
 #include "chrome/browser/extensions/extension_management_api.h"
+#include "chrome/browser/extensions/extension_pref_value_map.h"
 #include "chrome/browser/extensions/extension_preference_api.h"
 #include "chrome/browser/extensions/extension_process_manager.h"
 #include "chrome/browser/extensions/extension_processes_api.h"
 #include "chrome/browser/extensions/extension_sorting.h"
 #include "chrome/browser/extensions/extension_special_storage_policy.h"
 #include "chrome/browser/extensions/extension_sync_data.h"
 #include "chrome/browser/extensions/extension_updater.h"
 #include "chrome/browser/extensions/extension_web_ui.h"
 #include "chrome/browser/extensions/extension_webnavigation_api.h"
 #include "chrome/browser/extensions/external_extension_provider_impl.h"
@@ skipping 13 matching lines @@
 #include "chrome/browser/sync/api/sync_change.h"
 #include "chrome/browser/themes/theme_service.h"
 #include "chrome/browser/themes/theme_service_factory.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/global_error_service.h"
 #include "chrome/browser/ui/global_error_service_factory.h"
 #include "chrome/browser/ui/webui/chrome_url_data_manager.h"
 #include "chrome/browser/ui/webui/favicon_source.h"
 #include "chrome/browser/ui/webui/ntp/thumbnail_source.h"
+#include "chrome/browser/web_applications/web_app.h"
 #include "chrome/common/child_process_logging.h"
+#include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/extensions/extension_constants.h"
 #include "chrome/common/extensions/extension_error_utils.h"
 #include "chrome/common/extensions/extension_file_util.h"
 #include "chrome/common/extensions/extension_messages.h"
 #include "chrome/common/extensions/extension_resource.h"
 #include "chrome/common/pref_names.h"
@@ skipping 52 matching lines @@
 static bool IsSyncableExtension(const Extension& extension) {
   return extension.GetSyncType() == Extension::SYNC_TYPE_EXTENSION;
 }
 
 static bool IsSyncableApp(const Extension& extension) {
   return extension.GetSyncType() == Extension::SYNC_TYPE_APP;
 }
 
 }  // namespace
 
+// static
+bool ExtensionService::PlatformAppRequiresSeparateDataDirectory() {

[Aaron Boodman, 2012/02/10 20:09:46]

I think this would make more sense as a member of Extension:

bool Extension::RequiesSeparateUserDataDirectory() const {
#if defined(OS_MACOSX)
  return is_platform_app();
#else
  return false;
}

[sail, 2012/02/11 23:15:11]

Done.

+  // Currently separate data directories are only supported on Mac OSX.
+#if defined(OS_MACOSX)
+  return true;
+#else
+  return false;
+#endif
+}
+
 ExtensionService::ExtensionRuntimeData::ExtensionRuntimeData()
     : background_page_ready(false),
       being_upgraded(false),
       has_used_webrequest(false) {
 }
 
 ExtensionService::ExtensionRuntimeData::~ExtensionRuntimeData() {
 }
 
 ExtensionService::NaClModuleInfo::NaClModuleInfo() {
@@ skipping 2054 matching lines @@
   }
 
   // Do not record the install histograms for upgrades.
   if (!GetExtensionByIdInternal(extension->id(), true, true, false)) {
     UMA_HISTOGRAM_ENUMERATION("Extensions.InstallType",
                               extension->GetType(), 100);
     RecordPermissionMessagesHistogram(
         extension, "Extensions.Permissions_Install");
   }
 
-  extension_prefs_->OnExtensionInstalled(
-      extension,
-      initial_enable ? Extension::ENABLED : Extension::DISABLED,
-      from_webstore,
-      page_ordinal);
-
-  // Unpacked extensions default to allowing file access, but if that has been
-  // overridden, don't reset the value.

[Aaron Boodman, 2012/02/10 20:09:46]

Seems like these could also be moved into ExtensionPrefs::OnExtensionInstalled?

[sail, 2012/02/11 23:15:11]

Done.

-  if (Extension::ShouldAlwaysAllowFileAccess(extension->location()) &&
-      !extension_prefs_->HasAllowFileAccessSetting(id)) {
-    extension_prefs_->SetAllowFileAccess(id, true);
+  SetupExtensionPrefs(
+      extension_prefs_, extension, from_webstore, page_ordinal, initial_enable);
+  if (initial_enable && PlatformAppRequiresSeparateDataDirectory()) {
+    SetupExtensionPrefsInSeparateDataDirectory(
+        extension, from_webstore, page_ordinal);
   }
 
-  // If the extension should automatically block network startup (e.g., it uses
-  // the webRequest API), set the preference. Otherwise clear it, in case the
-  // extension stopped using a relevant API.
-  extension_prefs_->SetDelaysNetworkRequests(
-      extension->id(), extension->ImplicitlyDelaysNetworkStartup());
-
   content::NotificationService::current()->Notify(
       chrome::NOTIFICATION_EXTENSION_INSTALLED,
       content::Source<Profile>(profile_),
       content::Details<const Extension>(extension));
 
   // Transfer ownership of |extension| to AddExtension.
   AddExtension(scoped_extension);
 }
 
 const Extension* ExtensionService::GetExtensionByIdInternal(
@@ skipping 372 matching lines @@
   //
   // To coexist with certain unit tests that don't have an IO thread message
   // loop available at ExtensionService shutdown, we lazy-initialize this
   // object so that those cases neither create nor destroy a SocketController.
   CHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   if (!socket_controller_) {
     socket_controller_ = new extensions::SocketController();
   }
   return socket_controller_;
 }
+
+void ExtensionService::SetupExtensionPrefs(ExtensionPrefs* prefs,
+                                           const Extension* extension,
+                                           bool from_webstore,
+                                           const StringOrdinal& page_ordinal,
+                                           bool initial_enable) {
+  const std::string& id = extension->id();
+  prefs->OnExtensionInstalled(
+      extension,
+      initial_enable ? Extension::ENABLED : Extension::DISABLED,
+      from_webstore,
+      page_ordinal);
+
+  // Unpacked extensions default to allowing file access, but if that has been
+  // overridden, don't reset the value.
+  if (Extension::ShouldAlwaysAllowFileAccess(extension->location()) &&
+      !prefs->HasAllowFileAccessSetting(id)) {
+    prefs->SetAllowFileAccess(id, true);
+  }
+
+  // If the extension should automatically block network startup (e.g., it uses
+  // the webRequest API), set the preference. Otherwise clear it, in case the
+  // extension stopped using a relevant API.
+  prefs->SetDelaysNetworkRequests(
+      id, extension->ImplicitlyDelaysNetworkStartup());
+}
+
+void ExtensionService::SetupExtensionPrefsInSeparateDataDirectory(
+    const Extension* extension,
+    bool from_webstore,
+    const StringOrdinal& page_ordinal) {
+  const std::string& id = extension->id();
+  FilePath data_dir = web_app::GetWebAppDataDirectory(

[Aaron Boodman, 2012/02/10 20:09:46]

These few lines of code shows up in a couple places. Refactor into
Extension::GetPlatformAppUserDataDirectory()?

[sail, 2012/02/11 23:15:11]

This would involve moving several functions from web_app.cc to extension.cc. It
would aslo require a profile path to be passed in since the extension doesn't
have a profile reference.

To simplify things I added a new GetWebAppDataDirectory() overload that takes a
profile and an extension.

I also cleaned up the existing code. Currently there are 3 functions with very
similar names:
  GetWebAppDataDirectory
  GetWebAppDir
  GetDataDir
I consolidated the last two into the first.

+      profile_->GetPath(), id, GURL(extension->launch_web_url()));
+  FilePath profile_path = data_dir.AppendASCII(chrome::kInitialProfile);
+  FilePath pref_path = profile_path.Append(chrome::kPreferencesFilename);
+  scoped_ptr<PrefService> prefs_service(PrefService::CreatePrefService(
+      pref_path, NULL, false));
+
+  ExtensionPrefValueMap map;
+  ExtensionPrefs::RegisterUserPrefs(prefs_service.get());
+  ExtensionPrefs extension_prefs(prefs_service.get(), install_directory_, &map);
+  extension_prefs.Init(false);
+
+  SetupExtensionPrefs(
+      &extension_prefs, extension, from_webstore, page_ordinal, true);
+  extension_prefs.AddGrantedPermissions(id, extension->GetActivePermissions());
+  extension_prefs.SetExtensionState(id, Extension::ENABLED);
+
+  prefs_service->CommitPendingWrite();
+}