[Extensions] Restrict background page entries to manifest v2

Manifest V3 extensions should be service-worker based. Restrict
background pages (both persistent and lazy/event-based) to manifest v2,
and add an error when trying to load them with manifest V3.

Add unittests for the same, and update previous unittests that exercised
manifest v3 with platform apps (since platform apps currently require
background.page or background.scripts, which are now disallowed).

Bug: 987838
Change-Id: I1cf9038e240a5c7b62341311c9342e3d1dea4237
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1665300
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#681936}

chrome/common/extensions/manifest_tests/extension_manifests_background_unittest.cc

@@ -26,6 +26,8 @@ namespace keys = manifest_keys;
 class ExtensionManifestBackgroundTest : public ChromeManifestTest {
 };
 
+// TODO(devlin): Can this file move to //extensions?
+
 TEST_F(ExtensionManifestBackgroundTest, BackgroundPermission) {
   LoadAndExpectError("background_permission.json",
                      errors::kBackgroundPermissionNeeded);
@@ -182,4 +184,73 @@ TEST_F(ExtensionManifestBackgroundTest, ServiceWorkerBasedBackgroundKey) {
   }
 }
 
+TEST_F(ExtensionManifestBackgroundTest, ManifestV3Restrictions) {
+  auto get_expected_error = [](base::StringPiece key) {
+    return ErrorUtils::FormatErrorMessage(
+        errors::kBackgroundSpecificationInvalidForManifestV3, key);
+  };
+
+  {
+    constexpr char kManifestBackgroundPage[] =
+        R"({
+             "name": "MV3 Test",
+             "manifest_version": 3,
+             "version": "0.1",
+             "background": {
+               "page": "background.html"
+             }
+           })";
+    base::Value manifest_value = base::test::ParseJson(kManifestBackgroundPage);
+    LoadAndExpectError(
+        ManifestData(std::move(manifest_value), "background page"),
+        get_expected_error(keys::kBackgroundPage));
+  }
+  {
+    constexpr char kManifestBackgroundScripts[] =
+        R"({
+             "name": "MV3 Test",
+             "manifest_version": 3,
+             "version": "0.1",
+             "background": {
+               "scripts": ["background.js"]
+             }
+           })";
+    base::Value manifest_value =
+        base::test::ParseJson(kManifestBackgroundScripts);
+    LoadAndExpectError(
+        ManifestData(std::move(manifest_value), "background scripts"),
+        get_expected_error(keys::kBackgroundScripts));
+  }
+  {
+    constexpr char kManifestBackgroundPersistent[] =
+        R"({
+             "name": "MV3 Test",
+             "manifest_version": 3,
+             "version": "0.1",
+             "background": {
+               "service_worker": "worker.js",
+               "persistent": true
+             }
+           })";
+    base::Value manifest_value =
+        base::test::ParseJson(kManifestBackgroundPersistent);
+    LoadAndExpectError(
+        ManifestData(std::move(manifest_value), "persistent background"),
+        get_expected_error(keys::kBackgroundPersistent));
+  }
+  {
+    // An extension with no background key present should still be allowed.
+    constexpr char kManifestBackgroundPersistent[] =
+        R"({
+             "name": "MV3 Test",
+             "manifest_version": 3,
+             "version": "0.1"
+           })";
+    base::Value manifest_value =
+        base::test::ParseJson(kManifestBackgroundPersistent);
+    LoadAndExpectSuccess(
+        ManifestData(std::move(manifest_value), "no background"));
+  }
+}
+
 }  // namespace extensions

extensions/common/manifest_constants.cc

@@ -270,6 +270,9 @@ const char kBackgroundPersistentInvalidForPlatformApps[] =
     "The key 'background.persistent' is not supported for packaged apps.";
 const char kBackgroundRequiredForPlatformApps[] =
     "Packaged apps must have a background page or background scripts.";
+const char kBackgroundSpecificationInvalidForManifestV3[] =
+    "The \"*\" key cannot be used with manifest_version 3. Use the "
+    "\"service_worker\" key instead.";
 const char kCannotAccessAboutUrl[] =
     "Cannot access \"*\" at origin \"*\". Extension must have permission to "
     "access the frame's origin, and matchAboutBlank must be true.";

extensions/common/manifest_constants.h

@@ -259,6 +259,7 @@ extern const char kAppsNotEnabled[];
 extern const char kBackgroundPermissionNeeded[];
 extern const char kBackgroundPersistentInvalidForPlatformApps[];
 extern const char kBackgroundRequiredForPlatformApps[];
+extern const char kBackgroundSpecificationInvalidForManifestV3[];
 extern const char kCannotAccessAboutUrl[];
 extern const char kCannotAccessChromeUrl[];
 extern const char kCannotAccessExtensionUrl[];

extensions/common/manifest_handlers/background_info.cc

@@ -47,6 +47,37 @@ const BackgroundInfo& GetBackgroundInfo(const Extension* extension) {
   return *info;
 }
 
+// Checks features that are restricted to manifest v2, populating |error| if
+// the |extension|'s manifest version is too high.
+// TODO(devlin): It's unfortunate that the features system doesn't handle this
+// automatically, but it only adds a warning (rather than an error). Depending
+// on how many keys we want to error on, it may make sense to change that.
+bool CheckManifestV2RestrictedFeatures(const Extension* extension,
+                                       base::string16* error) {
+  if (extension->manifest_version() < 3) {
+    // No special restrictions for manifest v2 extensions (or v1, if the legacy
+    // commandline flag is being used).
+    return true;
+  }
+
+  auto check_path = [error, extension](const char* path) {
+    if (extension->manifest()->HasPath(path)) {
+      *error = base::UTF8ToUTF16(ErrorUtils::FormatErrorMessage(
+          errors::kBackgroundSpecificationInvalidForManifestV3, path));
+      return false;
+    }
+    return true;
+  };
+
+  if (!check_path(keys::kBackgroundPage) ||
+      !check_path(keys::kBackgroundScripts) ||
+      !check_path(keys::kBackgroundPersistent)) {
+    return false;
+  }
+
+  return true;
+}
+
 }  // namespace
 
 BackgroundInfo::BackgroundInfo()
@@ -114,7 +145,8 @@ bool BackgroundInfo::IsServiceWorkerBased(const Extension* extension) {
 bool BackgroundInfo::Parse(const Extension* extension, base::string16* error) {
   const std::string& bg_scripts_key = extension->is_platform_app() ?
       keys::kPlatformAppBackgroundScripts : keys::kBackgroundScripts;
-  if (!LoadBackgroundScripts(extension, bg_scripts_key, error) ||
+  if (!CheckManifestV2RestrictedFeatures(extension, error) ||
+      !LoadBackgroundScripts(extension, bg_scripts_key, error) ||
       !LoadBackgroundPage(extension, error) ||
       !LoadBackgroundServiceWorkerScript(extension, error) ||
       !LoadBackgroundPersistent(extension, error) ||
@@ -239,8 +271,9 @@ bool BackgroundInfo::LoadBackgroundPersistent(const Extension* extension,
 
   const base::Value* background_persistent = NULL;
   if (!extension->manifest()->Get(keys::kBackgroundPersistent,
-                                  &background_persistent))
+                                  &background_persistent)) {
     return true;
+  }
 
   if (!background_persistent->GetAsBoolean(&is_persistent_)) {
     *error = ASCIIToUTF16(errors::kInvalidBackgroundPersistent);