[Extensions] Move Permissions registration to the ExtensionsAPIProvider

Move Permissions registration out of the body of
ExtensionsClient::Initialize() and into the ExtensionsAPIProviders by
introducing ExtensionsAPIProvider::AddPermissionsProviders().

Have each provider register their associated permissions, and have
a common ExtensionsClient initialization step call into them in order
to trigger the registration.

Bug: 873937

Change-Id: Idfc55d0690dd8cf77f62b63650deec2ae42623a2
Reviewed-on: https://chromium-review.googlesource.com/1177181Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Reviewed-by: Luke Halliwell <halliwell@chromium.org>
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584788}

chrome/common/extensions/chrome_extensions_api_provider.cc

@@ -8,12 +8,14 @@
 #include "chrome/common/extensions/api/generated_schemas.h"
 #include "chrome/common/extensions/api/manifest_features.h"
 #include "chrome/common/extensions/api/permission_features.h"
+#include "chrome/common/extensions/chrome_aliases.h"
 #include "chrome/grit/common_resources.h"
 #include "extensions/common/features/json_feature_provider_source.h"
+#include "extensions/common/permissions/permissions_info.h"
 
 namespace extensions {
 
-ChromeExtensionsAPIProvider::ChromeExtensionsAPIProvider() = default;
+ChromeExtensionsAPIProvider::ChromeExtensionsAPIProvider() {}
 ChromeExtensionsAPIProvider::~ChromeExtensionsAPIProvider() = default;
 
 void ChromeExtensionsAPIProvider::AddAPIFeatures(FeatureProvider* provider) {
@@ -50,4 +52,9 @@ base::StringPiece ChromeExtensionsAPIProvider::GetAPISchema(
   return api::ChromeGeneratedSchemas::Get(name);
 }
 
+void ChromeExtensionsAPIProvider::AddPermissionsProviders(
+    PermissionsInfo* permissions_info) {
+  permissions_info->AddProvider(api_permissions_, GetChromePermissionAliases());
+}
+
 }  // namespace extensions

chrome/common/extensions/chrome_extensions_api_provider.h

@@ -6,6 +6,7 @@
 #define CHROME_COMMON_EXTENSIONS_CHROME_EXTENSIONS_API_PROVIDER_H_
 
 #include "base/macros.h"
+#include "chrome/common/extensions/permissions/chrome_api_permissions.h"
 #include "extensions/common/extensions_api_provider.h"
 
 namespace extensions {
@@ -23,8 +24,11 @@ class ChromeExtensionsAPIProvider : public ExtensionsAPIProvider {
   void AddAPIJSONSources(JSONFeatureProviderSource* json_source) override;
   bool IsAPISchemaGenerated(const std::string& name) override;
   base::StringPiece GetAPISchema(const std::string& name) override;
+  void AddPermissionsProviders(PermissionsInfo* permissions_info) override;
 
  private:
+  const ChromeAPIPermissions api_permissions_;
+
   DISALLOW_COPY_AND_ASSIGN(ChromeExtensionsAPIProvider);
 };
 

chrome/common/extensions/chrome_extensions_client.cc

@@ -16,7 +16,6 @@
 #include "chrome/common/chrome_content_client.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/extensions/api/extension_action/action_info.h"
-#include "chrome/common/extensions/chrome_aliases.h"
 #include "chrome/common/extensions/chrome_extensions_api_provider.h"
 #include "chrome/common/extensions/chrome_manifest_handlers.h"
 #include "chrome/common/extensions/manifest_handlers/theme_handler.h"
@@ -31,13 +30,11 @@
 #include "extensions/common/extension_api.h"
 #include "extensions/common/extension_icon_set.h"
 #include "extensions/common/extension_urls.h"
-#include "extensions/common/extensions_aliases.h"
 #include "extensions/common/features/feature_channel.h"
 #include "extensions/common/manifest_constants.h"
 #include "extensions/common/manifest_handler.h"
 #include "extensions/common/manifest_handlers/icons_handler.h"
 #include "extensions/common/permissions/api_permission_set.h"
-#include "extensions/common/permissions/permissions_info.h"
 #include "extensions/common/url_pattern.h"
 #include "extensions/common/url_pattern_set.h"
 #include "ui/base/l10n/l10n_util.h"
@@ -104,12 +101,6 @@ void ChromeExtensionsClient::Initialize() {
     ManifestHandler::FinalizeRegistration();
   }
 
-  // Set up permissions.
-  PermissionsInfo::GetInstance()->AddProvider(chrome_api_permissions_,
-                                              GetChromePermissionAliases());
-  PermissionsInfo::GetInstance()->AddProvider(extensions_api_permissions_,
-                                              GetExtensionsPermissionAliases());
-
   // Set up the scripting whitelist.
   // Whitelist ChromeVox, an accessibility extension from Google that needs
   // the ability to script webui pages. This is temporary and is not

chrome/common/extensions/chrome_extensions_client.h

@@ -9,10 +9,8 @@
 
 #include "base/compiler_specific.h"
 #include "base/macros.h"
-#include "chrome/common/extensions/permissions/chrome_api_permissions.h"
 #include "chrome/common/extensions/permissions/chrome_permission_message_provider.h"
 #include "extensions/common/extensions_client.h"
-#include "extensions/common/permissions/extensions_api_permissions.h"
 #include "url/gurl.h"
 
 namespace extensions {
@@ -51,8 +49,6 @@ class ChromeExtensionsClient : public ExtensionsClient {
   std::string GetUserAgent() const override;
 
  private:
-  const ChromeAPIPermissions chrome_api_permissions_;
-  const ExtensionsAPIPermissions extensions_api_permissions_;
   const ChromePermissionMessageProvider permission_message_provider_;
 
   // A whitelist of extensions that can script anywhere. Do not add to this

chromecast/common/cast_extensions_api_provider.cc

@@ -4,17 +4,19 @@
 
 #include "chromecast/common/cast_extensions_api_provider.h"
 
+#include "chromecast/common/extensions_api/cast_aliases.h"
 #include "chromecast/common/extensions_api/cast_api_features.h"
 #include "chromecast/common/extensions_api/cast_api_permissions.h"
 #include "chromecast/common/extensions_api/cast_manifest_features.h"
 #include "chromecast/common/extensions_api/cast_permission_features.h"
 #include "chromecast/common/extensions_api/generated_schemas.h"
 #include "extensions/common/features/json_feature_provider_source.h"
+#include "extensions/common/permissions/permissions_info.h"
 #include "extensions/shell/grit/app_shell_resources.h"
 
 namespace extensions {
 
-CastExtensionsAPIProvider::CastExtensionsAPIProvider() = default;
+CastExtensionsAPIProvider::CastExtensionsAPIProvider() {}
 CastExtensionsAPIProvider::~CastExtensionsAPIProvider() = default;
 
 void CastExtensionsAPIProvider::AddAPIFeatures(FeatureProvider* provider) {
@@ -48,4 +50,9 @@ base::StringPiece CastExtensionsAPIProvider::GetAPISchema(
   return cast::api::CastGeneratedSchemas::Get(name);
 }
 
+void CastExtensionsAPIProvider::AddPermissionsProviders(
+    PermissionsInfo* permissions_info) {
+  permissions_info->AddProvider(api_permissions_, GetCastPermissionAliases());
+}
+
 }  // namespace extensions

chromecast/common/cast_extensions_api_provider.h

@@ -6,6 +6,7 @@
 #define CHROMECAST_COMMON_CAST_EXTENSIONS_API_PROVIDER_H_
 
 #include "base/macros.h"
+#include "chromecast/common/extensions_api/cast_api_permissions.h"
 #include "extensions/common/extensions_api_provider.h"
 
 namespace extensions {
@@ -23,8 +24,11 @@ class CastExtensionsAPIProvider : public ExtensionsAPIProvider {
   void AddAPIJSONSources(JSONFeatureProviderSource* json_source) override;
   bool IsAPISchemaGenerated(const std::string& name) override;
   base::StringPiece GetAPISchema(const std::string& name) override;
+  void AddPermissionsProviders(PermissionsInfo* permissions_info) override;
 
  private:
+  const CastAPIPermissions api_permissions_;
+
   DISALLOW_COPY_AND_ASSIGN(CastExtensionsAPIProvider);
 };
 

chromecast/common/cast_extensions_client.cc

@@ -12,7 +12,6 @@
 #include "base/no_destructor.h"
 #include "chromecast/common/cast_extensions_api_provider.h"
 #include "chromecast/common/cast_redirect_manifest_handler.h"
-#include "chromecast/common/extensions_api/cast_aliases.h"
 #include "components/version_info/version_info.h"
 #include "content/public/common/user_agent.h"
 #include "extensions/common/api/api_features.h"
@@ -23,7 +22,6 @@
 #include "extensions/common/common_manifest_handlers.h"
 #include "extensions/common/core_extensions_api_provider.h"
 #include "extensions/common/extension_urls.h"
-#include "extensions/common/extensions_aliases.h"
 #include "extensions/common/features/feature_provider.h"
 #include "extensions/common/features/manifest_feature.h"
 #include "extensions/common/features/simple_feature.h"
@@ -31,8 +29,6 @@
 #include "extensions/common/manifest_handlers/automation.h"
 #include "extensions/common/manifest_handlers/content_scripts_handler.h"
 #include "extensions/common/permissions/permission_message_provider.h"
-#include "extensions/common/permissions/permissions_info.h"
-#include "extensions/common/permissions/permissions_provider.h"
 #include "extensions/common/url_pattern_set.h"
 
 namespace extensions {
@@ -80,8 +76,7 @@ class ShellPermissionMessageProvider : public PermissionMessageProvider {
 }  // namespace
 
 CastExtensionsClient::CastExtensionsClient()
-    : extensions_api_permissions_(ExtensionsAPIPermissions()),
-      webstore_base_url_(extension_urls::kChromeWebstoreBaseURL),
+    : webstore_base_url_(extension_urls::kChromeWebstoreBaseURL),
       webstore_update_url_(extension_urls::kChromeWebstoreUpdateURL) {
   AddAPIProvider(std::make_unique<CoreExtensionsAPIProvider>());
   AddAPIProvider(std::make_unique<CastExtensionsAPIProvider>());
@@ -94,11 +89,6 @@ void CastExtensionsClient::Initialize() {
   RegisterCastManifestHandlers();
   ManifestHandler::FinalizeRegistration();
   // TODO(jamescook): Do we need to whitelist any extensions?
-
-  PermissionsInfo::GetInstance()->AddProvider(cast_api_permissions_,
-                                              GetCastPermissionAliases());
-  PermissionsInfo::GetInstance()->AddProvider(extensions_api_permissions_,
-                                              GetExtensionsPermissionAliases());
 }
 
 void CastExtensionsClient::InitializeWebStoreUrls(

chromecast/common/cast_extensions_client.h

@@ -7,9 +7,7 @@
 
 #include "base/compiler_specific.h"
 #include "base/macros.h"
-#include "chromecast/common/extensions_api/cast_api_permissions.h"
 #include "extensions/common/extensions_client.h"
-#include "extensions/common/permissions/extensions_api_permissions.h"
 #include "url/gurl.h"
 
 namespace extensions {
@@ -43,9 +41,6 @@ class CastExtensionsClient : public ExtensionsClient {
   std::string GetUserAgent() const override;
 
  private:
-  const CastAPIPermissions cast_api_permissions_;
-  const ExtensionsAPIPermissions extensions_api_permissions_;
-
   ScriptingWhitelist scripting_whitelist_;
 
   const GURL webstore_base_url_;

extensions/common/core_extensions_api_provider.cc

@@ -9,12 +9,14 @@
 #include "extensions/common/api/generated_schemas.h"
 #include "extensions/common/api/manifest_features.h"
 #include "extensions/common/api/permission_features.h"
+#include "extensions/common/extensions_aliases.h"
 #include "extensions/common/features/json_feature_provider_source.h"
+#include "extensions/common/permissions/permissions_info.h"
 #include "extensions/grit/extensions_resources.h"
 
 namespace extensions {
 
-CoreExtensionsAPIProvider::CoreExtensionsAPIProvider() = default;
+CoreExtensionsAPIProvider::CoreExtensionsAPIProvider() {}
 CoreExtensionsAPIProvider::~CoreExtensionsAPIProvider() = default;
 
 void CoreExtensionsAPIProvider::AddAPIFeatures(FeatureProvider* provider) {
@@ -48,4 +50,10 @@ base::StringPiece CoreExtensionsAPIProvider::GetAPISchema(
   return api::GeneratedSchemas::Get(name);
 }
 
+void CoreExtensionsAPIProvider::AddPermissionsProviders(
+    PermissionsInfo* permissions_info) {
+  permissions_info->AddProvider(api_permissions_,
+                                GetExtensionsPermissionAliases());
+}
+
 }  // namespace extensions

extensions/common/core_extensions_api_provider.h

@@ -7,6 +7,7 @@
 
 #include "base/macros.h"
 #include "extensions/common/extensions_api_provider.h"
+#include "extensions/common/permissions/extensions_api_permissions.h"
 
 namespace extensions {
 
@@ -23,8 +24,11 @@ class CoreExtensionsAPIProvider : public ExtensionsAPIProvider {
   void AddAPIJSONSources(JSONFeatureProviderSource* json_source) override;
   bool IsAPISchemaGenerated(const std::string& name) override;
   base::StringPiece GetAPISchema(const std::string& name) override;
+  void AddPermissionsProviders(PermissionsInfo* permissions_info) override;
 
  private:
+  const ExtensionsAPIPermissions api_permissions_;
+
   DISALLOW_COPY_AND_ASSIGN(CoreExtensionsAPIProvider);
 };
 

extensions/common/extensions_api_provider.h

@@ -13,6 +13,7 @@
 namespace extensions {
 class FeatureProvider;
 class JSONFeatureProviderSource;
+class PermissionsInfo;
 
 // A class to provide API-specific bits and bobs to the extensions system.
 // This allows for composition of multiple providers, so that we can easily
@@ -39,6 +40,9 @@ class ExtensionsAPIProvider {
   // or an empty string if this provider doesn't know of the generated API.
   virtual base::StringPiece GetAPISchema(const std::string& name) = 0;
 
+  // Adds any associated permissions.
+  virtual void AddPermissionsProviders(PermissionsInfo* permissions_info) = 0;
+
  private:
   DISALLOW_COPY_AND_ASSIGN(ExtensionsAPIProvider);
 };

extensions/common/extensions_client.cc

@@ -10,6 +10,7 @@
 #include "extensions/common/features/feature_provider.h"
 #include "extensions/common/features/json_feature_provider_source.h"
 #include "extensions/common/manifest_handlers/icons_handler.h"
+#include "extensions/common/permissions/permissions_info.h"
 
 namespace extensions {
 
@@ -29,7 +30,7 @@ void ExtensionsClient::Set(ExtensionsClient* client) {
   if (g_client)
     return;
   g_client = client;
-  g_client->Initialize();
+  g_client->DoInitialize();
 }
 
 ExtensionsClient::ExtensionsClient() = default;
@@ -85,6 +86,8 @@ base::StringPiece ExtensionsClient::GetAPISchema(
 
 void ExtensionsClient::AddAPIProvider(
     std::unique_ptr<ExtensionsAPIProvider> provider) {
+  DCHECK(!initialize_called_)
+      << "APIProviders can only be added before client initialization.";
   api_providers_.push_back(std::move(provider));
 }
 
@@ -103,4 +106,14 @@ std::string ExtensionsClient::GetUserAgent() const {
   return std::string();
 }
 
+void ExtensionsClient::DoInitialize() {
+  initialize_called_ = true;
+
+  PermissionsInfo* permissions_info = PermissionsInfo::GetInstance();
+  for (const auto& provider : api_providers_)
+    provider->AddPermissionsProviders(permissions_info);
+
+  Initialize();
+}
+
 }  // namespace extensions

extensions/common/extensions_client.h

@@ -151,8 +151,15 @@ class ExtensionsClient {
   virtual std::string GetUserAgent() const;
 
  private:
+  // Performs common initialization and calls Initialize() to allow subclasses
+  // to do any extra initialization.
+  void DoInitialize();
+
   std::vector<std::unique_ptr<ExtensionsAPIProvider>> api_providers_;
 
+  // Whether DoInitialize() has been called.
+  bool initialize_called_ = false;
+
   DISALLOW_COPY_AND_ASSIGN(ExtensionsClient);
 };
 

extensions/shell/common/shell_extensions_api_provider.cc

@@ -47,4 +47,7 @@ base::StringPiece ShellExtensionsAPIProvider::GetAPISchema(
   return shell::api::ShellGeneratedSchemas::Get(name);
 }
 
+void ShellExtensionsAPIProvider::AddPermissionsProviders(
+    PermissionsInfo* permissions_info) {}
+
 }  // namespace extensions

extensions/shell/common/shell_extensions_api_provider.h

@@ -23,6 +23,7 @@ class ShellExtensionsAPIProvider : public ExtensionsAPIProvider {
   void AddAPIJSONSources(JSONFeatureProviderSource* json_source) override;
   bool IsAPISchemaGenerated(const std::string& name) override;
   base::StringPiece GetAPISchema(const std::string& name) override;
+  void AddPermissionsProviders(PermissionsInfo* permissions_info) override;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(ShellExtensionsAPIProvider);

extensions/shell/common/shell_extensions_client.cc

@@ -15,11 +15,9 @@
 #include "extensions/common/common_manifest_handlers.h"
 #include "extensions/common/core_extensions_api_provider.h"
 #include "extensions/common/extension_urls.h"
-#include "extensions/common/extensions_aliases.h"
 #include "extensions/common/features/simple_feature.h"
 #include "extensions/common/manifest_handler.h"
 #include "extensions/common/permissions/permission_message_provider.h"
-#include "extensions/common/permissions/permissions_info.h"
 #include "extensions/common/permissions/permissions_provider.h"
 #include "extensions/common/url_pattern_set.h"
 #include "extensions/shell/common/shell_extensions_api_provider.h"
@@ -65,8 +63,7 @@ base::LazyInstance<ShellPermissionMessageProvider>::DestructorAtExit
 }  // namespace
 
 ShellExtensionsClient::ShellExtensionsClient()
-    : extensions_api_permissions_(ExtensionsAPIPermissions()),
-      webstore_base_url_(extension_urls::kChromeWebstoreBaseURL),
+    : webstore_base_url_(extension_urls::kChromeWebstoreBaseURL),
       webstore_update_url_(extension_urls::kChromeWebstoreUpdateURL) {
   AddAPIProvider(std::make_unique<CoreExtensionsAPIProvider>());
   AddAPIProvider(std::make_unique<ShellExtensionsAPIProvider>());
@@ -79,9 +76,6 @@ void ShellExtensionsClient::Initialize() {
   RegisterCommonManifestHandlers();
   ManifestHandler::FinalizeRegistration();
   // TODO(jamescook): Do we need to whitelist any extensions?
-
-  PermissionsInfo::GetInstance()->AddProvider(extensions_api_permissions_,
-                                              GetExtensionsPermissionAliases());
 }
 
 void ShellExtensionsClient::InitializeWebStoreUrls(

extensions/shell/common/shell_extensions_client.h

@@ -8,7 +8,6 @@
 #include "base/compiler_specific.h"
 #include "base/macros.h"
 #include "extensions/common/extensions_client.h"
-#include "extensions/common/permissions/extensions_api_permissions.h"
 #include "url/gurl.h"
 
 namespace extensions {
@@ -42,8 +41,6 @@ class ShellExtensionsClient : public ExtensionsClient {
   std::string GetUserAgent() const override;
 
  private:
-  const ExtensionsAPIPermissions extensions_api_permissions_;
-
   ScriptingWhitelist scripting_whitelist_;
 
   const GURL webstore_base_url_;

extensions/test/test_extensions_client.cc

@@ -14,10 +14,7 @@
 #include "extensions/common/common_manifest_handlers.h"
 #include "extensions/common/core_extensions_api_provider.h"
 #include "extensions/common/extension_urls.h"
-#include "extensions/common/extensions_aliases.h"
 #include "extensions/common/manifest_handler.h"
-#include "extensions/common/permissions/extensions_api_permissions.h"
-#include "extensions/common/permissions/permissions_info.h"
 #include "extensions/common/url_pattern_set.h"
 #include "extensions/grit/extensions_resources.h"
 #include "extensions/test/test_permission_message_provider.h"
@@ -50,11 +47,6 @@ void TestExtensionsClient::Initialize() {
     RegisterCommonManifestHandlers();
     ManifestHandler::FinalizeRegistration();
   }
-
-  // Allow the core API permissions.
-  static ExtensionsAPIPermissions extensions_api_permissions;
-  PermissionsInfo::GetInstance()->AddProvider(extensions_api_permissions,
-                                              GetExtensionsPermissionAliases());
 }
 
 void TestExtensionsClient::InitializeWebStoreUrls(