Make platform apps get isolated storage by default.

Also centralizes the checks for when isolated storage is applicable to happen
in Extension::LoadAppIsolation, so that Extension::is_storage_isolated()
don't have to check for the experimental permission anymore.

[email protected]


Review URL: http://codereview.chromium.org/10349015

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@135269 0039d316-1c4b-4281-b951-d872f2087c98

Author: [email protected]

chrome/browser/extensions/extension_service.cc

@@ -826,9 +826,7 @@ bool ExtensionService::UninstallExtension(
 
   GURL launch_web_url_origin(extension->launch_web_url());
   launch_web_url_origin = launch_web_url_origin.GetOrigin();
-  bool is_storage_isolated =
-    (extension->is_storage_isolated() &&
-    extension->HasAPIPermission(ExtensionAPIPermission::kExperimental));
+  bool is_storage_isolated = extension->is_storage_isolated();
 
   if (extension->is_hosted_app() &&
       !profile_->GetExtensionSpecialStoragePolicy()->

chrome/browser/extensions/platform_app_browsertest.cc

@@ -5,6 +5,7 @@
 #include "base/command_line.h"
 #include "base/stringprintf.h"
 #include "base/utf_string_conversions.h"
+#include "chrome/browser/automation/automation_util.h"
 #include "chrome/browser/extensions/extension_apitest.h"
 #include "chrome/browser/extensions/extension_browsertest.h"
 #include "chrome/browser/extensions/extension_host.h"
@@ -179,3 +180,34 @@ IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest, Restrictions) {
 IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest, WindowsApi) {
   ASSERT_TRUE(RunPlatformAppTest("platform_apps/windows_api")) << message_;
 }
+
+// Tests that platform apps have isolated storage by default.
+IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest, Isolation) {
+  ASSERT_TRUE(StartTestServer());
+
+  // Load a (non-app) page under the "localhost" origin that sets a cookie.
+  GURL set_cookie_url = test_server()->GetURL(
+      "files/extensions/platform_apps/isolation/set_cookie.html");
+  GURL::Replacements replace_host;
+  std::string host_str("localhost");  // Must stay in scope with replace_host.
+  replace_host.SetHostStr(host_str);
+  set_cookie_url = set_cookie_url.ReplaceComponents(replace_host);
+
+  ui_test_utils::NavigateToURLWithDisposition(
+      browser(), set_cookie_url,
+      CURRENT_TAB, ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
+
+  // Make sure the cookie is set.
+  int cookie_size;
+  std::string cookie_value;
+  automation_util::GetCookies(
+      set_cookie_url,
+      browser()->GetWebContentsAt(0),
+      &cookie_size,
+      &cookie_value);
+  ASSERT_EQ("testCookie=1", cookie_value);
+
+  // Let the platform app request the same URL, and make sure that it doesn't
+  // see the cookie.
+  ASSERT_TRUE(RunPlatformAppTest("platform_apps/isolation")) << message_;
+}

chrome/browser/profiles/off_the_record_profile_impl.cc

@@ -282,9 +282,7 @@ net::URLRequestContextGetter*
   if (GetExtensionService()) {
     const Extension* installed_app = GetExtensionService()->
         GetInstalledAppForRenderer(renderer_child_id);
-    if (installed_app != NULL && installed_app->is_storage_isolated() &&
-        installed_app->HasAPIPermission(
-            ExtensionAPIPermission::kExperimental)) {
+    if (installed_app != NULL && installed_app->is_storage_isolated()) {
       return GetRequestContextForIsolatedApp(installed_app->id());
     }
   }

chrome/browser/profiles/profile_impl.cc

@@ -75,7 +75,6 @@
 #include "chrome/common/chrome_paths_internal.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/chrome_version_info.h"
-#include "chrome/common/extensions/extension_permission_set.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "content/public/browser/browser_thread.h"
@@ -704,9 +703,7 @@ net::URLRequestContextGetter* ProfileImpl::GetRequestContextForRenderProcess(
   if (extension_service) {
     const Extension* installed_app = extension_service->
         GetInstalledAppForRenderer(renderer_child_id);
-    if (installed_app != NULL && installed_app->is_storage_isolated() &&
-        installed_app->HasAPIPermission(
-            ExtensionAPIPermission::kExperimental)) {
+    if (installed_app != NULL && installed_app->is_storage_isolated()) {
       return GetRequestContextForIsolatedApp(installed_app->id());
     }
   }

chrome/common/extensions/api/_manifest_features.json

@@ -11,6 +11,12 @@
     "channel": "stable",
     "extension_types": ["packaged_app", "hosted_app"]
   },
+  "app.isolation": {
+    "channel": "stable",
+    // Platform apps always have isolated storage, thus they cannot specify it
+    // via the manifest.
+    "extension_types": ["packaged_app", "hosted_app"]
+  },
   "background": {
     "channel": "stable",
     "extension_types": [

chrome/common/extensions/extension.cc

@@ -2695,7 +2695,21 @@ bool Extension::LoadContentSecurityPolicy(string16* error) {
   return true;
 }
 
-bool Extension::LoadAppIsolation(string16* error) {
+bool Extension::LoadAppIsolation(
+    const ExtensionAPIPermissionSet& api_permissions, string16* error) {
+  // Platform apps always get isolated storage.
+  if (is_platform_app()) {
+    is_storage_isolated_ = true;
+    return true;
+  }
+
+  // Other apps only get it if it is requested _and_ experimental APIs are
+  // enabled.
+  if (!api_permissions.count(ExtensionAPIPermission::kExperimental) ||
+      !is_app()) {
+    return true;
+  }
+
   Value* temp = NULL;
   if (!manifest_->Get(keys::kIsolation, &temp))
     return true;
@@ -3082,9 +3096,7 @@ bool Extension::InitFromValue(int flags, string16* error) {
     return false;
   }
 
-  // App isolation.
-  if (api_permissions.count(ExtensionAPIPermission::kExperimental) &&
-      is_app() && !LoadAppIsolation(error))
+  if (!LoadAppIsolation(api_permissions, error))
     return false;
 
   if (!LoadSharedFeatures(api_permissions, error))

chrome/common/extensions/extension.h

@@ -677,7 +677,7 @@ class Extension : public base::RefCountedThreadSafe<Extension> {
   bool is_platform_app() const;
   bool is_hosted_app() const;
   bool is_packaged_app() const;
-  bool is_storage_isolated() const { return is_app() && is_storage_isolated_; }
+  bool is_storage_isolated() const { return is_storage_isolated_; }
   const URLPatternSet& web_extent() const { return extent_; }
   const std::string& launch_local_path() const { return launch_local_path_; }
   const std::string& launch_web_url() const { return launch_web_url_; }
@@ -751,7 +751,8 @@ class Extension : public base::RefCountedThreadSafe<Extension> {
   // extension from the manifest.
 
   bool CheckMinimumChromeVersion(string16* error);
-  bool LoadAppIsolation(string16* error);
+  bool LoadAppIsolation(const ExtensionAPIPermissionSet& api_permissions,
+                        string16* error);
 
   bool LoadRequiredFeatures(string16* error);
   bool LoadName(string16* error);

chrome/common/extensions/manifest_tests/extension_manifests_platformapp_unittest.cc

@@ -20,10 +20,11 @@ TEST_F(ExtensionManifestTest, PlatformApps) {
 
   CommandLine::ForCurrentProcess()->AppendSwitch(switches::kEnablePlatformApps);
 
-  LoadAndExpectSuccess("init_valid_platform_app.json");
-
   scoped_refptr<Extension> extension =
-      LoadAndExpectSuccess("init_invalid_platform_app_1.json");
+      LoadAndExpectSuccess("init_valid_platform_app.json");
+  EXPECT_TRUE(extension->is_storage_isolated());
+
+  extension = LoadAndExpectSuccess("init_invalid_platform_app_1.json");
   ASSERT_TRUE(extension);
   ASSERT_EQ(1u, extension->install_warnings().size());
   EXPECT_EQ("'app.launch' is not allowed for specified package type "

chrome/test/data/extensions/platform_apps/isolation/manifest.json

@@ -0,0 +1,12 @@
+{
+  "name": "Platform App Test: isolated storage",
+  "platform_app": true,
+  "version": "1",
+  "manifest_version": 2,
+  "background": {
+    "scripts": ["test.js"]
+  },
+  "permissions": [
+    "http://localhost/"
+  ]
+}

chrome/test/data/extensions/platform_apps/isolation/set_cookie.html

@@ -0,0 +1,9 @@
+<html>
+<body>
+<script>
+  // Tomorrow.
+  var expire = new Date(Date.now() + 24 * 60 * 60 * 1000);
+  document.cookie = 'testCookie=1; path=/; expires=' + expire + ';'
+</script>
+</body>
+</html>