Commit 9ee032f8 authored by Devlin Cronin's avatar Devlin Cronin Committed by Commit Bot

[Extensions] Allow multiple feature registration methods

Currently, we have a single generated class that registers all the
extension features in the constructor. For instance,
ChromeAPIFeatureProvider registers both features from both //chrome
and //extensions.

Refactor this to generate a method that takes a FeatureProvider and adds
features to it, rather than generating a FeatureProvider subclass. This
allows us to have multiple feature registration methods called for a
single provider, rather than needing a single class per build.

This is necessary in order to split out app and extension feature
registration.

Bug: 862310

Change-Id: I9fba5ea57d8b14ca9f893a2694661163c8bc4140
Reviewed-on: https://chromium-review.googlesource.com/1132124Reviewed-by: default avatarStephen Lanham <slan@chromium.org>
Reviewed-by: default avatarAlbert Chaulk <achaulk@chromium.org>
Reviewed-by: default avatarIstiaque Ahmed <lazyboy@chromium.org>
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575018}
parent 4eccb972
......@@ -11,7 +11,7 @@ assert(enable_extensions)
json_features("extension_features_unittest") {
feature_type = "APIFeature"
provider_class = "UnittestFeatureProvider"
method_name = "AddUnittestAPIFeatures"
sources = [
"//chrome/test/data/extensions/extension_api_unittest/api_features.json",
]
......
......@@ -225,44 +225,37 @@ json_schema_api("api_registration") {
json_features("api_features") {
feature_type = "APIFeature"
provider_class = "APIFeatureProvider"
method_name = "AddChromeAPIFeatures"
sources = [
"../../../../extensions/common/api/_api_features.json",
"_api_features.json",
]
}
json_features("permission_features") {
feature_type = "PermissionFeature"
provider_class = "PermissionFeatureProvider"
method_name = "AddChromePermissionFeatures"
sources = [
"../../../../extensions/common/api/_permission_features.json",
"_permission_features.json",
]
}
json_features("manifest_features") {
feature_type = "ManifestFeature"
provider_class = "ManifestFeatureProvider"
method_name = "AddChromeManifestFeatures"
sources = [
"../../../../extensions/common/api/_manifest_features.json",
"_manifest_features.json",
]
}
json_features("behavior_features") {
feature_type = "BehaviorFeature"
provider_class = "BehaviorFeatureProvider"
sources = [
"../../../../extensions/common/api/_behavior_features.json",
]
}
group("extensions_features") {
public_deps = [
":api_features",
":behavior_features",
":manifest_features",
":permission_features",
# TODO(devlin): It would be nicer to have this dependency hoisted up to
# //extensions/common (since that's where it's consumed), but there's some
# cycles to be resolved first.
"//extensions/common/api:extensions_features",
]
}
......@@ -192,7 +192,8 @@ TEST(ExtensionAPITest, APIFeatures) {
GURL() }
};
UnittestFeatureProvider api_feature_provider;
FeatureProvider api_feature_provider;
AddUnittestAPIFeatures(&api_feature_provider);
for (size_t i = 0; i < arraysize(test_data); ++i) {
TestExtensionAPI api;
......@@ -215,7 +216,8 @@ TEST(ExtensionAPITest, APIFeatures) {
}
TEST(ExtensionAPITest, APIFeaturesAlias) {
UnittestFeatureProvider api_feature_provider;
FeatureProvider api_feature_provider;
AddUnittestAPIFeatures(&api_feature_provider);
TestExtensionAPI api;
api.RegisterDependencyProvider("api", &api_feature_provider);
......@@ -315,7 +317,8 @@ TEST(ExtensionAPITest, IsAnyFeatureAvailableToContext) {
{"test7", false, Feature::WEB_PAGE_CONTEXT, nullptr,
GURL("http://bar.com")}};
UnittestFeatureProvider api_feature_provider;
FeatureProvider api_feature_provider;
AddUnittestAPIFeatures(&api_feature_provider);
for (size_t i = 0; i < arraysize(test_data); ++i) {
TestExtensionAPI api;
......@@ -375,7 +378,8 @@ TEST(ExtensionAPITest, SessionTypeFeature) {
{"test6.foo", true, FeatureSessionType::REGULAR},
{"test6.foo", true, FeatureSessionType::UNKNOWN}});
UnittestFeatureProvider api_feature_provider;
FeatureProvider api_feature_provider;
AddUnittestAPIFeatures(&api_feature_provider);
for (const auto& test : kTestData) {
TestExtensionAPI api;
......
......@@ -16,7 +16,6 @@
#include "chrome/common/chrome_content_client.h"
#include "chrome/common/chrome_switches.h"
#include "chrome/common/extensions/api/api_features.h"
#include "chrome/common/extensions/api/behavior_features.h"
#include "chrome/common/extensions/api/extension_action/action_info.h"
#include "chrome/common/extensions/api/generated_schemas.h"
#include "chrome/common/extensions/api/manifest_features.h"
......@@ -29,7 +28,11 @@
#include "chrome/grit/common_resources.h"
#include "components/version_info/version_info.h"
#include "content/public/common/url_constants.h"
#include "extensions/common/api/api_features.h"
#include "extensions/common/api/behavior_features.h"
#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/constants.h"
#include "extensions/common/extension.h"
......@@ -153,15 +156,19 @@ const std::string ChromeExtensionsClient::GetProductName() {
std::unique_ptr<FeatureProvider> ChromeExtensionsClient::CreateFeatureProvider(
const std::string& name) const {
std::unique_ptr<FeatureProvider> provider;
auto provider = std::make_unique<FeatureProvider>();
if (name == "api") {
provider.reset(new APIFeatureProvider());
AddCoreAPIFeatures(provider.get());
AddChromeAPIFeatures(provider.get());
} else if (name == "manifest") {
provider.reset(new ManifestFeatureProvider());
AddCoreManifestFeatures(provider.get());
AddChromeManifestFeatures(provider.get());
} else if (name == "permission") {
provider.reset(new PermissionFeatureProvider());
AddCorePermissionFeatures(provider.get());
AddChromePermissionFeatures(provider.get());
} else if (name == "behavior") {
provider.reset(new BehaviorFeatureProvider());
// Note: There are no chrome-specific behavior features.
AddCoreBehaviorFeatures(provider.get());
} else {
NOTREACHED();
}
......
......@@ -14,16 +14,20 @@
#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_behavior_features.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 "components/version_info/version_info.h"
#include "content/public/common/user_agent.h"
#include "extensions/common/api/api_features.h"
#include "extensions/common/api/behavior_features.h"
#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/extension_urls.h"
#include "extensions/common/extensions_aliases.h"
#include "extensions/common/features/feature_provider.h"
#include "extensions/common/features/json_feature_provider_source.h"
#include "extensions/common/features/manifest_feature.h"
#include "extensions/common/features/simple_feature.h"
......@@ -117,15 +121,19 @@ const std::string CastExtensionsClient::GetProductName() {
std::unique_ptr<FeatureProvider> CastExtensionsClient::CreateFeatureProvider(
const std::string& name) const {
std::unique_ptr<FeatureProvider> provider;
auto provider = std::make_unique<FeatureProvider>();
if (name == "api") {
provider = std::make_unique<CastAPIFeatureProvider>();
AddCoreAPIFeatures(provider.get());
AddCastAPIFeatures(provider.get());
} else if (name == "manifest") {
provider = std::make_unique<CastManifestFeatureProvider>();
AddCoreManifestFeatures(provider.get());
AddCastManifestFeatures(provider.get());
} else if (name == "permission") {
provider = std::make_unique<CastPermissionFeatureProvider>();
AddCorePermissionFeatures(provider.get());
AddCastPermissionFeatures(provider.get());
} else if (name == "behavior") {
provider = std::make_unique<CastBehaviorFeatureProvider>();
// Note: There are no cast-specific behavior features.
AddCoreBehaviorFeatures(provider.get());
} else {
NOTREACHED();
}
......
......@@ -60,44 +60,33 @@ json_schema_api("api_registration") {
json_features("cast_api_features") {
feature_type = "APIFeature"
provider_class = "CastAPIFeatureProvider"
method_name = "AddCastAPIFeatures"
sources = [
"../../../extensions/common/api/_api_features.json",
"_api_features.json",
]
}
json_features("cast_permission_features") {
feature_type = "PermissionFeature"
provider_class = "CastPermissionFeatureProvider"
method_name = "AddCastPermissionFeatures"
sources = [
"../../../extensions/common/api/_permission_features.json",
"_permission_features.json",
]
}
json_features("cast_manifest_features") {
feature_type = "ManifestFeature"
provider_class = "CastManifestFeatureProvider"
method_name = "AddCastManifestFeatures"
sources = [
"../../../extensions/common/api/_manifest_features.json",
"_manifest_features.json",
]
}
json_features("cast_behavior_features") {
feature_type = "BehaviorFeature"
provider_class = "CastBehaviorFeatureProvider"
sources = [
"../../../extensions/common/api/_behavior_features.json",
]
}
group("extensions_features") {
public_deps = [
":cast_api_features",
":cast_behavior_features",
":cast_manifest_features",
":cast_permission_features",
"//extensions/common/api:extensions_features",
]
}
......@@ -148,7 +148,7 @@ jumbo_static_library("test_support") {
"//extensions/browser/api:api_registration",
"//extensions/common",
"//extensions/common/api",
"//extensions/test:extensions_features",
"//extensions/common/api:extensions_features",
"//net:test_support",
"//testing/gmock",
"//testing/gtest",
......
......@@ -6,6 +6,7 @@ import("//build/config/features.gni")
import("//extensions/common/api/schema.gni")
import("//extensions/buildflags/buildflags.gni")
import("//mojo/public/tools/bindings/mojom.gni")
import("//tools/json_schema_compiler/json_features.gni")
import("//tools/json_schema_compiler/json_schema_api.gni")
assert(enable_extensions)
......@@ -38,3 +39,44 @@ group("api") {
"//extensions/buildflags",
]
}
json_features("api_features") {
feature_type = "APIFeature"
method_name = "AddCoreAPIFeatures"
sources = [
"_api_features.json",
]
}
json_features("permission_features") {
feature_type = "PermissionFeature"
method_name = "AddCorePermissionFeatures"
sources = [
"_permission_features.json",
]
}
json_features("manifest_features") {
feature_type = "ManifestFeature"
method_name = "AddCoreManifestFeatures"
sources = [
"_manifest_features.json",
]
}
json_features("behavior_features") {
feature_type = "BehaviorFeature"
method_name = "AddCoreBehaviorFeatures"
sources = [
"_behavior_features.json",
]
}
group("extensions_features") {
public_deps = [
":api_features",
":behavior_features",
":manifest_features",
":permission_features",
]
}
......@@ -33,42 +33,15 @@ json_schema_api("api_registration") {
json_features("shell_api_features") {
feature_type = "APIFeature"
provider_class = "ShellAPIFeatureProvider"
method_name = "AddShellAPIFeatures"
sources = [
"../../../common/api/_api_features.json",
"_api_features.json",
]
}
json_features("shell_permission_features") {
feature_type = "PermissionFeature"
provider_class = "ShellPermissionFeatureProvider"
sources = [
"../../../common/api/_permission_features.json",
]
}
json_features("shell_manifest_features") {
feature_type = "ManifestFeature"
provider_class = "ShellManifestFeatureProvider"
sources = [
"../../../common/api/_manifest_features.json",
]
}
json_features("shell_behavior_features") {
feature_type = "BehaviorFeature"
provider_class = "ShellBehaviorFeatureProvider"
sources = [
"../../../common/api/_behavior_features.json",
]
}
group("extensions_features") {
public_deps = [
":shell_api_features",
":shell_behavior_features",
":shell_manifest_features",
":shell_permission_features",
"//extensions/common/api:extensions_features",
]
}
......@@ -12,10 +12,15 @@
#include "base/macros.h"
#include "components/version_info/version_info.h"
#include "content/public/common/user_agent.h"
#include "extensions/common/api/api_features.h"
#include "extensions/common/api/behavior_features.h"
#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/extension_urls.h"
#include "extensions/common/extensions_aliases.h"
#include "extensions/common/features/feature_provider.h"
#include "extensions/common/features/json_feature_provider_source.h"
#include "extensions/common/features/simple_feature.h"
#include "extensions/common/manifest_handler.h"
......@@ -26,9 +31,6 @@
#include "extensions/grit/extensions_resources.h"
#include "extensions/shell/common/api/generated_schemas.h"
#include "extensions/shell/common/api/shell_api_features.h"
#include "extensions/shell/common/api/shell_behavior_features.h"
#include "extensions/shell/common/api/shell_manifest_features.h"
#include "extensions/shell/common/api/shell_permission_features.h"
#include "extensions/shell/grit/app_shell_resources.h"
namespace extensions {
......@@ -103,15 +105,16 @@ const std::string ShellExtensionsClient::GetProductName() {
std::unique_ptr<FeatureProvider> ShellExtensionsClient::CreateFeatureProvider(
const std::string& name) const {
std::unique_ptr<FeatureProvider> provider;
auto provider = std::make_unique<FeatureProvider>();
if (name == "api") {
provider.reset(new ShellAPIFeatureProvider());
AddCoreAPIFeatures(provider.get());
AddShellAPIFeatures(provider.get());
} else if (name == "manifest") {
provider.reset(new ShellManifestFeatureProvider());
AddCoreManifestFeatures(provider.get());
} else if (name == "permission") {
provider.reset(new ShellPermissionFeatureProvider());
AddCorePermissionFeatures(provider.get());
} else if (name == "behavior") {
provider.reset(new ShellBehaviorFeatureProvider());
AddCoreBehaviorFeatures(provider.get());
} else {
NOTREACHED();
}
......
# Copyright 2016 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.
import("//extensions/buildflags/buildflags.gni")
import("//tools/json_schema_compiler/json_features.gni")
assert(enable_extensions)
json_features("test_api_features") {
feature_type = "APIFeature"
provider_class = "TestAPIFeatureProvider"
sources = [
"../common/api/_api_features.json",
]
}
json_features("test_permission_features") {
feature_type = "PermissionFeature"
provider_class = "TestPermissionFeatureProvider"
sources = [
"../common/api/_permission_features.json",
]
}
json_features("test_manifest_features") {
feature_type = "ManifestFeature"
provider_class = "TestManifestFeatureProvider"
sources = [
"../common/api/_manifest_features.json",
]
}
json_features("test_behavior_features") {
feature_type = "BehaviorFeature"
provider_class = "TestBehaviorFeatureProvider"
sources = [
"../common/api/_behavior_features.json",
]
}
group("extensions_features") {
public_deps = [
":test_api_features",
":test_behavior_features",
":test_manifest_features",
":test_permission_features",
]
}
......@@ -11,7 +11,11 @@
#include "base/command_line.h"
#include "base/files/file_path.h"
#include "base/stl_util.h"
#include "extensions/common/api/api_features.h"
#include "extensions/common/api/behavior_features.h"
#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/extension_urls.h"
#include "extensions/common/extensions_aliases.h"
......@@ -22,10 +26,6 @@
#include "extensions/common/permissions/permissions_info.h"
#include "extensions/common/url_pattern_set.h"
#include "extensions/grit/extensions_resources.h"
#include "extensions/test/test_api_features.h"
#include "extensions/test/test_behavior_features.h"
#include "extensions/test/test_manifest_features.h"
#include "extensions/test/test_permission_features.h"
#include "extensions/test/test_permission_message_provider.h"
namespace extensions {
......@@ -83,15 +83,15 @@ const std::string TestExtensionsClient::GetProductName() {
std::unique_ptr<FeatureProvider> TestExtensionsClient::CreateFeatureProvider(
const std::string& name) const {
std::unique_ptr<FeatureProvider> provider;
auto provider = std::make_unique<FeatureProvider>();
if (name == "api") {
provider.reset(new TestAPIFeatureProvider());
AddCoreAPIFeatures(provider.get());
} else if (name == "manifest") {
provider.reset(new TestManifestFeatureProvider());
AddCoreManifestFeatures(provider.get());
} else if (name == "permission") {
provider.reset(new TestPermissionFeatureProvider());
AddCorePermissionFeatures(provider.get());
} else if (name == "behavior") {
provider.reset(new TestBehaviorFeatureProvider());
AddCoreBehaviorFeatures(provider.get());
} else {
NOTREACHED();
}
......
......@@ -25,18 +25,10 @@ HEADER_FILE_TEMPLATE = """
#ifndef %(header_guard)s
#define %(header_guard)s
#include "extensions/common/features/feature_provider.h"
namespace extensions {
class FeatureProvider;
class %(provider_class)s : public FeatureProvider {
public:
%(provider_class)s();
~%(provider_class)s() override;
private:
DISALLOW_COPY_AND_ASSIGN(%(provider_class)s);
};
void %(method_name)s(FeatureProvider* provider);
} // namespace extensions
......@@ -56,16 +48,18 @@ CC_FILE_BEGIN = """
#include "%(header_file_path)s"
#include "extensions/common/features/complex_feature.h"
#include "extensions/common/features/feature_provider.h"
#include "extensions/common/features/manifest_feature.h"
#include "extensions/common/features/permission_feature.h"
namespace extensions {
void %(method_name)s(FeatureProvider* provider) {
"""
# The end of the .cc file for the generated FeatureProvider.
CC_FILE_END = """
%(provider_class)s::~%(provider_class)s() {}
}
} // namespace extensions
"""
......@@ -626,12 +620,12 @@ class FeatureCompiler(object):
"""A compiler to load, parse, and generate C++ code for a number of
features.json files."""
def __init__(self, chrome_root, source_files, feature_type,
provider_class, out_root, out_base_filename):
method_name, out_root, out_base_filename):
# See __main__'s ArgumentParser for documentation on these properties.
self._chrome_root = chrome_root
self._source_files = source_files
self._feature_type = feature_type
self._provider_class = provider_class
self._method_name = method_name
self._out_root = out_root
self._out_base_filename = out_base_filename
......@@ -754,39 +748,45 @@ class FeatureCompiler(object):
"""Returns the Code object for the body of the .cc file, which handles the
initialization of all features."""
c = Code()
c.Append('%s::%s() {' % (self._provider_class, self._provider_class))
c.Sblock()
for k in sorted(self._features.keys()):
c.Sblock('{')
feature = self._features[k]
c.Concat(feature.GetCode(self._feature_type))
c.Append('AddFeature("%s", feature);' % k)
c.Append('provider->AddFeature("%s", feature);' % k)
c.Eblock('}')
c.Eblock('}')
c.Eblock()
return c
def Write(self):
"""Writes the output."""
header_file_path = self._out_base_filename + '.h'
cc_file_path = self._out_base_filename + '.cc'
header_file = self._out_base_filename + '.h'
cc_file = self._out_base_filename + '.cc'
include_file_root = self._out_root
GEN_DIR_PREFIX = 'gen/'
if include_file_root.startswith(GEN_DIR_PREFIX):
include_file_root = include_file_root[len(GEN_DIR_PREFIX):]
header_file_path = '%s/%s' % (include_file_root, header_file)
cc_file_path = '%s/%s' % (include_file_root, cc_file)
substitutions = ({
'header_file_path': header_file_path,
'header_guard': (header_file_path.replace('/', '_').
replace('.', '_').upper()),
'provider_class': self._provider_class,
'method_name': self._method_name,
'source_files': str(self._source_files),
'year': str(datetime.now().year)
})
if not os.path.exists(self._out_root):
os.makedirs(self._out_root)
# Write the .h file.
with open(os.path.join(self._out_root, header_file_path), 'w') as f:
with open(os.path.join(self._out_root, header_file), 'w') as f:
header_file = Code()
header_file.Append(HEADER_FILE_TEMPLATE)
header_file.Substitute(substitutions)
f.write(header_file.Render().strip())
# Write the .cc file.
with open(os.path.join(self._out_root, cc_file_path), 'w') as f:
with open(os.path.join(self._out_root, cc_file), 'w') as f:
cc_file = Code()
cc_file.Append(CC_FILE_BEGIN)
cc_file.Substitute(substitutions)
......@@ -805,8 +805,8 @@ if __name__ == '__main__':
'feature_type', type=str,
help='The name of the class to use in feature generation ' +
'(e.g. APIFeature, PermissionFeature)')
parser.add_argument('provider_class', type=str,
help='The name of the class for the feature provider')
parser.add_argument('method_name', type=str,
help='The name of the method to populate the provider')
parser.add_argument('out_root', type=str,
help='The root directory to generate the C++ files into')
parser.add_argument(
......@@ -818,7 +818,7 @@ if __name__ == '__main__':
if args.feature_type not in FEATURE_TYPES:
raise NameError('Unknown feature type: %s' % args.feature_type)
c = FeatureCompiler(args.chrome_root, args.source_files, args.feature_type,
args.provider_class, args.out_root,
args.method_name, args.out_root,
args.out_base_filename)
c.Load()
c.Compile()
......
......@@ -6,18 +6,17 @@
# The following variables are required:
# sources: The features.json files to use.
# feature_type: The type of the features to generate, e.g. APIFeature.
# provider_class: The name of the provider class to generate, e.g.
# APIFeatureProvider.
# method_name: The name of the method to generate, e.g. AddChromeAPIFeatures.
# deps/public_deps/visibility: normal meaning
template("json_features") {
assert(defined(invoker.sources),
"\"sources\" must be defined for the $target_name template.")
assert(defined(invoker.feature_type),
"\"feature_type\" must be defined for the $target_name template.")
assert(defined(invoker.provider_class),
"\"provider_class\" must be defined for the $target_name template.")
assert(defined(invoker.method_name),
"\"method_name\" must be defined for the $target_name template.")
feature_type = invoker.feature_type
provider_class = invoker.provider_class
method_name = invoker.method_name
compiler_root = "//tools/json_schema_compiler"
base_filename = target_name
......@@ -41,7 +40,7 @@ template("json_features") {
args = [
".",
"$feature_type",
"$provider_class",
"$method_name",
rebase_path(target_gen_dir, root_build_dir),
"$base_filename",
] + rebased
......
......@@ -44,7 +44,7 @@ json_schema_api("api") {
json_features("features_compiler_test") {
feature_type = "APIFeature"
provider_class = "CompilerTestFeatureProvider"
method_name = "CompilerTestAddFeaturesMethod"
sources = [
"features_test.json",
"features_test2.json",
......
......@@ -5,6 +5,7 @@
#include "base/optional.h"
#include "extensions/common/features/complex_feature.h"
#include "extensions/common/features/feature.h"
#include "extensions/common/features/feature_provider.h"
#include "extensions/common/features/simple_feature.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "tools/json_schema_compiler/test/features_compiler_test.h"
......@@ -89,7 +90,8 @@ void FeatureComparator::CompareFeature(const SimpleFeature* feature) {
}
TEST(FeaturesGenerationTest, FeaturesTest) {
CompilerTestFeatureProvider provider;
FeatureProvider provider;
CompilerTestAddFeaturesMethod(&provider);
auto GetAsSimpleFeature = [&provider](const std::string& name) {
const Feature* feature = provider.GetFeature(name);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment