[Extensions] Remove manifest v1 support for accessible resources

In manifest version 1, all extension resources were considered
accessible by the web (i.e., could be embedded within or
requested by a web page). In manifest v2, only resources
explicitly specified in the web_accessible_resources key are
accessible. Remove support for manifest v1, and always check
the web accessible resources.

Update unit tests to migrate relevant tests and remove tests for
manifest v1-specific behavior.

Bug: 816677, 816679
Change-Id: Id0dd3ef75cd5d49e3f87e45ac3a8b19a00391146
Reviewed-on: https://chromium-review.googlesource.com/953177
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#545898}

chrome/browser/extensions/extension_protocols_unittest.cc

@@ -97,19 +97,24 @@ scoped_refptr<Extension> CreateTestExtension(const std::string& name,
 }
 
 scoped_refptr<Extension> CreateWebStoreExtension() {
-  base::DictionaryValue manifest;
-  manifest.SetString("name", "WebStore");
-  manifest.SetString("version", "1");
-  manifest.SetString("icons.16", "webstore_icon_16.png");
+  std::unique_ptr<base::DictionaryValue> manifest =
+      DictionaryBuilder()
+          .Set("name", "WebStore")
+          .Set("version", "1")
+          .Set("manifest_version", 2)
+          .Set("icons",
+               DictionaryBuilder().Set("16", "webstore_icon_16.png").Build())
+          .Set("web_accessible_resources",
+               ListBuilder().Append("webstore_icon_16.png").Build())
+          .Build();
 
   base::FilePath path;
   EXPECT_TRUE(PathService::Get(chrome::DIR_RESOURCES, &path));
   path = path.AppendASCII("web_store");
 
   std::string error;
-  scoped_refptr<Extension> extension(
-      Extension::Create(path, Manifest::COMPONENT, manifest,
-                        Extension::NO_FLAGS, &error));
+  scoped_refptr<Extension> extension(Extension::Create(
+      path, Manifest::COMPONENT, *manifest, Extension::NO_FLAGS, &error));
   EXPECT_TRUE(extension.get()) << error;
   return extension;
 }

chrome/common/extensions/manifest_tests/extension_manifests_web_accessible_resources_unittest.cc

@@ -13,70 +13,43 @@ class WebAccessibleResourcesManifestTest : public ChromeManifestTest {
 };
 
 TEST_F(WebAccessibleResourcesManifestTest, WebAccessibleResources) {
-  // Manifest version 2 with web accessible resources specified.
-  scoped_refptr<Extension> extension1(
-      LoadAndExpectSuccess("web_accessible_resources_1.json"));
-
-  // Manifest version 2 with no web accessible resources.
-  scoped_refptr<Extension> extension2(
-      LoadAndExpectSuccess("web_accessible_resources_2.json"));
-
-  // Default manifest version with web accessible resources specified.
-  scoped_refptr<Extension> extension3(
-      LoadAndExpectSuccess("web_accessible_resources_3.json"));
-
-  // Default manifest version with no web accessible resources.
-  scoped_refptr<Extension> extension4(
-      LoadAndExpectSuccess("web_accessible_resources_4.json"));
-
-  // Default manifest version with wildcard web accessible resource.
-  scoped_refptr<Extension> extension5(
-      LoadAndExpectSuccess("web_accessible_resources_5.json"));
-
-  // Default manifest version with wildcard with specific path and extension.
-  scoped_refptr<Extension> extension6(
-      LoadAndExpectSuccess("web_accessible_resources_6.json"));
-
-  EXPECT_TRUE(
-      WebAccessibleResourcesInfo::HasWebAccessibleResources(extension1.get()));
+  // No web_accessible_resources.
+  scoped_refptr<Extension> none(
+      LoadAndExpectSuccess("web_accessible_resources_none.json"));
   EXPECT_FALSE(
-      WebAccessibleResourcesInfo::HasWebAccessibleResources(extension2.get()));
-  EXPECT_TRUE(
-      WebAccessibleResourcesInfo::HasWebAccessibleResources(extension3.get()));
+      WebAccessibleResourcesInfo::HasWebAccessibleResources(none.get()));
   EXPECT_FALSE(
-      WebAccessibleResourcesInfo::HasWebAccessibleResources(extension4.get()));
+      WebAccessibleResourcesInfo::IsResourceWebAccessible(none.get(), "test"));
+
+  // web_accessible_resources: ["test"].
+  scoped_refptr<Extension> single(
+      LoadAndExpectSuccess("web_accessible_resources_single.json"));
   EXPECT_TRUE(
-      WebAccessibleResourcesInfo::HasWebAccessibleResources(extension5.get()));
+      WebAccessibleResourcesInfo::HasWebAccessibleResources(single.get()));
+  EXPECT_TRUE(WebAccessibleResourcesInfo::IsResourceWebAccessible(single.get(),
+                                                                  "test"));
+  EXPECT_FALSE(WebAccessibleResourcesInfo::IsResourceWebAccessible(single.get(),
+                                                                   "other"));
+
+  // web_accessible_resources: ["*"].
+  scoped_refptr<Extension> wildcard(
+      LoadAndExpectSuccess("web_accessible_resources_wildcard.json"));
   EXPECT_TRUE(
-      WebAccessibleResourcesInfo::HasWebAccessibleResources(extension6.get()));
-
-  EXPECT_TRUE(WebAccessibleResourcesInfo::IsResourceWebAccessible(
-      extension1.get(), "test"));
-  EXPECT_FALSE(WebAccessibleResourcesInfo::IsResourceWebAccessible(
-      extension1.get(), "none"));
-
-  EXPECT_FALSE(WebAccessibleResourcesInfo::IsResourceWebAccessible(
-      extension2.get(), "test"));
-
+      WebAccessibleResourcesInfo::HasWebAccessibleResources(wildcard.get()));
   EXPECT_TRUE(WebAccessibleResourcesInfo::IsResourceWebAccessible(
-      extension3.get(), "test"));
-  EXPECT_FALSE(WebAccessibleResourcesInfo::IsResourceWebAccessible(
-      extension3.get(), "none"));
-
+      wildcard.get(), "anything"));
   EXPECT_TRUE(WebAccessibleResourcesInfo::IsResourceWebAccessible(
-      extension4.get(), "test"));
-  EXPECT_TRUE(WebAccessibleResourcesInfo::IsResourceWebAccessible(
-      extension4.get(), "none"));
-
-  EXPECT_TRUE(WebAccessibleResourcesInfo::IsResourceWebAccessible(
-      extension5.get(), "anything"));
-  EXPECT_TRUE(WebAccessibleResourcesInfo::IsResourceWebAccessible(
-      extension5.get(), "path/anything"));
+      wildcard.get(), "path/anything"));
 
