[Extensions] Clean up API Permission Registration

Clean up API Permission registration to happen directly in the
ExtensionAPIProviders, rather than through a separate
PermissionsProvider class (though the permissions are still defined in
the old file).  Permissions are now registered by passing a base::span
of APIPermissionInfo::InitInfos to the PermissionsInfo. Also combine
alias definition in the same file.
This has a number of advantages:
- Less code (getting rid of PermissionsProvider, which didn't really add
  much, and get rid of the separate aliases file).
- Avoid constructing a vector or each call to
  PermissionsProvider::GetAllPermissions(). base::spans are cheaper to
  construct.
- Make all the c-style arrays of APIPermissionInfo::InitInfos static.
  Avoid reconstructing them. (In practice, this doesn't matter, since
  these methods should only be called once per process, but it makes it
  guaranteed should anything change.)

This might have a slight improvement on ExtensionsClient init time
(since we save on vector construction), but mostly just to simplify
things.

Bug: 847237

Change-Id: I4e161f899a0b3b9ac816c130222db6ea462ddcc3
Reviewed-on: https://chromium-review.googlesource.com/1225211
Reviewed-by: Luke Halliwell <halliwell@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#591875}

Author: rdcronin

chrome/browser/extensions/permissions_based_management_policy_provider_unittest.cc

