Improve parsing manifest for FSP API.

This CL makes the FSP section a prerequisite for the permission. It also adds
a warning if a section is used without a permission.

TEST=unit_tests: *FileSystemProviderHandlerTest*"
BUG=474146

Review URL: https://codereview.chromium.org/1133013002

Cr-Commit-Position: refs/heads/master@{#329374}

chrome/chrome_tests_unit.gypi

@@ -1326,6 +1326,7 @@
       'browser/ui/webui/chromeos/login/signin_userlist_unittest.cc',
       'browser/ui/webui/options/chromeos/cros_language_options_handler_unittest.cc',
       'common/extensions/api/file_browser_handlers/file_browser_handler_manifest_unittest.cc',
+      'common/extensions/api/file_system_provider/file_system_provider_handler_unittest.cc',
     ],
     'chrome_unit_tests_desktop_linux_sources': [
       'browser/password_manager/native_backend_kwallet_x_unittest.cc',

chrome/common/extensions/api/file_system_provider/file_system_provider_handler_unittest.cc

+// Copyright 2015 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/common/extensions/manifest_tests/chrome_manifest_test.h"
+#include "extensions/common/manifest_constants.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace extensions {
+
+class FileSystemProviderHandlerTest : public ChromeManifestTest {};
+
+TEST_F(FileSystemProviderHandlerTest, Valid) {
+  RunTestcase(Testcase("filesystemprovider_valid.json"), EXPECT_TYPE_SUCCESS);
+}
+
+TEST_F(FileSystemProviderHandlerTest, Invalid_MissingCapabilities) {
+  RunTestcase(
+      Testcase("filesystemprovider_missing_capabilities.json",
+               manifest_errors::kInvalidFileSystemProviderMissingCapabilities),
+      EXPECT_TYPE_ERROR);
+}
+
+TEST_F(FileSystemProviderHandlerTest, Invalid_MissingPermission) {
+  RunTestcase(
+      Testcase("filesystemprovider_missing_permission.json",
+               manifest_errors::kInvalidFileSystemProviderMissingPermission),
+      EXPECT_TYPE_WARNING);
+}
+
+}  // namespace extensions

chrome/common/extensions/api/file_system_provider_capabilities/file_system_provider_capabilities_handler.cc

@@ -10,6 +10,7 @@
 #include "chrome/common/extensions/api/manifest_types.h"
 #include "extensions/common/error_utils.h"
 #include "extensions/common/manifest_constants.h"