+  // web_accessible_resources: ["path/*.ext"].
+  scoped_refptr<Extension> pattern(
+      LoadAndExpectSuccess("web_accessible_resources_pattern.json"));
+  EXPECT_TRUE(
+      WebAccessibleResourcesInfo::HasWebAccessibleResources(pattern.get()));
   EXPECT_TRUE(WebAccessibleResourcesInfo::IsResourceWebAccessible(
-      extension6.get(), "path/anything.ext"));
+      pattern.get(), "path/anything.ext"));
   EXPECT_FALSE(WebAccessibleResourcesInfo::IsResourceWebAccessible(
-      extension6.get(), "anything.ext"));
+      pattern.get(), "anything.ext"));
   EXPECT_FALSE(WebAccessibleResourcesInfo::IsResourceWebAccessible(
-      extension6.get(), "path/anything.badext"));
+      pattern.get(), "path/anything.badext"));
 }

chrome/renderer/extensions/resource_request_policy.cc

@@ -34,12 +34,6 @@ ResourceRequestPolicy::~ResourceRequestPolicy() = default;
 
 void ResourceRequestPolicy::OnExtensionLoaded(const Extension& extension) {
   if (WebAccessibleResourcesInfo::HasWebAccessibleResources(&extension) ||
-      // Extensions below manifest version 2 had all resources accessible by
-      // default.
-      // TODO(devlin): Two things - first, we might not have any v1 extensions
-      // anymore; second, this should maybe be included in
-      // HasWebAccessibleResources().
-      extension.manifest_version() < 2 ||
       WebviewInfo::HasWebviewAccessibleResources(
           extension, dispatcher_->webview_partition_id()) ||
       // Hosted app icons are accessible.

chrome/test/data/extensions/manifest_tests/web_accessible_resources_2.json → chrome/test/data/extensions/manifest_tests/web_accessible_resources_none.json

@@ -2,4 +2,4 @@
   "name": "test",
   "version": "0.1",
   "manifest_version": 2
-}
+}

chrome/test/data/extensions/manifest_tests/web_accessible_resources_6.json → chrome/test/data/extensions/manifest_tests/web_accessible_resources_pattern.json

@@ -1,5 +1,6 @@
 {
   "name": "test",
+  "manifest_version": 2,
   "version": "0.1",
   "web_accessible_resources": ["path/*.ext"]
-}
+}

chrome/test/data/extensions/manifest_tests/web_accessible_resources_5.json → chrome/test/data/extensions/manifest_tests/web_accessible_resources_wildcard.json

@@ -1,5 +1,6 @@
 {
   "name": "test",
+  "manifest_version": 2,
   "version": "0.1",
   "web_accessible_resources": ["*"]
-}
+}

extensions/browser/extension_protocols.cc

@@ -506,10 +506,7 @@ void GetSecurityPolicyForURL(const GURL& url,
                                                               resource_path);
   }
 
-  if ((extension->manifest_version() >= 2 ||
-       extensions::WebAccessibleResourcesInfo::HasWebAccessibleResources(
-           extension)) &&
-      extensions::WebAccessibleResourcesInfo::IsResourceWebAccessible(
+  if (extensions::WebAccessibleResourcesInfo::IsResourceWebAccessible(
           extension, resource_path)) {
     *send_cors_header = true;
   }

extensions/browser/url_request_util.cc

@@ -79,13 +79,6 @@ bool AllowCrossRendererResourceLoad(const GURL& url,
 
   DCHECK_EQ(extension->url(), url.GetWithEmptyPath());
 
-  // Extensions with manifest before v2 did not have web_accessible_resource
-  // section, therefore the request needs to be allowed.
-  if (extension->manifest_version() < 2) {
-    *allowed = true;
-    return true;
-  }
-
   // Navigating the main frame to an extension URL is allowed, even if not
   // explicitly listed as web_accessible_resource.
   if (resource_type == content::RESOURCE_TYPE_MAIN_FRAME) {

extensions/common/manifest_handlers/web_accessible_resources_info.cc

@@ -39,12 +39,6 @@ WebAccessibleResourcesInfo::~WebAccessibleResourcesInfo() {
 bool WebAccessibleResourcesInfo::IsResourceWebAccessible(
     const Extension* extension,
     const std::string& relative_path) {
-  // For old manifest versions which do not specify web_accessible_resources
-  // we always allow resource loads.
-  if (extension->manifest_version() < 2 &&
-      !WebAccessibleResourcesInfo::HasWebAccessibleResources(extension))
-    return true;
-
   const WebAccessibleResourcesInfo* info = GetResourcesInfo(extension);
   return info &&
          extension->ResourceMatches(