Skip to content

Commit

Permalink
Move Mojo and IPC protobuf support to separate targets
Browse files Browse the repository at this point in the history
Currently, any target depending on Mojo or IPC must also depend on
protobuf, but the only dependency is some helper traits headers. As a
result, we still need to build protobuf for NaCl.

Split these into separate targets. This removes the last protobuf
dependency in NaCl and removes the need to build it for NaCl at all. In
particular, the new version of protobuf does not build with NaCl's libc
(it expects an <endian.h>). Rather than patch that, remove the
dependency altogether.

Bug: 1294200
Change-Id: If4e448e681c94a5eed329c57ef08f4dd88be5437
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3594979
Reviewed-by: Joe Downing <joedow@chromium.org>
Reviewed-by: Ken Rockot <rockot@google.com>
Commit-Queue: David Benjamin <davidben@chromium.org>
Cr-Commit-Position: refs/heads/main@{#994534}
  • Loading branch information
davidben authored and Chromium LUCI CQ committed Apr 21, 2022
1 parent cf02a44 commit 38677e7
Show file tree
Hide file tree
Showing 6 changed files with 29 additions and 8 deletions.
9 changes: 5 additions & 4 deletions chrome/services/file_util/public/mojom/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,12 @@ mojom("mojom") {
if (safe_browsing_mode == 1 && enable_maldoca) {
sources += [
"document_analysis_service.mojom",
"safe_document_analyzer.mojom"
"safe_document_analyzer.mojom",
]
enabled_features += [ "enable_maldoca" ]
public_deps += [
"//components/services/filesystem/public/mojom",
"//sandbox/policy/mojom"
"//sandbox/policy/mojom",
]
}

Expand All @@ -58,8 +58,9 @@ mojom("mojom") {
"//chrome/common/safe_browsing:proto",
"//components/safe_browsing:buildflags",
"//components/safe_browsing/core/common/proto:csd_proto",
"//ipc:protobuf_support",
]
}
},
]

if (safe_browsing_mode == 1 && enable_maldoca) {
Expand All @@ -78,7 +79,7 @@ mojom("mojom") {
"//components/safe_browsing:buildflags",
"//components/safe_browsing/core/common/proto:csd_proto",
]
}
},
]
}
}
1 change: 1 addition & 0 deletions chrome/test/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -7977,6 +7977,7 @@ test("unit_tests") {
"//chrome/common/safe_browsing:binary_feature_extractor",
"//chrome/common/safe_browsing:download_type_util",
"//chrome/services/file_util/public/cpp:unit_tests",
"//ipc:protobuf_support",
]
} else if (safe_browsing_mode == 2 && is_android) {
sources += [
Expand Down
13 changes: 11 additions & 2 deletions ipc/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ component("ipc") {
"ipc_message_macros.h",
"ipc_message_pipe_reader.cc",
"ipc_message_pipe_reader.h",
"ipc_message_protobuf_utils.h",
"ipc_message_start.h",
"ipc_message_templates.h",
"ipc_message_templates_impl.h",
Expand Down Expand Up @@ -97,7 +96,6 @@ component("ipc") {
"//mojo/public/cpp/bindings",
"//mojo/public/cpp/system",
"//services/tracing/public/cpp",
"//third_party/protobuf:protobuf_lite",
]

deps = [ "//base" ]
Expand Down Expand Up @@ -234,6 +232,16 @@ source_set("param_traits") {
public = [ "ipc_param_traits.h" ]
}

# This is provided as a separate target so other targets can use IPC without
# necessarily linking to protobuf.
source_set("protobuf_support") {
public = [ "ipc_message_protobuf_utils.h" ]
public_deps = [
":ipc",
"//third_party/protobuf:protobuf_lite",
]
}

if (!is_ios) {
source_set("run_all_unittests") {
testonly = true
Expand Down Expand Up @@ -277,6 +285,7 @@ if (!is_ios) {

deps = [
":ipc",
":protobuf_support",
":run_all_unittests",
":test_interfaces",
":test_proto",
Expand Down
12 changes: 10 additions & 2 deletions mojo/public/cpp/bindings/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ component("bindings_base") {
sources = [
"array_data_view.h",
"array_traits.h",
"array_traits_protobuf.h",
"array_traits_span.h",
"array_traits_stl.h",
"associated_group.h",
Expand Down Expand Up @@ -121,7 +120,6 @@ component("bindings_base") {
":mojo_buildflags",
"//base",
"//mojo/public/cpp/system",
"//third_party/protobuf:protobuf_lite",
]

if (enable_ipc_fuzzer) {
Expand Down Expand Up @@ -264,6 +262,16 @@ source_set("struct_traits") {
]
}

# This is provided as a separate target so other targets can use Mojo bindings
# without necessarily linking to protobuf.
source_set("protobuf_support") {
public = [ "array_traits_protobuf.h" ]
public_deps = [
":bindings",
"//third_party/protobuf:protobuf_lite",
]
}

if (!is_ios) {
component("wtf_support") {
sources = [
Expand Down
1 change: 1 addition & 0 deletions remoting/host/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,7 @@ static_library("common") {
"//components/webrtc:thread_wrapper",
"//crypto",
"//google_apis",
"//ipc:protobuf_support",
"//media",
"//remoting/base",
"//remoting/base:authorization",
Expand Down
1 change: 1 addition & 0 deletions remoting/host/mojom/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ mojom("mojom") {
traits_public_deps = [
"//mojo/public/cpp/base:shared_typemap_traits",
"//mojo/public/cpp/bindings",
"//mojo/public/cpp/bindings:protobuf_support",
"//remoting/host/base",
"//remoting/proto",
"//remoting/protocol:errors",
Expand Down

0 comments on commit 38677e7

Please sign in to comment.