Skip to content

Commit

Permalink
Mojo: separate generation of MojoLPM and JS fuzzing bindings.
Browse files Browse the repository at this point in the history
This CL replaces the enable_fuzzing mojom buildrule parameter with two
flags, enable_js_fuzzing and enable_mojolpm_fuzzing, which allow
independently controlling the generation of the MojoLPM and JS fuzzing
bindings.

This means that MojoLPM fuzzers don't need to require the generation
of legacy JS bindings, and allows cleaning up some of the WebUI and
ChromeOS specific interfaces which currently need to generate unused
legacy bindings.

Bug: 1413182
Change-Id: I426f40a96c8ba15898c78a30d235d26e0c23fc4b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4225396
Reviewed-by: Rebekah Potter <rbpotter@chromium.org>
Reviewed-by: Alex Gough <ajgo@chromium.org>
Reviewed-by: Jack Hsieh <chengweih@chromium.org>
Reviewed-by: Michael van Ouwerkerk <mvanouwerkerk@chromium.org>
Reviewed-by: Austin Sullivan <asully@chromium.org>
Commit-Queue: Mark Brand <markbrand@google.com>
Reviewed-by: Andrew Paseltiner <apaseltiner@chromium.org>
Reviewed-by: Roland Bock <rbock@google.com>
Cr-Commit-Position: refs/heads/main@{#1111488}
  • Loading branch information
c01db33f authored and Chromium LUCI CQ committed Mar 1, 2023
1 parent 85b65eb commit 0695beb
Show file tree
Hide file tree
Showing 9 changed files with 50 additions and 38 deletions.
2 changes: 2 additions & 0 deletions chromeos/ash/services/nearby/public/mojom/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ mojom("mojom") {
"//url/mojom:url_mojom_gurl",
]

enable_js_fuzzing = false

cpp_only = true

cpp_typemaps = [
Expand Down
1 change: 1 addition & 0 deletions chromeos/crosapi/mojom/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ mojom("mojom") {
"web_page_info.mojom",
]
disable_variants = true
enable_js_fuzzing = false
cpp_only = true

public_deps = [
Expand Down
8 changes: 1 addition & 7 deletions components/attribution_reporting/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@ mojom("mojom") {
webui_module_path = "/"
use_typescript_sources = true

# The 3 legacy JS bindings types are required by Blink and by the fuzzer
# targets below.
# Used by Blink, so requires legacy JS bindings.
generate_legacy_js_bindings = true
generate_java = true
}
Expand Down Expand Up @@ -305,9 +304,4 @@ mojom("source_type_mojom") {
sources = [ "source_type.mojom" ]
webui_module_path = "/"
use_typescript_sources = true

# Fuzzers use the 3 legacy JS bindings types. This mojom is a dependency of
# the attribution_reporting component, which is used by the fuzzer tests
# above.
generate_legacy_js_bindings = enable_mojom_fuzzer
}
8 changes: 5 additions & 3 deletions components/services/storage/privileged/mojom/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,6 @@ mojom("mojom_bucket") {

webui_module_path = "/"
use_typescript_sources = true

# Fuzzers require legacy bindings.
generate_legacy_js_bindings = enable_mojom_fuzzer
}

mojom("mojom") {
Expand All @@ -26,8 +23,13 @@ mojom("mojom") {
"indexed_db_control.mojom",
"indexed_db_control_test.mojom",
]

cpp_only = true

# The interfaces defined here are privileged, and not vended to a renderer, so
# they are not fuzzable by javascript-based fuzzers.
enable_js_fuzzing = false

public_deps = [
":mojom_bucket",
"//components/services/storage/public/mojom",
Expand Down
4 changes: 4 additions & 0 deletions components/services/storage/public/mojom/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ import("//mojo/public/tools/bindings/mojom.gni")
mojom("mojom") {
cpp_only = true

# The interfaces defined here are privileged, and not vended to a renderer, so
# they are not fuzzable by javascript-based fuzzers.
enable_js_fuzzing = false

sources = [
"blob_storage_context.mojom",
"cache_storage_control.mojom",
Expand Down
3 changes: 0 additions & 3 deletions components/services/storage/public/mojom/buckets/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,4 @@ mojom("buckets") {

webui_module_path = "/"
use_typescript_sources = true

# Fuzzers use legacy bindings.
generate_legacy_js_bindings = enable_mojom_fuzzer
}
8 changes: 4 additions & 4 deletions gpu/ipc/common/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -665,11 +665,11 @@ mojom("gpu_preferences_interface") {
mojom("vulkan_interface") {
generate_java = true

# TODO(crbug.com/1062364): This interface code is used by some javascript targets even
# when vulkan isn't enabled, but the C++ fuzzer code will fail to compile if
# the headers aren't available.
# TODO(crbug.com/1062364): This interface code is used by some javascript
# targets even when vulkan isn't enabled, but the C++ fuzzer code will fail
# to compile if the headers aren't available.
if (!enable_vulkan) {
enable_fuzzing = false
enable_mojolpm_fuzzing = false
}

sources = [
Expand Down
46 changes: 29 additions & 17 deletions mojo/public/tools/bindings/mojom.gni
Original file line number Diff line number Diff line change
Expand Up @@ -262,12 +262,16 @@ if (enable_scrambled_message_ids) {
# |cpp_only| is set to true, it overrides this to prevent generation of
# Java bindings.
#
# enable_fuzzing (optional)
# enable_js_fuzzing (optional)
# Enables generation of javascript fuzzing sources for the target if the
# global build arg |enable_mojom_fuzzer| is also set to |true|.
# Defaults to |true|. If JS fuzzing generation is enabled for a target,
# the target will always generate JS bindings even if |cpp_only| is set to
# |true|. See note above.
#
# enable_mojolpm_fuzzing (optional)
# Enables generation of fuzzing sources for the target if the global build
# arg |enable_mojom_fuzzer| is also set to |true|. Defaults to |true|. If
# fuzzing generation is enabled for a target, the target will always
# generate JS bindings even if |cpp_only| is set to |true|. See note
# above.
# arg |enable_mojom_fuzzer| is also set to |true|. Defaults to |true|.
#
# support_lazy_serialization (optional)
# If set to |true|, generated C++ bindings will effectively prefer to
Expand Down Expand Up @@ -667,10 +671,15 @@ template("mojom") {
}
write_file(build_metadata_filename, build_metadata, "json")

generate_fuzzing =
(!defined(invoker.enable_fuzzing) || invoker.enable_fuzzing) &&
generate_js_fuzzing =
(!defined(invoker.enable_js_fuzzing) || invoker.enable_js_fuzzing) &&
enable_mojom_fuzzer && (!defined(invoker.testonly) || !invoker.testonly)

generate_mojolpm_fuzzing =
(!defined(invoker.enable_mojolpm_fuzzing) ||
invoker.enable_mojolpm_fuzzing) && enable_mojom_fuzzer &&
(!defined(invoker.testonly) || !invoker.testonly)

parser_target_name = "${target_name}__parser"
parser_deps = []
foreach(dep, all_deps) {
Expand Down Expand Up @@ -1036,7 +1045,7 @@ template("mojom") {
}
}

if (generate_fuzzing) {
if (generate_mojolpm_fuzzing) {
# This block generates the proto files used for the MojoLPM fuzzer,
# and the corresponding proto targets that will be linked in the fuzzer
# targets. These are independent of the typemappings, and can be done
Expand Down Expand Up @@ -1317,7 +1326,8 @@ template("mojom") {
"$root_gen_dir/mojo/public/tools/bindings/cpp_templates.zip",
type_mappings_path,
]
if (generate_fuzzing && !defined(bindings_configuration.variant)) {
if (generate_mojolpm_fuzzing &&
!defined(bindings_configuration.variant)) {
sources += [
"$root_gen_dir/mojo/public/tools/bindings/mojolpm_templates.zip",
]
Expand Down Expand Up @@ -1349,7 +1359,8 @@ template("mojom") {
"$root_gen_dir/${base_path}${variant_dash_suffix}.cc",
"$root_gen_dir/${base_path}${variant_dash_suffix}.h",
]
if (generate_fuzzing && !defined(bindings_configuration.variant)) {
if (generate_mojolpm_fuzzing &&
!defined(bindings_configuration.variant)) {
outputs += [
"$root_gen_dir/${base_path}${variant_dash_suffix}-mojolpm.cc",
"$root_gen_dir/${base_path}${variant_dash_suffix}-mojolpm.h",
Expand All @@ -1366,7 +1377,8 @@ template("mojom") {
"-g",
]

if (generate_fuzzing && !defined(bindings_configuration.variant)) {
if (generate_mojolpm_fuzzing &&
!defined(bindings_configuration.variant)) {
args += [ "c++,mojolpm" ]
} else {
args += [ "c++" ]
Expand Down Expand Up @@ -1644,7 +1656,7 @@ template("mojom") {
}
}

if (generate_fuzzing && !defined(variant)) {
if (generate_mojolpm_fuzzing && !defined(variant)) {
# This block contains the C++ targets for the MojoLPM fuzzer, we need to
# do this here so that we can use the typemap configuration for the
# empty-variant Mojo target.
Expand Down Expand Up @@ -1896,8 +1908,8 @@ template("mojom") {
# targets when generate_legacy_js_bindings is true. This option is provided
# since the legacy bindings are needed by Blink tests and non-Chromium users,
# which are not expected to migrate to modules or TypeScript.
if (generate_legacy_js &&
(generate_fuzzing || !defined(invoker.cpp_only) || !invoker.cpp_only)) {
if (generate_legacy_js && (generate_js_fuzzing ||
!defined(invoker.cpp_only) || !invoker.cpp_only)) {
if (sources_list != []) {
generator_js_target_name = "${target_name}_js__generator"

Expand Down Expand Up @@ -1951,7 +1963,7 @@ template("mojom") {
args += message_scrambling_args
}

if (generate_fuzzing) {
if (generate_js_fuzzing) {
args += [ "--generate_fuzzing" ]
}
}
Expand Down Expand Up @@ -2078,8 +2090,8 @@ template("mojom") {
}
}
}
if ((generate_fuzzing || !defined(invoker.cpp_only) || !invoker.cpp_only) &&
use_typescript_for_target) {
if ((generate_js_fuzzing || !defined(invoker.cpp_only) ||
!invoker.cpp_only) && use_typescript_for_target) {
if (sources_list != []) {
source_filelist = []
foreach(source, sources_list) {
Expand Down
8 changes: 4 additions & 4 deletions services/device/public/mojom/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,10 @@ mojom("device_service") {
disable_variants = true
cpp_only = true

# The interfaces defined here are privileged, and not vended to a renderer, so
# they are not fuzzable by javascript-based fuzzers.
enable_js_fuzzing = false

enabled_features = []
if ((is_linux || is_chromeos) && use_udev) {
enabled_features += [ "enable_input_device_manager" ]
Expand Down Expand Up @@ -208,8 +212,4 @@ mojom("usb_test") {
public_deps = [ "//url/mojom:url_mojom_gurl" ]
webui_module_path = "/"
use_typescript_sources = true

# Generate legacy JS bindings in addition to WebUI bindings for fuzzers,
# since fuzzers still use the 3 legacy bindings types.
generate_legacy_js_bindings = enable_mojom_fuzzer
}

0 comments on commit 0695beb

Please sign in to comment.