Remove all runtime JSON service manifest parsing
This wipes out the last remaining usage of runtime JSON manifest parsing
in favor of using build-time generated C++ manifests. All support code
for parsing JSON manifests at runtime is removed as well.
Bug: 895616
Change-Id: I35fdc82b719de4fbedb54ba433dda4293fb0f893
Reviewed-on: https://chromium-review.googlesource.com/c/1423354
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Ken Rockot <rockot@google.com>
Cr-Commit-Position: refs/heads/master@{#625356}diff --git a/chrome/browser/BUILD.gn b/chrome/browser/BUILD.gn
index 5702e1d00..a00948e 100644
--- a/chrome/browser/BUILD.gn
+++ b/chrome/browser/BUILD.gn
@@ -4205,6 +4205,13 @@
"nacl_host/nacl_infobar_delegate.h",
]
deps += [ "//components/nacl/browser" ]
+
+ if (enable_nacl) {
+ deps += [ "//components/nacl/loader:nacl_loader_manifest" ]
+ if (is_win && target_cpu == "x86") {
+ deps += [ "//components/nacl/broker:nacl_broker_manifest" ]
+ }
+ }
}
if (enable_offline_pages) {
diff --git a/chrome/browser/browser_resources.grd b/chrome/browser/browser_resources.grd
index 359e2fb4..2ad0c5d5 100644
--- a/chrome/browser/browser_resources.grd
+++ b/chrome/browser/browser_resources.grd
@@ -675,10 +675,6 @@
<include name="IDR_IME_WINDOW_CLOSE_C" file="resources\input_ime\ime_window_close_click.png" type="BINDATA" />
<include name="IDR_IME_WINDOW_CLOSE_H" file="resources\input_ime\ime_window_close_hover.png" type="BINDATA" />
</if>
- <include name="IDR_NACL_LOADER_MANIFEST" file="../../components/nacl/loader/nacl_loader_manifest.json" type="BINDATA" />
- <if expr="is_win">
- <include name="IDR_NACL_BROKER_MANIFEST" file="../../components/nacl/broker/nacl_broker_manifest.json" type="BINDATA" />
- </if>
<if expr="is_win">
<include name="IDR_WELCOME_WIN10_DEFAULT_WEBP" file="resources\welcome\default.webp" type="BINDATA" />
<include name="IDR_WELCOME_WIN10_PIN_WEBP" file="resources\welcome\pin.webp" type="BINDATA" />
diff --git a/chrome/browser/chrome_content_browser_client.cc b/chrome/browser/chrome_content_browser_client.cc
index 14190ed..7388633 100644
--- a/chrome/browser/chrome_content_browser_client.cc
+++ b/chrome/browser/chrome_content_browser_client.cc
@@ -332,6 +332,13 @@
#include "url/gurl.h"
#include "url/origin.h"
+#if BUILDFLAG(ENABLE_NACL)
+#include "components/nacl/loader/nacl_loader_manifest.h"
+#if defined(OS_WIN) && defined(ARCH_CPU_X86)
+#include "components/nacl/broker/nacl_broker_manifest.h"
+#endif
+#endif
+
#if defined(OS_WIN)
#include "base/strings/string_tokenizer.h"
#include "chrome/browser/chrome_browser_main_win.h"
@@ -3930,17 +3937,17 @@
return base::nullopt;
}
-std::vector<content::ContentBrowserClient::ServiceManifestInfo>
+std::vector<service_manager::Manifest>
ChromeContentBrowserClient::GetExtraServiceManifests() {
- return std::vector<content::ContentBrowserClient::ServiceManifestInfo>({
+ return std::vector<service_manager::Manifest> {
+ GetChromeRendererManifest(),
#if BUILDFLAG(ENABLE_NACL)
- {nacl::kNaClLoaderServiceName, IDR_NACL_LOADER_MANIFEST},
-#if defined(OS_WIN)
- {nacl::kNaClBrokerServiceName, IDR_NACL_BROKER_MANIFEST},
+ nacl_loader::GetManifest(),
+#if defined(OS_WIN) && defined(ARCH_CPU_X86)
+ nacl_broker::GetManifest(),
#endif // defined(OS_WIN)
#endif // BUILDFLAG(ENABLE_NACL)
- {chrome::mojom::kRendererServiceName, -1, GetChromeRendererManifest()},
- });
+ };
}
std::vector<std::string> ChromeContentBrowserClient::GetStartupServices() {
diff --git a/chrome/browser/chrome_content_browser_client.h b/chrome/browser/chrome_content_browser_client.h
index ef44018..c0a8e73f 100644
--- a/chrome/browser/chrome_content_browser_client.h
+++ b/chrome/browser/chrome_content_browser_client.h
@@ -409,8 +409,7 @@
const service_manager::Identity& id) override;
base::Optional<service_manager::Manifest> GetServiceManifestOverlay(
base::StringPiece name) override;
- std::vector<content::ContentBrowserClient::ServiceManifestInfo>
- GetExtraServiceManifests() override;
+ std::vector<service_manager::Manifest> GetExtraServiceManifests() override;
std::vector<std::string> GetStartupServices() override;
void OpenURL(content::SiteInstance* site_instance,
const content::OpenURLParams& params,
diff --git a/content/browser/service_manager/service_manager_context.cc b/content/browser/service_manager/service_manager_context.cc
index 5596340..9462ede7 100644
--- a/content/browser/service_manager/service_manager_context.cc
+++ b/content/browser/service_manager/service_manager_context.cc
@@ -13,7 +13,6 @@
#include "base/command_line.h"
#include "base/deferred_sequenced_task_runner.h"
#include "base/feature_list.h"
-#include "base/json/json_reader.h"
#include "base/lazy_instance.h"
#include "base/macros.h"
#include "base/memory/scoped_refptr.h"
@@ -220,21 +219,6 @@
std::move(pid_receiver));
}
-service_manager::Manifest LoadServiceManifest(base::StringPiece service_name,
- int resource_id) {
- std::string contents =
- GetContentClient()
- ->GetDataResource(resource_id, ui::ScaleFactor::SCALE_FACTOR_NONE)
- .as_string();
- DCHECK(!contents.empty());
-
- service_manager::Manifest manifest =
- service_manager::Manifest::FromValueDeprecated(
- base::JSONReader::Read(contents));
-
- return manifest;
-}
-
class NullServiceProcessLauncherFactory
: public service_manager::ServiceProcessLauncherFactory {
public:
@@ -562,12 +546,9 @@
manifest.service_name, std::move(preloaded_files_map));
}
}
- for (const auto& info :
+ for (auto& extra_manifest :
GetContentClient()->browser()->GetExtraServiceManifests()) {
- if (info.resource_id != -1)
- manifests.push_back(LoadServiceManifest(info.name, info.resource_id));
- else
- manifests.push_back(info.manifest);
+ manifests.emplace_back(std::move(extra_manifest));
}
in_process_context_ =
new InProcessServiceManagerContext(service_manager_thread_task_runner_);
diff --git a/content/public/browser/content_browser_client.cc b/content/public/browser/content_browser_client.cc
index bce9b68..abc9829 100644
--- a/content/public/browser/content_browser_client.cc
+++ b/content/public/browser/content_browser_client.cc
@@ -680,9 +680,9 @@
return base::nullopt;
}
-std::vector<ContentBrowserClient::ServiceManifestInfo>
+std::vector<service_manager::Manifest>
ContentBrowserClient::GetExtraServiceManifests() {
- return std::vector<ContentBrowserClient::ServiceManifestInfo>();
+ return std::vector<service_manager::Manifest>();
}
std::vector<std::string> ContentBrowserClient::GetStartupServices() {
diff --git a/content/public/browser/content_browser_client.h b/content/public/browser/content_browser_client.h
index 92425b0..694796b 100644
--- a/content/public/browser/content_browser_client.h
+++ b/content/public/browser/content_browser_client.h
@@ -1027,20 +1027,9 @@
virtual base::Optional<service_manager::Manifest> GetServiceManifestOverlay(
base::StringPiece name);
- struct ServiceManifestInfo {
- // The name of the service.
- std::string name;
-
- // The resource ID of a blob of manifest JSON. Used if not -1.
- int resource_id;
-
- // A Manifest value to use if |resource_id| is -1.
- service_manager::Manifest manifest;
- };
-
// Allows the embedder to provide extra service manifests to be registered
// with the service manager context.
- virtual std::vector<ServiceManifestInfo> GetExtraServiceManifests();
+ virtual std::vector<service_manager::Manifest> GetExtraServiceManifests();
// Allows the embedder to have a list of services started after the
// in-process Service Manager has been initialized.
diff --git a/services/service_manager/public/cpp/manifest.cc b/services/service_manager/public/cpp/manifest.cc
index 464ad43..9d38fc98 100644
--- a/services/service_manager/public/cpp/manifest.cc
+++ b/services/service_manager/public/cpp/manifest.cc
@@ -59,177 +59,6 @@
Manifest::ExposedInterfaceFilterCapability::operator=(
ExposedInterfaceFilterCapability&&) = default;
-// static
-Manifest Manifest::FromValueDeprecated(std::unique_ptr<base::Value> value_ptr) {
- DCHECK(value_ptr);
- base::Value value(std::move(*value_ptr));
-
- Manifest manifest;
- if (!value.is_dict())
- return manifest;
-
- const base::Value* name_value = value.FindKey("name");
- if (name_value && name_value->is_string())
- manifest.service_name = name_value->GetString();
-
- const base::Value* display_name_value = value.FindKey("display_name");
- if (display_name_value && display_name_value->is_string())
- manifest.display_name.raw_string = display_name_value->GetString();
-
- const base::Value* sandbox_type_value = value.FindKey("sandbox_type");
- if (sandbox_type_value && sandbox_type_value->is_string()) {
- manifest.options.sandbox_type = sandbox_type_value->GetString();
- } else {
- // This is kind of weird, but the default when processing packaged service
- // manifests (which is all we care about now, in practice) is "utility". So
- // assume that if no "sandbox_type" key is present.
- manifest.options.sandbox_type = "utility";
- }
-
- const base::Value* options_value = value.FindKey("options");
- if (options_value && options_value->is_dict()) {
- const base::Value* can_connect_to_services_as_any_user_value =
- options_value->FindKey("can_connect_to_other_services_as_any_user");
- manifest.options.can_connect_to_instances_in_any_group =
- can_connect_to_services_as_any_user_value &&
- can_connect_to_services_as_any_user_value->is_bool() &&
- can_connect_to_services_as_any_user_value->GetBool();
-
- const base::Value*
- can_connect_to_other_services_with_any_instance_name_value =
- options_value->FindKey(
- "can_connect_to_other_services_with_any_instance_name");
- manifest.options.can_connect_to_instances_with_any_id =
- can_connect_to_other_services_with_any_instance_name_value &&
- can_connect_to_other_services_with_any_instance_name_value->is_bool() &&
- can_connect_to_other_services_with_any_instance_name_value->GetBool();
-
- const base::Value* can_create_other_service_instances_value =
- options_value->FindKey("can_create_other_service_instances");
- manifest.options.can_register_other_service_instances =
- can_create_other_service_instances_value &&
- can_create_other_service_instances_value->is_bool() &&
- can_create_other_service_instances_value->GetBool();
-
- const base::Value* instance_sharing_value =
- options_value->FindKey("instance_sharing");
- if (instance_sharing_value && instance_sharing_value->is_string()) {
- if (instance_sharing_value->GetString() == "singleton") {
- manifest.options.instance_sharing_policy =
- InstanceSharingPolicy::kSingleton;
- } else if (instance_sharing_value->GetString() ==
- "shared_instance_across_users" ||
- instance_sharing_value->GetString() ==
- "shared_across_instance_groups") {
- manifest.options.instance_sharing_policy =
- InstanceSharingPolicy::kSharedAcrossGroups;
- }
- }
- }
-
- const base::Value* interface_provider_specs_value =
- value.FindKey("interface_provider_specs");
- if (interface_provider_specs_value &&
- interface_provider_specs_value->is_dict()) {
- for (const auto& spec : interface_provider_specs_value->DictItems()) {
- if (!spec.second.is_dict())
- continue;
- const std::string& spec_name = spec.first;
- const bool is_main_spec = (spec_name == "service_manager:connector");
- const base::Value* provides_value = spec.second.FindKey("provides");
- if (provides_value && provides_value->is_dict()) {
- for (const auto& capability : provides_value->DictItems()) {
- if (!capability.second.is_list())
- continue;
- const std::string& capability_name = capability.first;
- std::set<std::string> interface_names;
- for (const auto& interface_name : capability.second.GetList())
- interface_names.insert(interface_name.GetString());
- if (is_main_spec) {
- ExposedCapability capability;
- capability.capability_name = capability_name;
- capability.interface_names = std::move(interface_names);
- manifest.exposed_capabilities.emplace_back(std::move(capability));
- } else {
- ExposedInterfaceFilterCapability capability;
- capability.filter_name = spec_name;
- capability.capability_name = capability_name;
- capability.interface_names = std::move(interface_names);
- manifest.exposed_interface_filter_capabilities.emplace_back(
- std::move(capability));
- }
- }
- }
-
- const base::Value* requires_value = spec.second.FindKey("requires");
- if (requires_value && requires_value->is_dict()) {
- for (const auto& entry : requires_value->DictItems()) {
- if (!entry.second.is_list())
- continue;
- const std::string& from_service_name = entry.first;
- if (is_main_spec) {
- if (entry.second.GetList().empty()) {
- manifest.required_capabilities.push_back({from_service_name, ""});
- } else {
- for (const auto& capability_name : entry.second.GetList()) {
- manifest.required_capabilities.push_back(
- {from_service_name, capability_name.GetString()});
- }
- }
- } else {
- for (const auto& capability_name : entry.second.GetList()) {
- manifest.required_interface_filter_capabilities.push_back(
- {from_service_name, spec_name, capability_name.GetString()});
- }
- }
- }
- }
- }
- }
-
- const base::Value* required_values_value = value.FindKey("required_files");
- if (required_values_value && required_values_value->is_dict()) {
-#if defined(OS_LINUX)
- constexpr const char kRequiredPlatform[] = "linux";
-#elif defined(OS_ANDROID)
- constexpr const char kRequiredPlatform[] = "android";
-#else
- constexpr const char kRequiredPlatform[] = "none";
-#endif
- for (const auto& item : required_values_value->DictItems()) {
- const std::string& key = item.first;
- if (!item.second.is_list())
- continue;
-
- for (const auto& entry : item.second.GetList()) {
- if (!entry.is_dict())
- continue;
- const base::Value* path_value = entry.FindKey("path");
- const base::Value* platform_value = entry.FindKey("platform");
- if (platform_value && platform_value->is_string() && path_value &&
- path_value->is_string() &&
- platform_value->GetString() == kRequiredPlatform) {
- PreloadedFileInfo info;
- info.key = key;
- info.path = base::FilePath().AppendASCII(path_value->GetString());
- manifest.preloaded_files.emplace_back(std::move(info));
- }
- }
- }
- }
-
- constexpr const char kServicesKey[] = "services";
- base::Value* services_value = value.FindKey(kServicesKey);
- if (services_value && services_value->is_list()) {
- for (auto& manifest_value : services_value->GetList()) {
- manifest.packaged_services.emplace_back(FromValueDeprecated(
- std::make_unique<base::Value>(std::move(manifest_value))));
- }
- }
-
- return manifest;
-}
-
Manifest& Manifest::Amend(Manifest other) {
for (auto& other_capability : other.exposed_capabilities) {
auto it = std::find_if(
diff --git a/services/service_manager/public/cpp/manifest.h b/services/service_manager/public/cpp/manifest.h
index 85fe136..33e6e0a 100644
--- a/services/service_manager/public/cpp/manifest.h
+++ b/services/service_manager/public/cpp/manifest.h
@@ -11,7 +11,6 @@
#include "base/component_export.h"
#include "base/files/file_path.h"
-#include "base/values.h"
namespace service_manager {
@@ -259,11 +258,6 @@
Manifest& operator=(const Manifest&);
Manifest& operator=(Manifest&&);
- // Creates a new Manifest object from a |base::Value| representation of the
- // deprecated JSON manifest format. This is a temporary function and should
- // only be used to transition services away from JSON manifests.
- static Manifest FromValueDeprecated(std::unique_ptr<base::Value> value_ptr);
-
// Amends this Manifest with a subset of |other|. Namely, exposed and required
// capabilities, exposed and required interface filter capabilities, packaged
// services, and preloaded files are all added from |other| if present.
diff --git a/services/service_manager/public/cpp/manifest_unittest.cc b/services/service_manager/public/cpp/manifest_unittest.cc
index 729e3c1..d8c315c 100644
--- a/services/service_manager/public/cpp/manifest_unittest.cc
+++ b/services/service_manager/public/cpp/manifest_unittest.cc
@@ -8,10 +8,8 @@
#include <string>
#include "base/files/file_path.h"
-#include "base/json/json_reader.h"
#include "base/no_destructor.h"
#include "base/stl_util.h"
-#include "base/values.h"
#include "services/service_manager/public/cpp/manifest_builder.h"
#include "services/service_manager/public/mojom/connector.mojom.h"
#include "testing/gmock/include/gmock/gmock.h"
@@ -97,102 +95,6 @@
EXPECT_EQ(3u, manifest.preloaded_files.size());
}
-TEST(ManifestTest, FromValueDeprecated) {
- constexpr const char kTestManifestJson[] = R"(
- {
- "name": "foo",
- "display_name": "bar",
- "sandbox_type": "utility",
- "services": [
- { "name": "packaged1" },
- { "name": "packaged2" }
- ],
- "options": {
- "can_connect_to_other_services_as_any_user": true,
- "can_connect_to_other_services_with_any_instance_name": true,
- "can_create_other_service_instances": true,
- "instance_sharing": "singleton"
- },
- "interface_provider_specs": {
- "service_manager:connector": {
- "provides": {
- "cap1": ["interface1", "interface2"],
- "cap2": ["interface3"],
- "cap3": []
- },
- "requires": {
- "a_service": ["cap3"],
- "another_service": ["cap4", "cap5"],
- "one_more_service": []
- }
- },
- "navigation:frame": {
- "provides": {
- "cap6": ["interface4"]
- },
- "requires": {
- "yet_another_service": ["cap7", "cap8"]
- }
- }
- }
- }
- )";
- const Manifest manifest{
- Manifest::FromValueDeprecated(base::JSONReader::Read(kTestManifestJson))};
-
- EXPECT_EQ("foo", manifest.service_name);
- EXPECT_EQ("bar", manifest.display_name.raw_string);
-
- EXPECT_EQ("utility", manifest.options.sandbox_type);
- EXPECT_EQ(Manifest::InstanceSharingPolicy::kSingleton,
- manifest.options.instance_sharing_policy);
- EXPECT_EQ(true, manifest.options.can_connect_to_instances_in_any_group);
- EXPECT_EQ(true, manifest.options.can_connect_to_instances_with_any_id);
- EXPECT_EQ(true, manifest.options.can_register_other_service_instances);
-
- const auto& exposed_capabilities = manifest.exposed_capabilities;
- ASSERT_EQ(3u, exposed_capabilities.size());
- EXPECT_EQ("cap1", exposed_capabilities[0].capability_name);
- EXPECT_THAT(exposed_capabilities[0].interface_names,
- ElementsAre("interface1", "interface2"));
- EXPECT_EQ("cap2", exposed_capabilities[1].capability_name);
- EXPECT_THAT(exposed_capabilities[1].interface_names,
- ElementsAre("interface3"));
- EXPECT_EQ("cap3", exposed_capabilities[2].capability_name);
- EXPECT_TRUE(exposed_capabilities[2].interface_names.empty());
-
- const auto& required_capabilities = manifest.required_capabilities;
- ASSERT_EQ(4u, required_capabilities.size());
- EXPECT_EQ("a_service", required_capabilities[0].service_name);
- EXPECT_EQ("cap3", required_capabilities[0].capability_name);
- EXPECT_EQ("another_service", required_capabilities[1].service_name);
- EXPECT_EQ("cap4", required_capabilities[1].capability_name);
- EXPECT_EQ("another_service", required_capabilities[2].service_name);
- EXPECT_EQ("cap5", required_capabilities[2].capability_name);
- EXPECT_EQ("one_more_service", required_capabilities[3].service_name);
- EXPECT_EQ("", required_capabilities[3].capability_name);
-
- const auto& exposed_filters = manifest.exposed_interface_filter_capabilities;
- ASSERT_EQ(1u, exposed_filters.size());
- EXPECT_EQ("navigation:frame", exposed_filters[0].filter_name);
- EXPECT_EQ("cap6", exposed_filters[0].capability_name);
- EXPECT_THAT(exposed_filters[0].interface_names, ElementsAre("interface4"));
-
- const auto& required_filters =
- manifest.required_interface_filter_capabilities;
- ASSERT_EQ(2u, required_filters.size());
- EXPECT_EQ("navigation:frame", required_filters[0].filter_name);
- EXPECT_EQ("yet_another_service", required_filters[0].service_name);
- EXPECT_EQ("cap7", required_filters[0].capability_name);
- EXPECT_EQ("navigation:frame", required_filters[1].filter_name);
- EXPECT_EQ("yet_another_service", required_filters[1].service_name);
- EXPECT_EQ("cap8", required_filters[1].capability_name);
-
- ASSERT_EQ(2u, manifest.packaged_services.size());
- EXPECT_EQ("packaged1", manifest.packaged_services[0].service_name);
- EXPECT_EQ("packaged2", manifest.packaged_services[1].service_name);
-}
-
TEST(ManifestTest, Amend) {
// Verify that everything is properly merged when amending potentially
// overlapping capability metadata.