+#include "extensions/common/manifest_handlers/permissions_parser.h"
 #include "url/gurl.h"
 
 namespace extensions {
@@ -47,10 +48,30 @@ const FileSystemProviderCapabilities* FileSystemProviderCapabilities::Get(
 
 bool FileSystemProviderCapabilitiesHandler::Parse(Extension* extension,
                                                   base::string16* error) {
-  const base::DictionaryValue* section = NULL;
+  const bool has_permission = extensions::PermissionsParser::HasAPIPermission(
+      extension, extensions::APIPermission::ID::kFileSystemProvider);
+  const base::DictionaryValue* section = nullptr;
   extension->manifest()->GetDictionary(
       manifest_keys::kFileSystemProviderCapabilities, &section);
-  DCHECK(section);
+
+  if (has_permission && !section) {
+    *error = base::ASCIIToUTF16(
+        manifest_errors::kInvalidFileSystemProviderMissingCapabilities);
+    return false;
+  }
+
+  if (!has_permission && section) {
+    // If the permission is not specified, then the section has simply no
+    // effect, hence just a warning.
+    extension->AddInstallWarning(extensions::InstallWarning(
+        manifest_errors::kInvalidFileSystemProviderMissingPermission));
+    return true;
+  }
+
+  if (!has_permission && !section) {
+    // No permission and no capabilities, so ignore.
+    return true;
+  }
 
   api::manifest_types::FileSystemProviderCapabilities idl_capabilities;
   if (!api::manifest_types::FileSystemProviderCapabilities::Populate(
@@ -88,4 +109,9 @@ const std::vector<std::string> FileSystemProviderCapabilitiesHandler::Keys()
   return SingleKey(manifest_keys::kFileSystemProviderCapabilities);
 }
 
+bool FileSystemProviderCapabilitiesHandler::AlwaysParseForType(
+    Manifest::Type type) const {
+  return true;
+}
+
 }  // namespace extensions

chrome/common/extensions/api/file_system_provider_capabilities/file_system_provider_capabilities_handler.h

@@ -48,6 +48,7 @@ class FileSystemProviderCapabilitiesHandler : public ManifestHandler {
 
   // ManifestHandler overrides.
   bool Parse(Extension* extension, base::string16* error) override;
+  bool AlwaysParseForType(Manifest::Type type) const override;
 
  private:
   const std::vector<std::string> Keys() const override;

chrome/test/data/extensions/api_test/file_system_provider/add_watcher/manifest.json

@@ -13,6 +13,11 @@
     },
     "fileManagerPrivate"
   ],
+  "file_system_provider_capabilities": {
+    "configurable": true,
+    "multiple_mounts": true,
+    "source": "device"
+  },
   "app": {
     "background": {
       "scripts": [

chrome/test/data/extensions/api_test/file_system_provider/big_file/manifest.json

@@ -13,6 +13,11 @@
     },
     "fileManagerPrivate"
   ],
+  "file_system_provider_capabilities": {
+    "configurable": true,
+    "multiple_mounts": true,
+    "source": "device"
+  },
   "app": {
     "background": {
       "scripts": [

chrome/test/data/extensions/api_test/file_system_provider/copy_entry/manifest.json

@@ -13,6 +13,11 @@
     },
     "fileManagerPrivate"
   ],
+  "file_system_provider_capabilities": {
+    "configurable": true,
+    "multiple_mounts": true,
+    "source": "device"
+  },
   "app": {
     "background": {
       "scripts": [

chrome/test/data/extensions/api_test/file_system_provider/create_directory/manifest.json

@@ -13,6 +13,11 @@
     },
     "fileManagerPrivate"
   ],
+  "file_system_provider_capabilities": {
+    "configurable": true,
+    "multiple_mounts": true,
+    "source": "device"
+  },
   "app": {
     "background": {
       "scripts": [

chrome/test/data/extensions/api_test/file_system_provider/create_file/manifest.json

@@ -13,6 +13,11 @@
     },
     "fileManagerPrivate"
   ],
+  "file_system_provider_capabilities": {
+    "configurable": true,
+    "multiple_mounts": true,
+    "source": "device"
+  },
   "app": {
     "background": {
       "scripts": [

chrome/test/data/extensions/api_test/file_system_provider/delete_entry/manifest.json

@@ -13,6 +13,11 @@
     },
     "fileManagerPrivate"
   ],
+  "file_system_provider_capabilities": {
+    "configurable": true,
+    "multiple_mounts": true,
+    "source": "device"
+  },
   "app": {
     "background": {
       "scripts": [

chrome/test/data/extensions/api_test/file_system_provider/evil/manifest.json

@@ -13,6 +13,11 @@
     },
     "fileManagerPrivate"
   ],
+  "file_system_provider_capabilities": {
+    "configurable": true,
+    "multiple_mounts": true,
+    "source": "device"
+  },
   "app": {
     "background": {
       "scripts": [

chrome/test/data/extensions/api_test/file_system_provider/extension/manifest.json

@@ -6,6 +6,11 @@
   "manifest_version": 2,
   "description": "Test for chrome.fileSystemProvider.mount() in an extension.",
   "permissions": ["fileSystemProvider", "fileManagerPrivate"],
+  "file_system_provider_capabilities": {
+    "configurable": true,
+    "multiple_mounts": true,
+    "source": "device"
+  },
   "background": {
     "persistent": false,
     "scripts": [

chrome/test/data/extensions/api_test/file_system_provider/get_all/manifest.json

@@ -12,6 +12,11 @@
     },
     "fileManagerPrivate"
   ],
+  "file_system_provider_capabilities": {
+    "configurable": true,
+    "multiple_mounts": true,
+    "source": "device"
+  },
   "app": {
     "background": {
       "scripts": [

chrome/test/data/extensions/api_test/file_system_provider/get_metadata/manifest.json

@@ -13,6 +13,11 @@
     },
     "fileManagerPrivate"
   ],
+  "file_system_provider_capabilities": {
+    "configurable": true,
+    "multiple_mounts": true,
+    "source": "device"
+  },
   "app": {
     "background": {
       "scripts": [

chrome/test/data/extensions/api_test/file_system_provider/mime_type/manifest.json

@@ -18,6 +18,11 @@
       "types": ["text/secret-testing-mime-type"]
     }
   },
+  "file_system_provider_capabilities": {
+    "configurable": true,
+    "multiple_mounts": true,
+    "source": "device"
+  },
   "app": {
     "background": {
       "scripts": [

chrome/test/data/extensions/api_test/file_system_provider/move_entry/manifest.json

@@ -13,6 +13,11 @@
     },
     "fileManagerPrivate"
   ],
+  "file_system_provider_capabilities": {
+    "configurable": true,
+    "multiple_mounts": true,
+    "source": "device"
+  },
   "app": {
     "background": {
       "scripts": [

chrome/test/data/extensions/api_test/file_system_provider/notify/manifest.json

@@ -13,6 +13,11 @@
     },
     "fileManagerPrivate"
   ],
+  "file_system_provider_capabilities": {
+    "configurable": true,
+    "multiple_mounts": true,
+    "source": "device"
+  },
   "app": {
     "background": {
       "scripts": [

chrome/test/data/extensions/api_test/file_system_provider/read_directory/manifest.json

@@ -13,6 +13,11 @@
     },
     "fileManagerPrivate"
   ],
+  "file_system_provider_capabilities": {
+    "configurable": true,
+    "multiple_mounts": true,
+    "source": "device"
+  },
   "app": {
     "background": {
       "scripts": [

chrome/test/data/extensions/api_test/file_system_provider/read_file/manifest.json

@@ -13,6 +13,11 @@
     },
     "fileManagerPrivate"
   ],
+  "file_system_provider_capabilities": {
+    "configurable": true,
+    "multiple_mounts": true,
+    "source": "device"
+  },
   "app": {
     "background": {
       "scripts": [

chrome/test/data/extensions/api_test/file_system_provider/remove_watcher/manifest.json

@@ -13,6 +13,11 @@
     },
     "fileManagerPrivate"
   ],
+  "file_system_provider_capabilities": {
+    "configurable": true,
+    "multiple_mounts": true,
+    "source": "device"
+  },
   "app": {
     "background": {
       "scripts": [

chrome/test/data/extensions/api_test/file_system_provider/thumbnail/manifest.json

@@ -13,6 +13,11 @@
     },
     "fileManagerPrivate"
   ],
+  "file_system_provider_capabilities": {
+    "configurable": true,
+    "multiple_mounts": true,
+    "source": "device"
+  },
   "app": {
     "background": {
       "scripts": [

chrome/test/data/extensions/api_test/file_system_provider/truncate/manifest.json

@@ -13,6 +13,11 @@
     },
     "fileManagerPrivate"
   ],
+  "file_system_provider_capabilities": {
+    "configurable": true,
+    "multiple_mounts": true,
+    "source": "device"
+  },
   "app": {
     "background": {
       "scripts": [

chrome/test/data/extensions/api_test/file_system_provider/unmount/manifest.json

@@ -5,6 +5,11 @@
   "manifest_version": 2,
   "description": "Test for chrome.fileSystemProvider.unmount().",
   "permissions": ["fileSystemProvider", "fileManagerPrivate"],
+  "file_system_provider_capabilities": {
+    "configurable": true,
+    "multiple_mounts": true,
+    "source": "device"
+  },
   "app": {
     "background": {
       "scripts": [

chrome/test/data/extensions/api_test/file_system_provider/write_file/manifest.json

@@ -13,6 +13,11 @@
     },
     "fileManagerPrivate"
   ],
+  "file_system_provider_capabilities": {
+    "configurable": true,
+    "multiple_mounts": true,
+    "source": "device"
+  },
   "app": {
     "background": {
       "scripts": [

chrome/test/data/extensions/manifest_tests/filesystemprovider_missing_capabilities.json

+{
+  "name": "test",
+  "version": "1",
+  "permissions": [
+    "fileSystemProvider"
+  ]
+}

chrome/test/data/extensions/manifest_tests/filesystemprovider_missing_permission.json

+{
+  "name": "test",
+  "version": "1",
+  "file_system_provider_capabilities": {
+    "configurable": false,
+    "multiple_mounts": false,
+    "source": "device"
+  }
+}

chrome/test/data/extensions/manifest_tests/filesystemprovider_valid.json

+{
+  "name": "test",
+  "version": "1",
+  "permissions": [
+    "fileSystemProvider"
+  ],
+  "file_system_provider_capabilities": {
+    "configurable": false,
+    "multiple_mounts": false,
+    "source": "device"
+  }
+}

extensions/common/manifest_constants.cc

@@ -713,7 +713,14 @@ const char kWebRequestConflictsWithLazyBackground[] =
     "The 'webRequest' API cannot be used with event pages.";
 #if defined(OS_CHROMEOS)
 const char kIllegalPlugins[] =
-    "Extensions cannot install plugins on Chrome OS";
+    "Extensions cannot install plugins on Chrome OS.";
+const char kInvalidFileSystemProviderMissingCapabilities[] =
+    "The 'fileSystemProvider' permission requires the "
+    "'file_system_provider_capabilities' section to be specified in the "
+    "manifest.";
+const char kInvalidFileSystemProviderMissingPermission[] =
+    "The 'file_system_provider_capabilities' section requires the "
+    "'fileSystemProvider' permission to be specified in the manifest.";
 #endif
 
 }  // namespace manifest_errors

extensions/common/manifest_constants.h

@@ -479,6 +479,8 @@ extern const char kUnrecognizedManifestProperty[];
 extern const char kWebRequestConflictsWithLazyBackground[];
 #if defined(OS_CHROMEOS)
 extern const char kIllegalPlugins[];
+extern const char kInvalidFileSystemProviderMissingCapabilities[];
+extern const char kInvalidFileSystemProviderMissingPermission[];
 #endif
 
 }  // namespace manifest_errors