0

mojo: drop lazy serialization support for interfaces

Lazy serialization is only used in tests, and it complicates the
binding generator a bunch. This test removes lazy serialization
for interfaces and all its test coverage. It does not remove lazy
message serialization.

Bug: 40732837
Change-Id: Ie5dfbf5c5d8524c8aa9ca425f31a5ab0f23e4772
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5381630
Reviewed-by: Richard (Torne) Coles <torne@chromium.org>
Commit-Queue: Elly FJ <ellyjones@chromium.org>
Reviewed-by: Oksana Zhuravlova <oksamyt@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1276568}
This commit is contained in:
Elly
2024-03-21 22:58:54 +00:00
committed by Chromium LUCI CQ
parent 2738edea66
commit ba734fdf7c
11 changed files with 18 additions and 358 deletions
components/cronet/tools/generators
ipc
mojo/public

@ -216,8 +216,7 @@ class MojomProcessor(object):
for_blink=args.for_blink,
export_attribute=args.export_attribute,
export_header=args.export_header,
generate_non_variant_code=args.generate_non_variant_code,
support_lazy_serialization=args.support_lazy_serialization)
generate_non_variant_code=args.generate_non_variant_code)
filtered_args = []
if hasattr(generator_module, 'GENERATOR_PREFIX'):
prefix = '--' + generator_module.GENERATOR_PREFIX + '_'
@ -366,10 +365,6 @@ def main():
"a salt for generating scrambled message IDs. If this switch is specified"
"more than once, the contents of all salt files are concatenated to form"
"the salt value.", default=[], action="append")
generate_parser.add_argument(
"--support_lazy_serialization",
help="If set, generated bindings will serialize lazily when possible.",
action="store_true")
generate_parser.set_defaults(func=_Generate)
precompile_parser = subparsers.add_parser("precompile",

@ -310,7 +310,6 @@ class Generator(generator.Generator):
"namespace": self.module.namespace,
"namespaces_as_array": NamespaceToArray(self.module.namespace),
"structs": self.module.structs,
"support_lazy_serialization": self.support_lazy_serialization,
"unions": self.module.unions,
"variant": self.variant,
}
@ -344,8 +343,6 @@ class Generator(generator.Generator):
"get_qualified_name_for_kind": self._GetQualifiedNameForKind,
"has_callbacks": mojom.HasCallbacks,
"has_sync_methods": mojom.HasSyncMethods,
"method_supports_lazy_serialization":
self._MethodSupportsLazySerialization,
"requires_context_for_data_view": RequiresContextForDataView,
"should_inline": ShouldInlineStruct,
"should_inline_union": ShouldInlineUnion,
@ -706,13 +703,6 @@ class Generator(generator.Generator):
add_same_module_namespaces=True)
return self._GetCppWrapperType(kind, add_same_module_namespaces=True)
def _MethodSupportsLazySerialization(self, method):
# TODO(crbug.com/753431,crbug.com/753433): Support lazy serialization for
# methods which pass associated handles and InterfacePtrs.
return self.support_lazy_serialization and (
not mojom.MethodPassesAssociatedKinds(method) and
not mojom.MethodPassesInterfaces(method))
def _TranslateConstants(self, token, kind):
if isinstance(token, mojom.NamedValue):
return self._GetNameForKind(token, flatten_nested_kind=True)

