Remove AppIsolationInfo and the app.isolation manifest key.

These are no longer needed now that isolated hosted apps have been removed. Bug: 159932, 1394513 Change-Id: Ibcdbf30afc82bdcdd36c185361648694abf56145 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4290205 Auto-Submit: Charlie Reis <creis@chromium.org> Reviewed-by: Devlin Cronin <rdevlin.cronin@chromium.org> Commit-Queue: Devlin Cronin <rdevlin.cronin@chromium.org> Cr-Commit-Position: refs/heads/main@{#1114846}
Denger-Network · Mar 8, 2023 · 507cabd · 507cabd
1 parent 2481af8
commit 507cabd
Show file tree

Hide file tree

Showing 16 changed files with 50 additions and 143 deletions.
diff --git a/chrome/browser/extensions/chrome_content_browser_client_extensions_part.cc b/chrome/browser/extensions/chrome_content_browser_client_extensions_part.cc
@@ -70,7 +70,6 @@
 #include "extensions/common/constants.h"
 #include "extensions/common/extension_urls.h"
 #include "extensions/common/manifest_constants.h"
-#include "extensions/common/manifest_handlers/app_isolation_info.h"
 #include "extensions/common/manifest_handlers/background_info.h"
 #include "extensions/common/manifest_handlers/web_accessible_resources_info.h"
 #include "extensions/common/mojom/event_router.mojom.h"

diff --git a/chrome/browser/extensions/data_deleter.cc b/chrome/browser/extensions/data_deleter.cc
@@ -12,6 +12,7 @@
 #include "chrome/browser/extensions/chrome_extension_cookies.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_special_storage_policy.h"
+#include "chrome/browser/extensions/extension_util.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_io_data.h"
 #include "chrome/common/extensions/manifest_handlers/app_launch_info.h"
@@ -26,7 +27,6 @@
 #include "extensions/browser/extension_util.h"
 #include "extensions/common/constants.h"
 #include "extensions/common/extension.h"
-#include "extensions/common/manifest_handlers/app_isolation_info.h"
 
 using base::WeakPtr;
 using content::BrowserContext;
@@ -106,7 +106,7 @@ void DataDeleter::StartDeleting(Profile* profile,
   GURL launch_web_url_origin;
   StoragePartition* partition = nullptr;
 
-  if (AppIsolationInfo::HasIsolatedStorage(extension)) {
+  if (extensions::util::LegacyHasIsolatedStorage(extension)) {
     has_isolated_storage = true;
     ++num_tasks;
   } else {

diff --git a/chrome/browser/extensions/extension_garbage_collector.cc b/chrome/browser/extensions/extension_garbage_collector.cc
@@ -24,6 +24,7 @@
 #include "base/time/time.h"
 #include "chrome/browser/extensions/extension_garbage_collector_factory.h"
 #include "chrome/browser/extensions/extension_service.h"
+#include "chrome/browser/extensions/extension_util.h"
 #include "chrome/browser/extensions/install_tracker.h"
 #include "chrome/browser/extensions/pending_extension_manager.h"
 #include "components/crx_file/id_util.h"
@@ -37,7 +38,6 @@
 #include "extensions/browser/extension_util.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/file_util.h"
-#include "extensions/common/manifest_handlers/app_isolation_info.h"
 
 namespace extensions {
 
@@ -223,7 +223,7 @@ void ExtensionGarbageCollector::GarbageCollectIsolatedStorageIfNeeded() {
   const ExtensionSet extensions =
       ExtensionRegistry::Get(context_)->GenerateInstalledExtensionsSet();
   for (const auto& ext : extensions) {
-    if (AppIsolationInfo::HasIsolatedStorage(ext.get())) {
+    if (extensions::util::LegacyHasIsolatedStorage(ext.get())) {
       active_paths.insert(
           util::GetStoragePartitionForExtensionId(ext->id(), context_)
               ->GetPath());

diff --git a/chrome/browser/extensions/extension_special_storage_policy.cc b/chrome/browser/extensions/extension_special_storage_policy.cc
@@ -24,6 +24,7 @@
 #include "base/thread_annotations.h"
 #include "build/chromeos_buildflags.h"
 #include "chrome/browser/content_settings/cookie_settings_factory.h"
+#include "chrome/browser/extensions/extension_util.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/url_constants.h"
 #include "components/content_settings/core/browser/cookie_settings.h"
@@ -35,7 +36,6 @@
 #include "extensions/common/constants.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/extension_set.h"
-#include "extensions/common/manifest_handlers/app_isolation_info.h"
 #include "extensions/common/manifest_handlers/content_capabilities_handler.h"
 #include "extensions/common/permissions/permissions_data.h"
 #include "url/gurl.h"
@@ -229,7 +229,7 @@ void ExtensionSpecialStoragePolicy::GrantRightsForExtension(
           APIPermissionID::kUnlimitedStorage) ||
       extension->permissions_data()->HasAPIPermission(
           APIPermissionID::kFileBrowserHandler) ||
-      extensions::AppIsolationInfo::HasIsolatedStorage(extension) ||
+      extensions::util::LegacyHasIsolatedStorage(extension) ||
       extension->is_app()) {
     if (NeedsProtection(extension) && protected_apps_.Add(extension)) {
       change_flags |= SpecialStoragePolicy::STORAGE_PROTECTED;
@@ -246,7 +246,7 @@ void ExtensionSpecialStoragePolicy::GrantRightsForExtension(
       file_handler_extensions_.Add(extension);
     }
 
-    if (extensions::AppIsolationInfo::HasIsolatedStorage(extension)) {
+    if (extensions::util::LegacyHasIsolatedStorage(extension)) {
       isolated_extensions_.Add(extension);
     }
   }
@@ -274,7 +274,7 @@ void ExtensionSpecialStoragePolicy::RevokeRightsForExtension(
           APIPermissionID::kUnlimitedStorage) ||
       extension->permissions_data()->HasAPIPermission(
           APIPermissionID::kFileBrowserHandler) ||
-      extensions::AppIsolationInfo::HasIsolatedStorage(extension) ||
+      extensions::util::LegacyHasIsolatedStorage(extension) ||
       extension->is_app()) {
     if (NeedsProtection(extension) && protected_apps_.Remove(extension)) {
       change_flags |= SpecialStoragePolicy::STORAGE_PROTECTED;
@@ -291,7 +291,7 @@ void ExtensionSpecialStoragePolicy::RevokeRightsForExtension(
       file_handler_extensions_.Remove(extension);
     }
 
-    if (extensions::AppIsolationInfo::HasIsolatedStorage(extension)) {
+    if (extensions::util::LegacyHasIsolatedStorage(extension)) {
       isolated_extensions_.Remove(extension);
     }
   }

diff --git a/chrome/browser/extensions/extension_util.cc b/chrome/browser/extensions/extension_util.cc
@@ -30,7 +30,6 @@
 #include "extensions/common/extension.h"
 #include "extensions/common/extension_icon_set.h"
 #include "extensions/common/extension_urls.h"
-#include "extensions/common/manifest_handlers/app_isolation_info.h"
 #include "extensions/common/manifest_handlers/incognito_info.h"
 #include "extensions/common/manifest_handlers/permissions_parser.h"
 #include "extensions/common/permissions/permissions_data.h"
@@ -78,18 +77,26 @@ bool HasIsolatedStorage(const std::string& extension_id,
   const Extension* extension =
       ExtensionRegistry::Get(context)->enabled_extensions().GetByID(
           extension_id);
+  return extension && HasIsolatedStorage(*extension, context);
+}
 
+bool HasIsolatedStorage(const Extension& extension,
+                        content::BrowserContext* context) {
 #if BUILDFLAG(IS_CHROMEOS_ASH)
   const bool is_policy_extension =
-      extension && Manifest::IsPolicyLocation(extension->location());
+      Manifest::IsPolicyLocation(extension.location());
   Profile* profile = Profile::FromBrowserContext(context);
   if (profile && ash::ProfileHelper::IsSigninProfile(profile) &&
       is_policy_extension) {
     return true;
   }
 #endif
 
-  return extension && AppIsolationInfo::HasIsolatedStorage(extension);
+  return LegacyHasIsolatedStorage(&extension);
+}
+
+bool LegacyHasIsolatedStorage(const Extension* extension) {
+  return extension->is_platform_app();
 }
 
 void SetIsIncognitoEnabled(const std::string& extension_id,

diff --git a/chrome/browser/extensions/extension_util.h b/chrome/browser/extensions/extension_util.h
@@ -33,6 +33,13 @@ namespace util {
 // the Chrome OS sign-in profile.
 bool HasIsolatedStorage(const std::string& extension_id,
                         content::BrowserContext* context);
+bool HasIsolatedStorage(const Extension& extension,
+                        content::BrowserContext* context);
+
+// Returns whether `extension` has isolated storage due to being a platform app.
+// TODO(https://crbug.com/159932): Move all callers to HasIsolatedStorage, so
+// that Chrome OS sign-in profile cases are handled as well.
+bool LegacyHasIsolatedStorage(const Extension* extension);
 
 // Sets whether |extension_id| can run in an incognito window. Reloads the
 // extension if it's enabled since this permission is applied at loading time

diff --git a/chrome/browser/extensions/extension_util_unittest.cc b/chrome/browser/extensions/extension_util_unittest.cc
@@ -12,16 +12,19 @@
 #include "components/sessions/content/session_tab_helper.h"
 #include "content/public/test/web_contents_tester.h"
 #include "extensions/browser/test_extension_registry_observer.h"
+#include "extensions/common/extension_builder.h"
 #include "extensions/common/permissions/permissions_data.h"
 #include "extensions/test/test_extension_dir.h"
 #include "url/gurl.h"
 
 namespace extensions {
 
-using ExtensionUtilUnittest = ExtensionServiceTestBase;
+class ExtensionUtilUnittest : public ExtensionServiceTestBase {
+ public:
+  void SetUp() override { InitializeEmptyExtensionService(); }
+};
 
 TEST_F(ExtensionUtilUnittest, SetAllowFileAccess) {
-  InitializeEmptyExtensionService();
   constexpr char kManifest[] =
       R"({
            "name": "foo",
@@ -78,7 +81,6 @@ TEST_F(ExtensionUtilUnittest, SetAllowFileAccess) {
 }
 
 TEST_F(ExtensionUtilUnittest, SetAllowFileAccessWhileDisabled) {
-  InitializeEmptyExtensionService();
   constexpr char kManifest[] =
       R"({
            "name": "foo",
@@ -146,4 +148,19 @@ TEST_F(ExtensionUtilUnittest, SetAllowFileAccessWhileDisabled) {
       file_url, tab_id, nullptr, CaptureRequirement::kActiveTabOrAllUrls));
 }
 
+TEST_F(ExtensionUtilUnittest, HasIsolatedStorage) {
+  // Platform apps should have isolated storage.
+  scoped_refptr<const Extension> app =
+      ExtensionBuilder("foo_app", ExtensionBuilder::Type::PLATFORM_APP).Build();
+  EXPECT_TRUE(app->is_platform_app());
+  EXPECT_TRUE(extensions::util::HasIsolatedStorage(*app.get(), profile()));
+
+  // Extensions should not have isolated storage.
+  scoped_refptr<const Extension> extension =
+      ExtensionBuilder("foo_ext", ExtensionBuilder::Type::EXTENSION).Build();
+  EXPECT_FALSE(extension->is_platform_app());
+  EXPECT_FALSE(
+      extensions::util::HasIsolatedStorage(*extension.get(), profile()));
+}
+
 }  // namespace extensions
diff --git a/chrome/browser/ui/extensions/extensions_overrides/simple_overrides_unittest.cc b/chrome/browser/ui/extensions/extensions_overrides/simple_overrides_unittest.cc
@@ -44,7 +44,6 @@ const char* kDisallowedFeatures[] = {
     extensions::manifest_keys::kApp,
     extensions::manifest_keys::kPlatformAppBackground,
     extensions::manifest_keys::kPlatformAppContentSecurityPolicy,
-    extensions::manifest_keys::kIsolation,
     extensions::manifest_keys::kLaunch,
     extensions::manifest_keys::kLinkedAppIcons,
     extensions::manifest_keys::kAutomation,

diff --git a/chrome/common/extensions/api/_manifest_features.json b/chrome/common/extensions/api/_manifest_features.json
@@ -20,12 +20,6 @@
     "channel": "stable",
     "extension_types": ["hosted_app"]
   },
-  "app.isolation": {
-    "channel": "stable",
-    // Platform apps always have isolated storage, thus they cannot specify it
-    // via the manifest.
-    "extension_types": ["legacy_packaged_app", "hosted_app"]
-  },
   "app.launch": {
     "channel": "stable",
     "extension_types": ["legacy_packaged_app", "hosted_app"]

diff --git a/chrome/common/extensions/chrome_manifest_handlers.cc b/chrome/common/extensions/chrome_manifest_handlers.cc
@@ -20,7 +20,6 @@
 #include "chrome/common/extensions/manifest_handlers/natively_connectable_handler.h"
 #include "chrome/common/extensions/manifest_handlers/settings_overrides_handler.h"
 #include "chrome/common/extensions/manifest_handlers/theme_handler.h"
-#include "extensions/common/manifest_handlers/app_isolation_info.h"
 #include "extensions/common/manifest_handlers/options_page_info.h"
 #include "extensions/common/manifest_url_handlers.h"
 
@@ -42,7 +41,6 @@ void RegisterChromeManifestHandlers() {
   DCHECK(!ManifestHandler::IsRegistrationFinalized());
 
   registry->RegisterHandler(std::make_unique<AboutPageHandler>());
-  registry->RegisterHandler(std::make_unique<AppIsolationHandler>());
   registry->RegisterHandler(std::make_unique<AppLaunchManifestHandler>());
   registry->RegisterHandler(std::make_unique<DevToolsPageHandler>());
   registry->RegisterHandler(std::make_unique<HomepageURLHandler>());

diff --git a/chrome/common/extensions/manifest_tests/extension_manifests_platformapp_unittest.cc b/chrome/common/extensions/manifest_tests/extension_manifests_platformapp_unittest.cc
@@ -10,7 +10,6 @@
 #include "extensions/common/error_utils.h"
 #include "extensions/common/features/simple_feature.h"
 #include "extensions/common/manifest_constants.h"
-#include "extensions/common/manifest_handlers/app_isolation_info.h"
 #include "extensions/common/manifest_handlers/csp_info.h"
 #include "extensions/common/manifest_handlers/incognito_info.h"
 #include "extensions/common/switches.h"
@@ -27,7 +26,10 @@ class PlatformAppsManifestTest : public ChromeManifestTest {
 TEST_F(PlatformAppsManifestTest, PlatformApps) {
   scoped_refptr<Extension> extension =
       LoadAndExpectSuccess("init_valid_platform_app.json");
-  EXPECT_TRUE(AppIsolationInfo::HasIsolatedStorage(extension.get()));
+  // Ensure this is treated as platform app, which causes it to have isolated
+  // storage in the browser process. See also
+  // ExtensionUtilUnittest.HasIsolatedStorage.
+  EXPECT_TRUE(extension->is_platform_app());
   EXPECT_FALSE(IncognitoInfo::IsSplitMode(extension.get()));
 
   extension =

diff --git a/extensions/common/BUILD.gn b/extensions/common/BUILD.gn
@@ -325,8 +325,6 @@ static_library("common") {
     "manifest_handler_helpers.h",
     "manifest_handlers/app_display_info.cc",
     "manifest_handlers/app_display_info.h",
-    "manifest_handlers/app_isolation_info.cc",
-    "manifest_handlers/app_isolation_info.h",
     "manifest_handlers/automation.cc",
     "manifest_handlers/automation.h",
     "manifest_handlers/background_info.cc",

diff --git a/extensions/common/manifest_constants.cc b/extensions/common/manifest_constants.cc
@@ -67,7 +67,6 @@ const char kImeOptionsPage[] = "options_page";
 const char kIndicator[] = "indicator";
 const char kInputComponents[] = "input_components";
 const char kInputView[] = "input_view";
-const char kIsolation[] = "app.isolation";
 const char kKey[] = "key";
 const char kKiosk[] = "kiosk";
 const char kKioskAlwaysUpdate[] = "kiosk.always_update";

diff --git a/extensions/common/manifest_constants.h b/extensions/common/manifest_constants.h
@@ -69,7 +69,6 @@ extern const char kImeOptionsPage[];
 extern const char kIndicator[];
 extern const char kInputComponents[];
 extern const char kInputView[];
-extern const char kIsolation[];
 extern const char kKey[];
 extern const char kKiosk[];
 extern const char kKioskAlwaysUpdate[];

diff --git a/extensions/common/manifest_handlers/app_isolation_info.cc b/extensions/common/manifest_handlers/app_isolation_info.cc