[Extensions] Move ManifestHandler registration into ExtensionsAPIProvider

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

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

Bug: 873937

Change-Id: I8c400efd9d91968a8dd28b249483ded31bbc79e0
Reviewed-on: https://chromium-review.googlesource.com/1177190Reviewed-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@{#585386}

chrome/common/extensions/chrome_extensions_api_provider.cc

@@ -9,6 +9,7 @@
 #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/common/extensions/chrome_manifest_handlers.h"
 #include "chrome/grit/common_resources.h"
 #include "extensions/common/features/json_feature_provider_source.h"
 #include "extensions/common/permissions/permissions_info.h"
@@ -57,4 +58,8 @@ void ChromeExtensionsAPIProvider::AddPermissionsProviders(
   permissions_info->AddProvider(api_permissions_, GetChromePermissionAliases());
 }
 
+void ChromeExtensionsAPIProvider::RegisterManifestHandlers() {
+  RegisterChromeManifestHandlers();
+}
+
 }  // namespace extensions

chrome/common/extensions/chrome_extensions_api_provider.h

@@ -25,6 +25,7 @@ class ChromeExtensionsAPIProvider : public ExtensionsAPIProvider {
   bool IsAPISchemaGenerated(const std::string& name) override;
   base::StringPiece GetAPISchema(const std::string& name) override;
   void AddPermissionsProviders(PermissionsInfo* permissions_info) override;
+  void RegisterManifestHandlers() override;
 
  private:
   const ChromeAPIPermissions api_permissions_;

chrome/common/extensions/chrome_extensions_client.cc

@@ -17,13 +17,11 @@
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/extensions/api/extension_action/action_info.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"
 #include "chrome/common/url_constants.h"
 #include "chrome/grit/chromium_strings.h"
 #include "components/version_info/version_info.h"
 #include "content/public/common/url_constants.h"
-#include "extensions/common/common_manifest_handlers.h"
 #include "extensions/common/constants.h"
 #include "extensions/common/core_extensions_api_provider.h"
 #include "extensions/common/extension.h"
@@ -32,7 +30,6 @@
 #include "extensions/common/extension_urls.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/url_pattern.h"
@@ -93,14 +90,6 @@ ChromeExtensionsClient::~ChromeExtensionsClient() {
 void ChromeExtensionsClient::Initialize() {
   SCOPED_UMA_HISTOGRAM_TIMER("Extensions.ChromeExtensionsClientInitTime");
 
-  // Registration could already be finalized in unit tests, where the utility
-  // thread runs in-process.
-  if (!ManifestHandler::IsRegistrationFinalized()) {
-    RegisterCommonManifestHandlers();
-    RegisterChromeManifestHandlers();
-    ManifestHandler::FinalizeRegistration();
-  }
-
   // 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

chromecast/common/cast_extensions_api_provider.cc

@@ -4,6 +4,7 @@
 
 #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 "chromecast/common/extensions_api/cast_api_features.h"
 #include "chromecast/common/extensions_api/cast_api_permissions.h"
@@ -11,6 +12,9 @@
 #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/manifest_handler.h"
+#include "extensions/common/manifest_handlers/automation.h"
+#include "extensions/common/manifest_handlers/content_scripts_handler.h"
 #include "extensions/common/permissions/permissions_info.h"
 #include "extensions/shell/grit/app_shell_resources.h"
 
@@ -55,4 +59,10 @@ void CastExtensionsAPIProvider::AddPermissionsProviders(
   permissions_info->AddProvider(api_permissions_, GetCastPermissionAliases());
 }
 
+void CastExtensionsAPIProvider::RegisterManifestHandlers() {
+  (new AutomationHandler)->Register();  // TODO(crbug/837773) De-dupe later.
+  (new chromecast::CastRedirectHandler)->Register();
+  (new ContentScriptsHandler)->Register();
+}
+
 }  // namespace extensions

chromecast/common/cast_extensions_api_provider.h

@@ -25,6 +25,7 @@ class CastExtensionsAPIProvider : public ExtensionsAPIProvider {
   bool IsAPISchemaGenerated(const std::string& name) override;
   base::StringPiece GetAPISchema(const std::string& name) override;
   void AddPermissionsProviders(PermissionsInfo* permissions_info) override;
+  void RegisterManifestHandlers() override;
 
  private:
   const CastAPIPermissions api_permissions_;

chromecast/common/cast_extensions_client.cc

@@ -11,7 +11,6 @@
 #include "base/macros.h"
 #include "base/no_destructor.h"
 #include "chromecast/common/cast_extensions_api_provider.h"
-#include "chromecast/common/cast_redirect_manifest_handler.h"
 #include "components/version_info/version_info.h"
 #include "content/public/common/user_agent.h"
 #include "extensions/common/api/api_features.h"
@@ -19,15 +18,11 @@
 #include "extensions/common/api/generated_schemas.h"
 #include "extensions/common/api/manifest_features.h"
 #include "extensions/common/api/permission_features.h"
-#include "extensions/common/common_manifest_handlers.h"
 #include "extensions/common/core_extensions_api_provider.h"
 #include "extensions/common/extension_urls.h"
 #include "extensions/common/features/feature_provider.h"
 #include "extensions/common/features/manifest_feature.h"
 #include "extensions/common/features/simple_feature.h"
-#include "extensions/common/manifest_handler.h"
-#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/url_pattern_set.h"
 
@@ -35,13 +30,6 @@ namespace extensions {
 
 namespace {
 
-void RegisterCastManifestHandlers() {
-  DCHECK(!ManifestHandler::IsRegistrationFinalized());
-  (new AutomationHandler)->Register();  // TODO(crbug/837773) De-dupe later.
-  (new chromecast::CastRedirectHandler)->Register();
-  (new ContentScriptsHandler)->Register();
-}
-
 // TODO(jamescook): Refactor ChromePermissionsMessageProvider so we can share
 // code. For now, this implementation does nothing.
 class ShellPermissionMessageProvider : public PermissionMessageProvider {
@@ -85,9 +73,6 @@ CastExtensionsClient::CastExtensionsClient()
 CastExtensionsClient::~CastExtensionsClient() {}
 
 void CastExtensionsClient::Initialize() {
-  RegisterCommonManifestHandlers();
-  RegisterCastManifestHandlers();
-  ManifestHandler::FinalizeRegistration();
   // TODO(jamescook): Do we need to whitelist any extensions?
 }
 

extensions/common/core_extensions_api_provider.cc

@@ -9,6 +9,7 @@
 #include "extensions/common/api/generated_schemas.h"
 #include "extensions/common/api/manifest_features.h"
 #include "extensions/common/api/permission_features.h"
+#include "extensions/common/common_manifest_handlers.h"
 #include "extensions/common/extensions_aliases.h"
 #include "extensions/common/features/json_feature_provider_source.h"
 #include "extensions/common/permissions/permissions_info.h"
@@ -56,4 +57,8 @@ void CoreExtensionsAPIProvider::AddPermissionsProviders(
                                 GetExtensionsPermissionAliases());
 }
 
+void CoreExtensionsAPIProvider::RegisterManifestHandlers() {
+  RegisterCommonManifestHandlers();
+}
+
 }  // namespace extensions

extensions/common/core_extensions_api_provider.h

@@ -25,6 +25,7 @@ class CoreExtensionsAPIProvider : public ExtensionsAPIProvider {
   bool IsAPISchemaGenerated(const std::string& name) override;
   base::StringPiece GetAPISchema(const std::string& name) override;
   void AddPermissionsProviders(PermissionsInfo* permissions_info) override;
+  void RegisterManifestHandlers() override;
 
  private:
   const ExtensionsAPIPermissions api_permissions_;

extensions/common/extensions_api_provider.h

@@ -43,6 +43,9 @@ class ExtensionsAPIProvider {
   // Adds any associated permissions.
   virtual void AddPermissionsProviders(PermissionsInfo* permissions_info) = 0;
 
+  // Registers manifest handlers for any associated API features.
+  virtual void RegisterManifestHandlers() = 0;
+
  private:
   DISALLOW_COPY_AND_ASSIGN(ExtensionsAPIProvider);
 };

extensions/common/extensions_client.cc

@@ -9,6 +9,7 @@
 #include "extensions/common/extensions_api_provider.h"
 #include "extensions/common/features/feature_provider.h"
 #include "extensions/common/features/json_feature_provider_source.h"
+#include "extensions/common/manifest_handler.h"
 #include "extensions/common/manifest_handlers/icons_handler.h"
 #include "extensions/common/permissions/permissions_info.h"
 
@@ -109,9 +110,13 @@ std::string ExtensionsClient::GetUserAgent() const {
 void ExtensionsClient::DoInitialize() {
   initialize_called_ = true;
 
+  DCHECK(!ManifestHandler::IsRegistrationFinalized());
   PermissionsInfo* permissions_info = PermissionsInfo::GetInstance();
-  for (const auto& provider : api_providers_)
+  for (const auto& provider : api_providers_) {
+    provider->RegisterManifestHandlers();
     provider->AddPermissionsProviders(permissions_info);
+  }
+  ManifestHandler::FinalizeRegistration();
 
   Initialize();
 }

extensions/shell/common/shell_extensions_api_provider.cc

@@ -50,4 +50,6 @@ base::StringPiece ShellExtensionsAPIProvider::GetAPISchema(
 void ShellExtensionsAPIProvider::AddPermissionsProviders(
     PermissionsInfo* permissions_info) {}
 
+void ShellExtensionsAPIProvider::RegisterManifestHandlers() {}
+
 }  // namespace extensions

extensions/shell/common/shell_extensions_api_provider.h

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

extensions/shell/common/shell_extensions_client.cc

@@ -12,11 +12,9 @@
 #include "base/macros.h"
 #include "components/version_info/version_info.h"
 #include "content/public/common/user_agent.h"
-#include "extensions/common/common_manifest_handlers.h"
 #include "extensions/common/core_extensions_api_provider.h"
 #include "extensions/common/extension_urls.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_provider.h"
 #include "extensions/common/url_pattern_set.h"
@@ -73,8 +71,6 @@ ShellExtensionsClient::~ShellExtensionsClient() {
 }
 
 void ShellExtensionsClient::Initialize() {
-  RegisterCommonManifestHandlers();
-  ManifestHandler::FinalizeRegistration();
   // TODO(jamescook): Do we need to whitelist any extensions?
 }
 

extensions/test/test_extensions_client.cc

@@ -11,10 +11,8 @@
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/stl_util.h"
-#include "extensions/common/common_manifest_handlers.h"
 #include "extensions/common/core_extensions_api_provider.h"
 #include "extensions/common/extension_urls.h"
-#include "extensions/common/manifest_handler.h"
 #include "extensions/common/url_pattern_set.h"
 #include "extensions/grit/extensions_resources.h"
 #include "extensions/test/test_permission_message_provider.h"
@@ -41,12 +39,6 @@ void TestExtensionsClient::RemoveBrowserImagePathsFilter(
 }
 
 void TestExtensionsClient::Initialize() {
-  // Registration could already be finalized in unit tests, where the utility
-  // thread runs in-process.
-  if (!ManifestHandler::IsRegistrationFinalized()) {
-    RegisterCommonManifestHandlers();
-    ManifestHandler::FinalizeRegistration();
-  }
 }
 
 void TestExtensionsClient::InitializeWebStoreUrls(