@@ -36,8 +36,6 @@ class PermissionsBasedManagementPolicyProviderTest : public testing::Test {
         provider_(settings_.get()) {}
 
   void SetUp() override {
-    ChromeAPIPermissions api_permissions;
-    perm_list_ = api_permissions.GetAllPermissions();
     pref_service_->registry()->RegisterDictionaryPref(
         pref_names::kExtensionManagement);
   }
@@ -47,9 +45,9 @@ class PermissionsBasedManagementPolicyProviderTest : public testing::Test {
   // Get API permissions name for |id|, we cannot use arbitrary strings since
   // they will be ignored by ExtensionManagementService.
   std::string GetAPIPermissionName(APIPermission::ID id) {
-    for (const auto& perm : perm_list_) {
-      if (perm->id() == id)
-        return perm->name();
+    for (const auto& perm : chrome_api_permissions::GetPermissionInfos()) {
+      if (perm.id == id)
+        return perm.name;
     }
     ADD_FAILURE() << "Permission not found: " << id;
     return std::string();
@@ -81,8 +79,6 @@ class PermissionsBasedManagementPolicyProviderTest : public testing::Test {
   }
 
  protected:
-  std::vector<std::unique_ptr<APIPermissionInfo>> perm_list_;
-
   std::unique_ptr<TestingPrefServiceSimple> pref_service_;
   std::unique_ptr<ExtensionManagement> settings_;
 

chrome/common/BUILD.gn

@@ -324,8 +324,6 @@ static_library("common") {
       "extensions/api/url_handlers/url_handlers_parser.h",
       "extensions/api/webstore/webstore_api_constants.cc",
       "extensions/api/webstore/webstore_api_constants.h",
-      "extensions/chrome_aliases.cc",
-      "extensions/chrome_aliases.h",
       "extensions/chrome_extension_messages.h",
       "extensions/chrome_extensions_api_provider.cc",
       "extensions/chrome_extensions_api_provider.h",

chrome/common/apps/platform_apps/chrome_apps_api_permissions.cc

@@ -4,38 +4,21 @@
 
 #include "chrome/common/apps/platform_apps/chrome_apps_api_permissions.h"
 
-#include "base/macros.h"
-#include "base/memory/ptr_util.h"
-#include "base/stl_util.h"
-#include "extensions/common/permissions/api_permission.h"
+namespace chrome_apps_api_permissions {
+namespace {
 
-namespace apps {
+// WARNING: If you are modifying a permission message in this list, be sure to
+// add the corresponding permission message rule to
+// ChromePermissionMessageProvider::GetPermissionMessages as well.
+constexpr extensions::APIPermissionInfo::InitInfo permissions_to_register[] = {
+    {extensions::APIPermission::kBrowser, "browser"},
+    {extensions::APIPermission::kEasyUnlockPrivate, "easyUnlockPrivate"},
+};
 
-ChromeAppsAPIPermissions::ChromeAppsAPIPermissions() = default;
-ChromeAppsAPIPermissions::~ChromeAppsAPIPermissions() = default;
+}  // namespace
 
-std::vector<std::unique_ptr<extensions::APIPermissionInfo>>
-ChromeAppsAPIPermissions::GetAllPermissions() const {
-  // WARNING: If you are modifying a permission message in this list, be sure to
-  // add the corresponding permission message rule to
-  // ChromePermissionMessageProvider::GetPermissionMessages as well.
-  static constexpr extensions::APIPermissionInfo::InitInfo
-      permissions_to_register[] = {
-          {extensions::APIPermission::kBrowser, "browser"},
-          {extensions::APIPermission::kEasyUnlockPrivate, "easyUnlockPrivate"},
-      };
-
-  std::vector<std::unique_ptr<extensions::APIPermissionInfo>> permissions;
-  permissions.reserve(base::size(permissions_to_register));
-
-  for (const auto& permission : permissions_to_register) {
-    // NOTE: Using base::WrapUnique() because APIPermissionsInfo ctor is
-    // private.
-    permissions.push_back(
-        base::WrapUnique(new extensions::APIPermissionInfo(permission)));
-  }
-
-  return permissions;
+base::span<const extensions::APIPermissionInfo::InitInfo> GetPermissionInfos() {
+  return base::make_span(permissions_to_register);
 }
 
-}  // namespace apps
+}  // namespace chrome_apps_api_permissions

chrome/common/apps/platform_apps/chrome_apps_api_permissions.h

@@ -5,28 +5,15 @@
 #ifndef CHROME_COMMON_APPS_PLATFORM_APPS_CHROME_APPS_API_PERMISSIONS_H_
 #define CHROME_COMMON_APPS_PLATFORM_APPS_CHROME_APPS_API_PERMISSIONS_H_
 
-#include <memory>
-#include <vector>
+#include "base/containers/span.h"
+#include "extensions/common/permissions/api_permission.h"
 
-#include "base/macros.h"
-#include "extensions/common/permissions/permissions_provider.h"
+namespace chrome_apps_api_permissions {
 
-namespace apps {
+// Returns the information necessary to construct Chrome app-specific
+// APIPermissions.
+base::span<const extensions::APIPermissionInfo::InitInfo> GetPermissionInfos();
 
-// A PermissionsProvider responsible for Chrome App API Permissions.
-class ChromeAppsAPIPermissions : public extensions::PermissionsProvider {
- public:
-  ChromeAppsAPIPermissions();
-  ~ChromeAppsAPIPermissions() override;
-
-  // extensions::PermissionsProvider:
-  std::vector<std::unique_ptr<extensions::APIPermissionInfo>>
-  GetAllPermissions() const override;
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(ChromeAppsAPIPermissions);
-};
-
-}  // namespace apps
+}  // namespace chrome_apps_api_permissions
 
 #endif  // CHROME_COMMON_APPS_PLATFORM_APPS_CHROME_APPS_API_PERMISSIONS_H_

chrome/common/apps/platform_apps/chrome_apps_api_provider.cc

@@ -7,6 +7,7 @@
 #include "chrome/common/apps/platform_apps/api/api_features.h"
 #include "chrome/common/apps/platform_apps/api/generated_schemas.h"
 #include "chrome/common/apps/platform_apps/api/permission_features.h"
+#include "chrome/common/apps/platform_apps/chrome_apps_api_permissions.h"
 #include "chrome/grit/common_resources.h"
 #include "extensions/common/alias.h"
 #include "extensions/common/features/json_feature_provider_source.h"
@@ -50,9 +51,10 @@ base::StringPiece ChromeAppsAPIProvider::GetAPISchema(const std::string& name) {
   return api::ChromeAppsGeneratedSchemas::Get(name);
 }
 
-void ChromeAppsAPIProvider::AddPermissionsProviders(
+void ChromeAppsAPIProvider::RegisterPermissions(
     extensions::PermissionsInfo* permissions_info) {
-  permissions_info->AddProvider(api_permissions_, {});
+  permissions_info->RegisterPermissions(
+      chrome_apps_api_permissions::GetPermissionInfos(), {});
 }
 
 void ChromeAppsAPIProvider::RegisterManifestHandlers() {

chrome/common/apps/platform_apps/chrome_apps_api_provider.h

@@ -5,7 +5,6 @@
 #ifndef CHROME_COMMON_APPS_PLATFORM_APPS_CHROME_APPS_API_PROVIDER_H_
 #define CHROME_COMMON_APPS_PLATFORM_APPS_CHROME_APPS_API_PROVIDER_H_
 
-#include "chrome/common/apps/platform_apps/chrome_apps_api_permissions.h"
 #include "extensions/common/extensions_api_provider.h"
 
 namespace apps {
@@ -24,13 +23,11 @@ class ChromeAppsAPIProvider : public extensions::ExtensionsAPIProvider {
       extensions::JSONFeatureProviderSource* json_source) override;
   bool IsAPISchemaGenerated(const std::string& name) override;
   base::StringPiece GetAPISchema(const std::string& name) override;
-  void AddPermissionsProviders(
+  void RegisterPermissions(
       extensions::PermissionsInfo* permissions_info) override;
   void RegisterManifestHandlers() override;
 
  private:
-  const ChromeAppsAPIPermissions api_permissions_;
-
   DISALLOW_COPY_AND_ASSIGN(ChromeAppsAPIProvider);
 };
 

chrome/common/extensions/chrome_aliases.cc

@@ -8,8 +8,8 @@
 #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/common/extensions/chrome_manifest_handlers.h"
+#include "chrome/common/extensions/permissions/chrome_api_permissions.h"
 #include "chrome/grit/common_resources.h"
 #include "extensions/common/features/json_feature_provider_source.h"
 #include "extensions/common/permissions/permissions_info.h"
@@ -53,9 +53,11 @@ base::StringPiece ChromeExtensionsAPIProvider::GetAPISchema(
   return api::ChromeGeneratedSchemas::Get(name);
 }
 
-void ChromeExtensionsAPIProvider::AddPermissionsProviders(
+void ChromeExtensionsAPIProvider::RegisterPermissions(
     PermissionsInfo* permissions_info) {
-  permissions_info->AddProvider(api_permissions_, GetChromePermissionAliases());
+  permissions_info->RegisterPermissions(
+      chrome_api_permissions::GetPermissionInfos(),
+      chrome_api_permissions::GetPermissionAliases());
 }
 
 void ChromeExtensionsAPIProvider::RegisterManifestHandlers() {

chrome/common/extensions/chrome_aliases.h

@@ -6,7 +6,6 @@
 #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 {
@@ -24,12 +23,10 @@ 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;
+  void RegisterPermissions(PermissionsInfo* permissions_info) override;
   void RegisterManifestHandlers() override;
 
  private:
-  const ChromeAPIPermissions api_permissions_;
-
   DISALLOW_COPY_AND_ASSIGN(ChromeExtensionsAPIProvider);
 };