@ -225,7 +225,6 @@ mojom("mojom_constants") {
mojom("test_interfaces") {
testonly = true
sources = [ "ipc_test.mojom" ]
support_lazy_serialization = true
}
# This is provided as a separate target so other targets can provide param

@ -33,7 +33,6 @@ source_set("tests") {
"hash_unittest.cc",
"idle_tracking_unittest.cc",
"interface_unittest.cc",
"lazy_serialization_unittest.cc",
"map_unittest.cc",
"message_queue.cc",
"message_queue.h",
@ -195,8 +194,6 @@ mojom("test_mojom") {
"//mojo/public/mojom/base",
]
support_lazy_serialization = true
cpp_typemaps = [
{
types = [

@ -1,171 +0,0 @@
// Copyright 2017 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "base/functional/bind.h"
#include "base/run_loop.h"
#include "base/test/bind.h"
#include "base/test/task_environment.h"
#include "mojo/core/embedder/embedder.h"
#include "mojo/public/cpp/bindings/receiver.h"
#include "mojo/public/cpp/bindings/remote.h"
#include "mojo/public/cpp/bindings/tests/bindings_test_base.h"
#include "mojo/public/interfaces/bindings/tests/struct_with_traits.mojom.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace mojo {
namespace {
class LazySerializationTest : public testing::Test {
public:
LazySerializationTest() = default;
LazySerializationTest(const LazySerializationTest&) = delete;
LazySerializationTest& operator=(const LazySerializationTest&) = delete;
~LazySerializationTest() override = default;
private:
base::test::TaskEnvironment task_environment_;
};
class TestUnserializedStructImpl : public test::TestUnserializedStruct {
public:
explicit TestUnserializedStructImpl(
PendingReceiver<test::TestUnserializedStruct> receiver)
: receiver_(this, std::move(receiver)) {}
TestUnserializedStructImpl(const TestUnserializedStructImpl&) = delete;
TestUnserializedStructImpl& operator=(const TestUnserializedStructImpl&) =
delete;
~TestUnserializedStructImpl() override = default;
// test::TestUnserializedStruct:
void PassUnserializedStruct(
const test::StructWithUnreachableTraitsImpl& s,
PassUnserializedStructCallback callback) override {
std::move(callback).Run(s);
}
private:
Receiver<test::TestUnserializedStruct> receiver_;
};
class ForceSerializeTesterImpl : public test::ForceSerializeTester {
public:
ForceSerializeTesterImpl(PendingReceiver<test::ForceSerializeTester> receiver)
: receiver_(this, std::move(receiver)) {}
ForceSerializeTesterImpl(const ForceSerializeTesterImpl&) = delete;
ForceSerializeTesterImpl& operator=(const ForceSerializeTesterImpl&) = delete;
~ForceSerializeTesterImpl() override = default;
// test::ForceSerializeTester:
void SendForceSerializedStruct(
const test::StructForceSerializeImpl& s,
SendForceSerializedStructCallback callback) override {
std::move(callback).Run(s);
}
void SendNestedForceSerializedStruct(
const test::StructNestedForceSerializeImpl& s,
SendNestedForceSerializedStructCallback callback) override {
std::move(callback).Run(s);
}
private:
Receiver<test::ForceSerializeTester> receiver_;
};
TEST_F(LazySerializationTest, NeverSerialize) {
// Basic sanity check to ensure that no messages are serialized by default in
// environments where lazy serialization is supported, on an interface which
// supports lazy serialization, and where both ends of the interface are in
// the same process.
Remote<test::TestUnserializedStruct> remote;
TestUnserializedStructImpl impl(remote.BindNewPipeAndPassReceiver());
const int32_t kTestMagicNumber = 42;
test::StructWithUnreachableTraitsImpl data;
EXPECT_EQ(0, data.magic_number);
data.magic_number = kTestMagicNumber;
// Send our data over the pipe and wait for it to come back. The value should
// be preserved. We know the data was never serialized because the
// StructTraits for this type will DCHECK if executed in any capacity.
base::RunLoop loop;
remote->PassUnserializedStruct(
data, base::BindLambdaForTesting(
[&](const test::StructWithUnreachableTraitsImpl& passed) {
EXPECT_EQ(kTestMagicNumber, passed.magic_number);
loop.Quit();
}));
loop.Run();
}
TEST_F(LazySerializationTest, ForceSerialize) {
// Verifies that the [force_serialize] attribute works as intended: i.e., even
// with lazy serialization enabled, messages which carry a force-serialized
// type will always serialize at call time.
Remote<test::ForceSerializeTester> tester;
ForceSerializeTesterImpl impl(tester.BindNewPipeAndPassReceiver());
constexpr int32_t kTestValue = 42;
base::RunLoop loop;
test::StructForceSerializeImpl in;
in.set_value(kTestValue);
EXPECT_FALSE(in.was_serialized());
EXPECT_FALSE(in.was_deserialized());
tester->SendForceSerializedStruct(
in, base::BindLambdaForTesting(
[&](const test::StructForceSerializeImpl& passed) {
EXPECT_EQ(kTestValue, passed.value());
EXPECT_TRUE(passed.was_deserialized());
EXPECT_FALSE(passed.was_serialized());
loop.Quit();
}));
EXPECT_TRUE(in.was_serialized());
EXPECT_FALSE(in.was_deserialized());
loop.Run();
EXPECT_TRUE(in.was_serialized());
EXPECT_FALSE(in.was_deserialized());
}
TEST_F(LazySerializationTest, ForceSerializeNested) {
// Verifies that the [force_serialize] attribute works as intended in a nested
// context, i.e. when a force-serialized type is contained within a
// non-force-serialized type,
Remote<test::ForceSerializeTester> tester;
ForceSerializeTesterImpl impl(tester.BindNewPipeAndPassReceiver());
constexpr int32_t kTestValue = 42;
base::RunLoop loop;
test::StructNestedForceSerializeImpl in;
in.force().set_value(kTestValue);
EXPECT_FALSE(in.was_serialized());
EXPECT_FALSE(in.was_deserialized());
tester->SendNestedForceSerializedStruct(
in, base::BindLambdaForTesting(
[&](const test::StructNestedForceSerializeImpl& passed) {
EXPECT_EQ(kTestValue, passed.force().value());
EXPECT_TRUE(passed.was_deserialized());
EXPECT_FALSE(passed.was_serialized());
loop.Quit();
}));
EXPECT_TRUE(in.was_serialized());
EXPECT_FALSE(in.was_deserialized());
loop.Run();
EXPECT_TRUE(in.was_serialized());
EXPECT_FALSE(in.was_deserialized());
}
} // namespace
} // namespace mojo

@ -189,8 +189,6 @@ mojom("test_interfaces") {
},
]
support_lazy_serialization = true
# Validation tests require precise message content matching, so we avoid
# scrambling message IDs for test interfaces.
scramble_message_ids = false
@ -353,8 +351,6 @@ mojom("test_struct_traits_interfaces") {
[ "//mojo/public/cpp/bindings/tests:struct_with_traits_impl" ]
},
]
support_lazy_serialization = true
}
mojom("test_associated_interfaces") {

@ -246,12 +246,6 @@ class {{class_name}}_{{method.name}}_ForwardToCallback
"%s.%s request"|format(interface.name, method.name) %}
{%- set message_typename = "%s_%s_Message"|format(proxy_name, method.name) %}
{%- if method|method_supports_lazy_serialization %}
{{interface_macros.define_message_type(
interface, message_typename, message_name, False, method, method.parameters,
params_struct, params_description)}}
{%- endif %}
{%- if method.sync %}
bool {{proxy_name}}::{{method.name}}(
{{interface_macros.declare_sync_method_params("param_", method)}}) {
@ -278,21 +272,12 @@ bool {{proxy_name}}::{{method.name}}(
{% else %}
const bool is_urgent = false;
{%- endif %}
{%- if method|method_supports_lazy_serialization %}
const bool kSerialize = receiver_->PrefersSerializedMessages();
auto message = {{message_typename}}::Build(
kSerialize, kExpectsResponse, kIsSync, kAllowInterrupt, is_urgent
{%- for param in method.parameters -%}
, std::move(param_{{param.name}})
{%- endfor %});
{%- else %} {#- if method|method_supports_lazy_serialization #}
{{interface_macros.build_message_flags(False, "kIsSync", "kAllowInterrupt",
"is_urgent", "kExpectsResponse",
"kFlags")}}
{{interface_macros.build_serialized_message(
message_name, method, "param_%s", params_struct, params_description,
"kFlags", "message")}}
{%- endif %} {#- if method|method_supports_lazy_serialization #}
{{interface_macros.build_message_flags(False, "kIsSync", "kAllowInterrupt",
"is_urgent", "kExpectsResponse",
"kFlags")}}
{{interface_macros.build_serialized_message(
message_name, method, "param_%s", params_struct, params_description,
"kFlags", "message")}}
#if defined(ENABLE_IPC_FUZZER)
message.set_interface_name({{class_name}}::Name_);
@ -342,21 +327,12 @@ void {{proxy_name}}::{{method.name}}(
{% else %}
const bool is_urgent = false;
{%- endif %}
{%- if method|method_supports_lazy_serialization %}
const bool kSerialize = receiver_->PrefersSerializedMessages();
auto message = {{message_typename}}::Build(
kSerialize, kExpectsResponse, kIsSync, kAllowInterrupt, is_urgent
{%- for param in method.parameters -%}
, std::move(in_{{param.name}})
{%- endfor %});
{%- else %}
{{interface_macros.build_message_flags(False, "kIsSync", "kAllowInterrupt",
"is_urgent", "kExpectsResponse",
"kFlags")}}
{{interface_macros.build_serialized_message(
message_name, method, "in_%s", params_struct, params_description,
"kFlags", "message", True)}}
{%- endif %}
{{interface_macros.build_message_flags(False, "kIsSync", "kAllowInterrupt",
"is_urgent", "kExpectsResponse",
"kFlags")}}
{{interface_macros.build_serialized_message(
message_name, method, "in_%s", params_struct, params_description,
"kFlags", "message", True)}}
#if defined(ENABLE_IPC_FUZZER)
message.set_interface_name({{class_name}}::Name_);
@ -439,31 +415,8 @@ class {{class_name}}_{{method.name}}_ProxyToResponder : public ::mojo::internal:
{{interface_macros.declare_params("in_", method.response_parameters)}});
};
{%- if method|method_supports_lazy_serialization %}
{{interface_macros.define_message_type(
interface, response_message_typename, message_name, True, method,
method.response_parameters, response_params_struct, params_description)}}
{%- endif %}
bool {{class_name}}_{{method.name}}_ForwardToCallback::Accept(
mojo::Message* message) {
{%- if method|method_supports_lazy_serialization %}
if (!message->is_serialized()) {
auto context =
message->TakeUnserializedContext<{{response_message_typename}}>();
if (!context) {
// The Message was not of the expected type. It may be a valid message
// which was build using a different variant of these bindings. Force
// serialization before dispatch in this case.
message->SerializeIfNecessary();
} else {
if (!callback_.is_null())
context->Dispatch(message, &callback_);
return true;
}
}
{%- endif %}
DCHECK(message->is_serialized());
internal::{{class_name}}_{{method.name}}_ResponseParams_Data* params =
reinterpret_cast<
@ -488,20 +441,11 @@ void {{class_name}}_{{method.name}}_ProxyToResponder::Run(
parameter_group="async_response_parameters",
dereference_parameters=False)}}
#endif
{%- if method|method_supports_lazy_serialization %}
const bool kSerialize = responder_->PrefersSerializedMessages();
auto message = {{response_message_typename}}::Build(kSerialize, is_sync_,
true, false
{%- for param in method.response_parameters -%}
, std::move(in_{{param.name}})
{%- endfor %});
{%- else %}
{{interface_macros.build_message_flags(True, "is_sync_", "true", "false",
"false", "kFlags")}}
{{interface_macros.build_serialized_message(
message_name, method, "in_%s", response_params_struct,
response_params_description, "kFlags", "message")}}
{%- endif %}
{{interface_macros.build_message_flags(True, "is_sync_", "true", "false",
"false", "kFlags")}}
{{interface_macros.build_serialized_message(
message_name, method, "in_%s", response_params_struct,
response_params_description, "kFlags", "message")}}
#if defined(ENABLE_IPC_FUZZER)
message.set_interface_name({{class_name}}::Name_);
@ -524,27 +468,6 @@ void {{class_name}}_{{method.name}}_ProxyToResponder::Run(
{%- if method.sync %}
bool {{class_name}}_{{method.name}}_HandleSyncResponse::Accept(
mojo::Message* message) {
{%- if method|method_supports_lazy_serialization %}
if (!message->is_serialized()) {
auto context =
message->TakeUnserializedContext<{{response_message_typename}}>();
if (!context) {
// The Message was not of the expected type. It may be a valid message
// which was built using a different variant of these bindings. Force
// serialization before dispatch in this case.
message->SerializeIfNecessary();
} else {
context->HandleSyncResponse(
message
{%- for param in method.response_parameters %},
out_{{param.name}}_
{%- endfor %});
*result_ = true;
return true;
}
}
{%- endif %}
DCHECK(message->is_serialized());
internal::{{class_name}}_{{method.name}}_ResponseParams_Data* params =
reinterpret_cast<internal::{{class_name}}_{{method.name}}_ResponseParams_Data*>(
@ -574,22 +497,6 @@ bool {{class_name}}StubDispatch::Accept(
{%- for method in interface.methods %}
case internal::k{{class_name}}_{{method.name}}_Name: {
{%- if method.response_parameters == None %}
{%- if method|method_supports_lazy_serialization %}
if (!message->is_serialized()) {
auto context = message->TakeUnserializedContext<
{{proxy_name}}_{{method.name}}_Message>();
if (!context) {
// The Message was not of the expected type. It may be a valid message
// which was serialized using a different variant of these bindings.
// Force serialization before dispatch in this case.
message->SerializeIfNecessary();
} else {
context->Dispatch(message, impl);
return true;
}
}
{%- endif %}
DCHECK(message->is_serialized());
internal::{{class_name}}_{{method.name}}_Params_Data* params =
reinterpret_cast<internal::{{class_name}}_{{method.name}}_Params_Data*>(
@ -625,25 +532,6 @@ bool {{class_name}}StubDispatch::AcceptWithResponder(
{%- for method in interface.methods %}
case internal::k{{class_name}}_{{method.name}}_Name: {
{%- if method.response_parameters != None %}
{%- if method|method_supports_lazy_serialization %}
if (!message->is_serialized()) {
auto context = message->TakeUnserializedContext<
{{proxy_name}}_{{method.name}}_Message>();
if (!context) {
// The Message was not of the expected type. It may be a valid message
// which was built using a different variant of these bindings. Force
// serialization before dispatch in this case.
message->SerializeIfNecessary();
} else {
{{class_name}}::{{method.name}}Callback callback =
{{class_name}}_{{method.name}}_ProxyToResponder::CreateCallback(
*message, std::move(responder));
context->Dispatch(message, impl, std::move(callback));
return true;
}
}
{%- endif %}
internal::{{class_name}}_{{method.name}}_Params_Data* params =
reinterpret_cast<
internal::{{class_name}}_{{method.name}}_Params_Data*>(

@ -359,7 +359,6 @@ class Generator(generator.Generator):
"module_namespace": self.module.namespace,
"namespaces_as_array": NamespaceToArray(self.module.namespace),
"structs": self.module.structs,
"support_lazy_serialization": self.support_lazy_serialization,
"unions": self.module.unions,
"uses_interfaces": self._ReferencesAnyHandleOrInterfaceType(),
"uses_native_types": self._ReferencesAnyNativeType(),
@ -406,8 +405,6 @@ class Generator(generator.Generator):
"get_sync_method_ordinals": mojom.GetSyncMethodOrdinals,
"has_uninterruptable_methods": mojom.HasUninterruptableMethods,
"has_estimate_size_methods": self._HasEstimateSizeMethods,
"method_supports_lazy_serialization":
self._MethodSupportsLazySerialization,
"requires_context_for_data_view": RequiresContextForDataView,
"should_inline": ShouldInlineStruct,
"should_inline_union": ShouldInlineUnion,
@ -951,18 +948,6 @@ class Generator(generator.Generator):
return False
def _MethodSupportsLazySerialization(self, method):
if not self.support_lazy_serialization:
return False
# TODO(crbug.com/753433): Support lazy serialization for methods which pass
# associated handles.
if mojom.MethodPassesAssociatedKinds(method):
return False
return not any(self._KindMustBeSerialized(param.kind) for param in
method.parameters + (method.response_parameters or []))
def _TranslateConstants(self, token, kind):
if isinstance(token, mojom.NamedValue):
return self._GetNameForKind(token, flatten_nested_kind=True)

@ -278,13 +278,6 @@ if (enable_scrambled_message_ids) {
# Enables generation of fuzzing sources for the target if the global build
# 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
# transmit messages in an unserialized form when going between endpoints
# in the same process. This avoids the runtime cost of serialization,
# deserialization, and validation logic at the expensive of increased
# code size. Defaults to |false|.
#
# disable_variants (optional)
# If |true|, no variant sources will be generated for the target. Defaults
# to |false|.
@ -1427,11 +1420,6 @@ template("mojom") {
args += [ "--for_blink" ]
}
if (defined(invoker.support_lazy_serialization) &&
invoker.support_lazy_serialization) {
args += [ "--support_lazy_serialization" ]
}
if (enable_kythe_annotations) {
args += [ "--enable_kythe_annotations" ]
}

@ -227,7 +227,6 @@ class MojomProcessor:
export_attribute=args.export_attribute,
export_header=args.export_header,
generate_non_variant_code=args.generate_non_variant_code,
support_lazy_serialization=args.support_lazy_serialization,
disallow_native_types=args.disallow_native_types,
disallow_interfaces=args.disallow_interfaces,
generate_message_ids=args.generate_message_ids,
@ -363,10 +362,6 @@ def main():
"a salt for generating scrambled message IDs. If this switch is specified"
"more than once, the contents of all salt files are concatenated to form"
"the salt value.", default=[], action="append")
generate_parser.add_argument(
"--support_lazy_serialization",
help="If set, generated bindings will serialize lazily when possible.",
action="store_true")
generate_parser.add_argument(
"--extra_cpp_template_paths",
dest="extra_cpp_template_paths",

@ -250,7 +250,6 @@ class Generator:
export_attribute=None,
export_header=None,
generate_non_variant_code=False,
support_lazy_serialization=False,
disallow_native_types=False,
disallow_interfaces=False,
generate_message_ids=False,
@ -268,7 +267,6 @@ class Generator:
self.export_attribute = export_attribute
self.export_header = export_header
self.generate_non_variant_code = generate_non_variant_code
self.support_lazy_serialization = support_lazy_serialization
self.disallow_native_types = disallow_native_types
self.disallow_interfaces = disallow_interfaces
self.generate_message_ids = generate_message_ids