MojoLPM: Refactor response handling.

This change removes the need for the separate response handling in MojoLPM
fuzzer targets, instead handling responses to methods called on fuzzer-hosted
receivers in the same way as calls to browser-hosted remotes. This also allows
more significant reordering of responses on the fuzzer side, hopefully
improving fuzzer performance.

Bug: 1076336
Change-Id: I6faceedae2bef31ced28c415f84e7e6ca009f458
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2302749
Commit-Queue: Mark Brand <markbrand@google.com>
Reviewed-by: Jonathan Metzman <metzman@chromium.org>
Reviewed-by: Ken Rockot <rockot@google.com>
Cr-Commit-Position: refs/heads/master@{#793129}

content/test/fuzzer/code_cache_host_mojolpm_fuzzer.cc

@@ -262,7 +262,6 @@ class CodeCacheHostTestcase : public mojolpm::TestcaseBase {
   ~CodeCacheHostTestcase() override;
   bool IsFinished() override;
   void NextAction() override;
-  int NextResponseIndex(mojolpm::TypeId type) override;
 };
 
 CodeCacheHostTestcase::CodeCacheHostTestcase(
@@ -316,13 +315,6 @@ void CodeCacheHostTestcase::NextAction() {
   }
 }
 
-int CodeCacheHostTestcase::NextResponseIndex(mojolpm::TypeId) {
-  // CodeCacheHost doesn't take any pending_receiver<X> parameters, so
-  // we do not need to handle fuzzing response types at all in this
-  // fuzzer.
-  return 0;
-}
-
 void run_testcase(
     content::CodeCacheHostFuzzerContext* context,
     const content::fuzzing::code_cache_host::proto::Testcase* testcase,

mojo/public/tools/bindings/generators/mojolpm_templates/mojolpm.cc.tmpl

@@ -90,55 +90,10 @@ class {{interface.name}}Impl : public {{mojom_type}} {
 {{ util.add_instance(kind, name, False)|indent(2, True) }}
 {%-     endfor %}
     mojolpmdbg("{{interface.name}}Impl.{{method.name}}\n");
-    mojolpm::GetContext()->NextAction();
 {%-       if method.response_parameters != None %}
-    auto mojolpm_response = mojolpm::GetContext()->GetInstance<
-      {{proto_type}}::{{interface.name}}_{{method.name}}Response>(
-        mojolpm::GetContext()->NextResponseIndex(
-          mojolpm::type_id<{{mojom_type}}>()));
-
-    if (!mojolpm_response) {
-      return;
-    }
-
-    bool mojolpm_result = true;
-{%-       for param in method.response_parameters %}
-{%-         set name = param.name|camel_to_under %}
-{%-         set kind = param.kind %}
-{%-         set param_mojom_type = kind|cpp_wrapper_type(add_same_module_namespaces=true) %}
-{%-         set param_maybe_mojom_type = kind|cpp_wrapper_type(add_same_module_namespaces=true, ignore_nullable=True) %}
-  {{param_mojom_type}} local_{{name}};
-{%-         if kind|is_nullable_kind %}
-  {{param_maybe_mojom_type}} local_maybe_{{name}};
-{%-         endif %}
-{%-       endfor %}
-
-{%-       for param in method.response_parameters -%}
-{%-         set name = param.name|camel_to_under %}
-{%-         set kind = param.kind %}
-{%-         if not kind|is_nullable_kind %}
-    mojolpm_result &= FromProto(mojolpm_response->m_{{name}}(), local_{{name}});
-    mojolpmdbg("{{name}} %i\n", mojolpm_result);
-{%-         else %}
-    if (FromProto(mojolpm_response->m_{{name}}(), local_maybe_{{name}})) {
-      local_{{name}} = std::move(local_maybe_{{name}});
-    }
-{%-         endif %}
-{%-       endfor %}
-    if (mojolpm_result) {
-      std::move(callback).Run(
-{%-       for param in method.response_parameters -%}
-{%-         set name = param.name|camel_to_under %}
-{%-         set kind = param.kind %}
-{%-         if kind|is_interface_kind or kind|is_associated_kind %}
-      {{kind|cpp_wrapper_param_type(add_same_module_namespaces=true)}}(std::move(local_{{name}})){{ ',' if not loop.last }}
-{%-         else %}
-      std::move(local_{{name}}){{ ',' if not loop.last }}
-{%-         endif %}
-{%-       endfor -%}
-);
-    }
+    mojolpm::GetContext()->AddInstance<{{mojom_type}}::{{method.name}}Callback>(std::move(callback));
 {%-       endif %}
+    mojolpm::GetContext()->NextAction();
   }
 {%-   endfor %}
 };
@@ -338,17 +293,13 @@ bool HandleAssociatedRemoteCall(const {{proto_type}}::AssociatedRemoteCall& inpu
   return result;
 }
 
-bool AddResponse(
+bool HandleResponse(
     const {{proto_type}}::ReceiverResponse& input) {
   bool result = true;
   switch (input.response_case()) {
 {%-     for method in interface.methods %}
-    case {{proto_type}}::ReceiverResponse::k{{("m_" ~ method.name)|under_to_camel(digits_split=True) ~ "Response"}}: {
-      {{proto_type}}::{{interface.name}}_{{method.name}}Response response_copy;
-      response_copy.CopyFrom(input.m_{{method.name|camel_to_under}}_response());
-      mojolpm::GetContext()->AddInstance<
-        {{proto_type}}::{{interface.name}}_{{method.name}}Response>(
-          std::move(response_copy));
+    case {{proto_type}}::ReceiverResponse::k{{("m_" ~ method.name ~ "_response")|under_to_camel(digits_split=True)}}: {
+      result = HandleResponse(input.id(), input.{{("m" ~ method.name ~ "_response")|camel_to_under}}());
     } break;
 {%-     endfor %}
 
@@ -448,6 +399,64 @@ bool HandleAssociatedRemoteCall(
     uint32_t instance_id, const {{proto_type}}::{{interface.name}}_{{method.name}}& input) {
   mojolpmdbg("HandleAssociatedRemoteCall({{interface.name}}::{{method.name}})\n");
   return HandleCall<::mojo::AssociatedRemote<{{mojom_type}}>>(instance_id, input);
+}
+
+bool HandleResponse(
+    uint32_t callback_id, const {{proto_type}}::{{interface.name}}_{{method.name}}Response& input) {
+  mojolpmdbg("HandleResponse({{interface.name}}::{{method.name}})\n");
+{%-       if method.response_parameters == None %}
+  return true;
+{%-     else %}
+  auto mojolpm_callback = mojolpm::GetContext()->GetAndRemoveInstance<
+    {{mojom_type}}::{{method.name}}Callback>(callback_id);
+
+  if (!mojolpm_callback) {
+    return true;
+  }
+
+  bool mojolpm_result = true;
+{%-         for param in method.response_parameters %}
+{%-           set name = param.name|camel_to_under %}
+{%-           set kind = param.kind %}
+{%-           set param_mojom_type = kind|cpp_wrapper_type(add_same_module_namespaces=true) %}
+{%-           set param_maybe_mojom_type = kind|cpp_wrapper_type(add_same_module_namespaces=true, ignore_nullable=True) %}
+  {{param_mojom_type}} local_{{name}};
+{%-           if kind|is_nullable_kind %}
+  {{param_maybe_mojom_type}} local_maybe_{{name}};
+{%-           endif %}
+{%-         endfor %}
+
+{%-         for param in method.response_parameters -%}
+{%-           set name = param.name|camel_to_under %}
+{%-           set kind = param.kind %}
+{%-           if not kind|is_nullable_kind %}
+  mojolpm_result &= FromProto(input.m_{{name}}(), local_{{name}});
+  mojolpmdbg("{{name}} %i\n", mojolpm_result);
+{%-           else %}
+  if (FromProto(input.m_{{name}}(), local_maybe_{{name}})) {
+    local_{{name}} = std::move(local_maybe_{{name}});
+  }
+{%-           endif %}
+{%-         endfor %}
+  if (mojolpm_result) {
+    std::move(*mojolpm_callback).Run(
+{%-         for param in method.response_parameters -%}
+{%-           set name = param.name|camel_to_under %}
+{%-           set kind = param.kind %}
+{%-           if kind|is_interface_kind or kind|is_associated_kind %}
+      {{kind|cpp_wrapper_param_type(add_same_module_namespaces=true)}}(std::move(local_{{name}})){{ ',' if not loop.last }}
+{%-           else %}
+      std::move(local_{{name}}){{ ',' if not loop.last }}
+{%-           endif %}
+{%-         endfor -%}
+);
+  } else {
+    mojolpm::GetContext()->AddInstance<
+      {{mojom_type}}::{{method.name}}Callback>(callback_id, std::move(*mojolpm_callback));
+  }
+
+  return mojolpm_result;
+{%-       endif %}
 }{{"\n"-}}
 {%-     endfor %}
 {%-   endif %}

mojo/public/tools/bindings/generators/mojolpm_templates/mojolpm.h.tmpl

@@ -183,7 +183,7 @@ bool HandleRemoteCall(
 bool HandleAssociatedRemoteCall(
   const {{proto_type}}::AssociatedRemoteCall& input);
 
-bool AddResponse(
+bool HandleResponse(
   const {{proto_type}}::ReceiverResponse& response);{{"\n"-}}
 {%-     for method in interface.methods %}
 bool HandleRemoteCall(
@@ -192,7 +192,11 @@ bool HandleRemoteCall(
 
 bool HandleAssociatedRemoteCall(
   uint32_t instance_id,
-  const {{proto_type}}::{{interface.name}}_{{method.name}}& input);{{"\n"-}}
+  const {{proto_type}}::{{interface.name}}_{{method.name}}& input);
+
+bool HandleResponse(
+  uint32_t callback_id,
+  const {{proto_type}}::{{interface.name}}_{{method.name}}Response& input);{{"\n"-}}
 {%-     endfor %}
 {%-   endif %}
 {%- endfor -%}

mojo/public/tools/bindings/generators/mojolpm_templates/mojolpm.proto.tmpl

@@ -339,6 +339,8 @@ message {{interface.name}} {
   }
 
   message ReceiverResponse {
+    required uint32 id = 1;
+
     oneof response {
 {%-     for method in interface.methods %}
       {{interface.name}}_{{method.name}}Response m_{{method.name|camel_to_under}}_response = {{loop.index + 2}};

mojo/public/tools/fuzzers/mojolpm.cc

@@ -64,13 +64,6 @@ void Context::PostNextAction() {
   }
 }
 
-int Context::NextResponseIndex(TypeId type_id) {
-  if (testcase_) {
-    return testcase_->NextResponseIndex(type_id);
-  }
-  return 0;
-}
-
 Context* g_context = nullptr;
 
 Context* GetContext() {

mojo/public/tools/fuzzers/mojolpm.h

@@ -50,7 +50,6 @@ class TestcaseBase {
   virtual ~TestcaseBase() = default;
   virtual bool IsFinished() = 0;
   virtual void NextAction() = 0;
-  virtual int NextResponseIndex(TypeId type) = 0;
 };
 
 class Context {
@@ -188,7 +187,6 @@ class Context {
   bool IsFinished();
   void NextAction();
   void PostNextAction();
-  int NextResponseIndex(TypeId type_id);
 
   scoped_refptr<base::SequencedTaskRunner> task_runner() const {
     return task_runner_;