MojoLPM: Remove re-entrancy and use of RunUntilIdle.

The existing example fuzzer code used RunUntilIdle to avoid runloop
recursion issues caused by re-entrancy in the scheduling of fuzzer
tasks. This change removes the re-entrancy and refactors out the use
of RunUntilIdle, working towards refactoring out the common code
duplication between fuzzers that's currently necessary.

Bug: 1076336
Change-Id: I3729d20264638c00c14c3a8149395e3f22e90089
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2329531
Commit-Queue: Mark Brand <markbrand@google.com>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#799545}

content/test/data/fuzzer_corpus/code_cache_host_mojolpm_fuzzer/did_generate_cacheable_metadata_origin_a.textproto

@@ -6,7 +6,7 @@ actions {
   }
 }
 actions {
-  code_cache_host_call {
+  code_cache_host_remote_action {
     id: 1
     m_did_generate_cacheable_metadata {
       m_cache_type: CodeCacheType_kJavascript

content/test/data/fuzzer_corpus/code_cache_host_mojolpm_fuzzer/did_generate_cacheable_metadata_origin_b.textproto

@@ -6,7 +6,7 @@ actions {
   }
 }
 actions {
-  code_cache_host_call {
+  code_cache_host_remote_action {
     id: 1
     m_did_generate_cacheable_metadata {
       m_cache_type: CodeCacheType_kJavascript

content/test/fuzzer/code_cache_host_mojolpm_fuzzer.cc

@@ -54,59 +54,16 @@ class ContentFuzzerEnvironment {
     fuzzer_thread_.StartAndWaitForTesting();
   }
 
-  void RunThreadUntilIdle(
-      const scoped_refptr<base::SingleThreadTaskRunner>& task_runner) {
-    if (task_runner->RunsTasksInCurrentSequence()) {
-      base::RunLoop(base::RunLoop::Type::kNestableTasksAllowed).RunUntilIdle();
-    } else {
-      base::WaitableEvent thread_idle(
-          base::WaitableEvent::ResetPolicy::MANUAL,
-          base::WaitableEvent::InitialState::NOT_SIGNALED);
-      task_runner->PostTask(
-          FROM_HERE,
-          base::BindOnce(
-              [](base::WaitableEvent* thread_idle) {
-                base::RunLoop(base::RunLoop::Type::kNestableTasksAllowed)
-                    .RunUntilIdle();
-                thread_idle->Signal();
-              },
-              base::Unretained(&thread_idle)));
-      thread_idle.Wait();
-    }
-  }
-
-  void RunUntilIdle() { RunThreadUntilIdle(fuzzer_thread_.task_runner()); }
-
-  void RunUIThreadUntilIdle() { RunThreadUntilIdle(ui_task_runner()); }
-
-  void RunIOThreadUntilIdle() { RunThreadUntilIdle(io_task_runner()); }
-
-  scoped_refptr<base::SequencedTaskRunner> task_runner() {
+  scoped_refptr<base::SequencedTaskRunner> fuzzer_task_runner() {
     return fuzzer_thread_.task_runner();
   }
 
-  scoped_refptr<base::SingleThreadTaskRunner> io_task_runner() {
-    if (!io_task_runner_) {
-      io_task_runner_ = base::CreateSingleThreadTaskRunner({BrowserThread::IO});
-    }
-    return io_task_runner_;
-  }
-
-  scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner() {
-    if (!ui_task_runner_) {
-      ui_task_runner_ = base::CreateSingleThreadTaskRunner({BrowserThread::UI});
-    }
-    return ui_task_runner_;
-  }
-
  private:
   base::AtExitManager at_exit_manager_;
   std::unique_ptr<base::FieldTrialList> field_trial_list_;
   base::test::ScopedFeatureList scoped_feature_list_;
   base::Thread fuzzer_thread_;
   BrowserTaskEnvironment task_environment_;
-  scoped_refptr<base::SingleThreadTaskRunner> io_task_runner_;
-  scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner_;
   TestContentClientInitializer content_client_initializer_;
 };
 
@@ -116,28 +73,8 @@ ContentFuzzerEnvironment& SingletonEnvironment() {
   return g_environment;
 }
 
-scoped_refptr<base::SequencedTaskRunner> GetTaskRunner() {
-  return SingletonEnvironment().task_runner();
-}
-
-scoped_refptr<base::SingleThreadTaskRunner> GetIOTaskRunner() {
-  return SingletonEnvironment().io_task_runner();
-}
-
-scoped_refptr<base::SingleThreadTaskRunner> GetUITaskRunner() {
-  return SingletonEnvironment().ui_task_runner();
-}
-
-void RunUntilIdle() {
-  SingletonEnvironment().RunUntilIdle();
-}
-
-void RunIOThreadUntilIdle() {
-  SingletonEnvironment().RunIOThreadUntilIdle();
-}
-
-void RunUIThreadUntilIdle() {
-  SingletonEnvironment().RunUIThreadUntilIdle();
+scoped_refptr<base::SequencedTaskRunner> GetFuzzerTaskRunner() {
+  return SingletonEnvironment().fuzzer_task_runner();
 }
 
 class CodeCacheHostFuzzerContext : public mojolpm::Context {
@@ -152,20 +89,14 @@ class CodeCacheHostFuzzerContext : public mojolpm::Context {
         kOriginB(url::Origin::Create(GURL("http://bbb.com/"))),
         kOriginOpaque(url::Origin::Create(GURL("opaque"))),
         kOriginEmpty(url::Origin::Create(GURL("file://this_becomes_empty"))),
-        browser_context_() {}
-
-  void InitializeServices() {
-    if (!initialized_) {
-      base::PostTask(
+        browser_context_() {
+    base::RunLoop run_loop(base::RunLoop::Type::kNestableTasksAllowed);
+    base::PostTaskAndReply(
         FROM_HERE, {BrowserThread::UI},
         base::BindOnce(&CodeCacheHostFuzzerContext::InitializeOnUIThread,
-                         base::Unretained(this)));
-      RunUIThreadUntilIdle();
-
-      RunUntilIdle();
-
-      initialized_ = true;
-    }
+                       base::Unretained(this)),
+        run_loop.QuitClosure());
+    run_loop.Run();
   }
 
   void InitializeOnUIThread() {
@@ -180,41 +111,23 @@ class CodeCacheHostFuzzerContext : public mojolpm::Context {
                                               65536);
   }
 
-  void CleanupServices() {
-    base::PostTask(
-        FROM_HERE, {BrowserThread::UI},
-        base::BindOnce(&CodeCacheHostFuzzerContext::CleanupOnUIThread,
-                       base::Unretained(this)));
-    RunUIThreadUntilIdle();
-
-    RunUntilIdle();
-
-    initialized_ = false;
-  }
-
-  void CleanupOnUIThread() {}
-
   void AddCodeCacheHostImpl(
       uint32_t id,
       int renderer_id,
       const Origin& origin,
-      mojo::PendingReceiver<::blink::mojom::CodeCacheHost>&& receiver,
-      base::WaitableEvent* receiver_bound) {
+      mojo::PendingReceiver<::blink::mojom::CodeCacheHost>&& receiver) {
     code_cache_hosts_[renderer_id] = std::make_unique<CodeCacheHostImpl>(
         renderer_id, cache_storage_context_, generated_code_cache_context_,
         std::move(receiver));
-
-    receiver_bound->Signal();
   }
 
   void AddCodeCacheHost(
       uint32_t id,
       int renderer_id,
-      content::fuzzing::code_cache_host::proto::NewCodeCacheHost::OriginId
+      content::fuzzing::code_cache_host::proto::NewCodeCacheHostAction::OriginId
           origin_id) {
     mojo::Remote<::blink::mojom::CodeCacheHost> remote;
     auto receiver = remote.BindNewPipeAndPassReceiver();
-    base::WaitableEvent receiver_bound;
 
     const Origin* origin = &kOriginA;
     if (origin_id == 1) {
@@ -225,13 +138,14 @@ class CodeCacheHostFuzzerContext : public mojolpm::Context {
       origin = &kOriginEmpty;
     }
 
-    GetUITaskRunner()->PostTask(
-        FROM_HERE,
+    base::RunLoop run_loop(base::RunLoop::Type::kNestableTasksAllowed);
+    base::PostTaskAndReply(
+        FROM_HERE, {BrowserThread::UI},
         base::BindOnce(&CodeCacheHostFuzzerContext::AddCodeCacheHostImpl,
                        base::Unretained(this), id, renderer_id, *origin,
-                       std::move(receiver), base::Unretained(&receiver_bound)));
-
-    receiver_bound.Wait();
+                       std::move(receiver)),
+        run_loop.QuitClosure());
+    run_loop.Run();
 
     AddInstance(id, std::move(remote));
   }
@@ -239,8 +153,6 @@ class CodeCacheHostFuzzerContext : public mojolpm::Context {
  private:
   TestBrowserContext browser_context_;
 
-  bool initialized_ = false;
-
   scoped_refptr<CacheStorageContextImpl> cache_storage_context_;
   scoped_refptr<GeneratedCodeCacheContext> generated_code_cache_context_;
 
@@ -295,17 +207,23 @@ void CodeCacheHostTestcase::NextAction() {
               action.new_code_cache_host().origin_id());
         } break;
 
-        case content::fuzzing::code_cache_host::proto::Action::kRunUntilIdle: {
-          if (action.run_until_idle().id()) {
-            content::RunUIThreadUntilIdle();
+        case content::fuzzing::code_cache_host::proto::Action::kRunThread: {
+          if (action.run_thread().id()) {
+            base::RunLoop run_loop(base::RunLoop::Type::kNestableTasksAllowed);
+            base::PostTask(FROM_HERE, {content::BrowserThread::UI},
+                           run_loop.QuitClosure());
+            run_loop.Run();
           } else {
-            content::RunIOThreadUntilIdle();
+            base::RunLoop run_loop(base::RunLoop::Type::kNestableTasksAllowed);
+            base::PostTask(FROM_HERE, {content::BrowserThread::IO},
+                           run_loop.QuitClosure());
+            run_loop.Run();
           }
         } break;
 
         case content::fuzzing::code_cache_host::proto::Action::
-            kCodeCacheHostCall: {
-          mojolpm::HandleRemoteCall(action.code_cache_host_call());
+            kCodeCacheHostRemoteAction: {
+          mojolpm::HandleRemoteAction(action.code_cache_host_remote_action());
         } break;
 
         case content::fuzzing::code_cache_host::proto::Action::ACTION_NOT_SET:
@@ -315,28 +233,36 @@ void CodeCacheHostTestcase::NextAction() {
   }
 }
 
-void run_testcase(
+void NextAction(content::CodeCacheHostFuzzerContext* context,
+                base::RepeatingClosure quit_closure) {
+  if (!context->IsFinished()) {
+    context->NextAction();
+    content::GetFuzzerTaskRunner()->PostTask(
+        FROM_HERE, base::BindOnce(NextAction, base::Unretained(context),
+                                  std::move(quit_closure)));
+  } else {
+    content::GetFuzzerTaskRunner()->PostTask(FROM_HERE,
+                                             std::move(quit_closure));
+  }
+}
+
+void RunTestcase(
     content::CodeCacheHostFuzzerContext* context,
-    const content::fuzzing::code_cache_host::proto::Testcase* testcase,
-    base::RepeatingClosure&& quit_closure) {
+    const content::fuzzing::code_cache_host::proto::Testcase* testcase) {
   mojo::Message message;
   auto dispatch_context =
       std::make_unique<mojo::internal::MessageDispatchContext>(&message);
 
   CodeCacheHostTestcase cch_testcase(*context, *testcase);
-  context->StartTestcase(&cch_testcase, content::GetTaskRunner());
+  context->StartTestcase(&cch_testcase, content::GetFuzzerTaskRunner());
 
-  while (!context->IsFinished()) {
-    context->NextAction();
-    content::RunUntilIdle();
-  }
-
-  content::RunIOThreadUntilIdle();
-  content::RunUIThreadUntilIdle();
+  base::RunLoop fuzzer_run_loop(base::RunLoop::Type::kNestableTasksAllowed);
+  content::GetFuzzerTaskRunner()->PostTask(
+      FROM_HERE, base::BindOnce(NextAction, base::Unretained(context),
+                                fuzzer_run_loop.QuitClosure()));
+  fuzzer_run_loop.Run();
 
   context->EndTestcase();
-
-  content::GetTaskRunner()->PostTask(FROM_HERE, std::move(quit_closure));
 }
 
 DEFINE_BINARY_PROTO_FUZZER(
@@ -347,18 +273,14 @@ DEFINE_BINARY_PROTO_FUZZER(
   }
 
   content::CodeCacheHostFuzzerContext context;
-  context.InitializeServices();
   mojolpm::SetContext(&context);
 
-  base::RunLoop ui_nested_runloop{base::RunLoop::Type::kNestableTasksAllowed};
-  auto ui_nested_quit = ui_nested_runloop.QuitClosure();
-
-  content::GetTaskRunner()->PostTask(
+  base::RunLoop ui_run_loop(base::RunLoop::Type::kNestableTasksAllowed);
+  content::GetFuzzerTaskRunner()->PostTaskAndReply(
       FROM_HERE,
-      base::BindOnce(run_testcase, base::Unretained(&context),
-                     base::Unretained(&testcase), std::move(ui_nested_quit)));
-
-  ui_nested_runloop.Run();
+      base::BindOnce(RunTestcase, base::Unretained(&context),
+                     base::Unretained(&testcase)),
+      ui_run_loop.QuitClosure());
 
-  context.CleanupServices();
+  ui_run_loop.Run();
 }

content/test/fuzzer/code_cache_host_mojolpm_fuzzer.proto

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Message format for the MojoLPM fuzzer for the CodeCacheHost interface.
+
 syntax = "proto2";
 
 package content.fuzzing.code_cache_host.proto;
 
 import "third_party/blink/public/mojom/loader/code_cache.mojom.mojolpm.proto";
 
-message NewCodeCacheHost {
+// Bind a new CodeCacheHost remote
+message NewCodeCacheHostAction {
   enum OriginId {
     ORIGIN_A = 0;
     ORIGIN_B = 1;
@@ -17,7 +24,10 @@ message NewCodeCacheHost {
   required OriginId origin_id = 3;
 }
 
-message RunUntilIdle {
+// Run the specific sequence for (an indeterminate) period. This is not
+// intended to create a specific ordering, but to allow the fuzzer to delay a
+// later task until previous tasks have completed.
+message RunThreadAction {
   enum ThreadId {
     IO = 0;
     UI = 1;
@@ -26,18 +36,23 @@ message RunUntilIdle {
   required ThreadId id = 1;
 }
 
+// Actions that can be performed by the fuzzer.
 message Action {
   oneof action {
-    NewCodeCacheHost new_code_cache_host = 1;
-    RunUntilIdle run_until_idle = 2;
-    mojolpm.blink.mojom.CodeCacheHost.RemoteCall code_cache_host_call = 3;
+    NewCodeCacheHostAction new_code_cache_host = 1;
+    RunThreadAction run_thread = 2;
+    mojolpm.blink.mojom.CodeCacheHost.RemoteAction
+        code_cache_host_remote_action = 3;
   }
 }
 
+// Sequence provides a level of indirection which allows Testcase to compactly
+// express repeated sequences of actions.
 message Sequence {
   repeated uint32 action_indexes = 1 [packed = true];
 }
 
+// Testcase is the top-level message type interpreted by the fuzzer.
 message Testcase {
   repeated Action actions = 1;
   repeated Sequence sequences = 2;

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

@@ -93,7 +93,6 @@ class {{interface.name}}Impl : public {{mojom_type}} {
 {%-       if method.response_parameters != None %}
     mojolpm::GetContext()->AddInstance<{{mojom_type}}::{{method.name}}Callback>(std::move(callback));
 {%-       endif %}
-    mojolpm::GetContext()->NextAction();
   }
 {%-   endfor %}
 };
@@ -251,16 +250,16 @@ bool ToProto(::mojo::PendingAssociatedReceiver<{{mojom_type}}>&& input,
 {%-   set mojom_type = interface|get_qualified_name_for_kind(flatten_nested_kind=True) %}
 {%-   set proto_type = "::mojolpm" ~ (interface|get_qualified_name_for_kind(flatten_nested_kind=True)) %}
 {%-   if interface.methods %}
-bool HandleRemoteCall(const {{proto_type}}::RemoteCall& input) {
+bool HandleRemoteAction(const {{proto_type}}::RemoteAction& input) {
   bool result = true;
 
   switch (input.method_case()) {
 {%-     for method in interface.methods %}
-    case {{proto_type}}::RemoteCall::k{{("m_" ~ method.name)|under_to_camel(digits_split=True)}}: {
+    case {{proto_type}}::RemoteAction::k{{("m_" ~ method.name)|under_to_camel(digits_split=True)}}: {
       result = HandleRemoteCall(input.id(), input.{{("m" ~ method.name)|camel_to_under}}());
     } break;
 {%-     endfor %}
-    case {{proto_type}}::RemoteCall::kReset: {
+    case {{proto_type}}::RemoteAction::kReset: {
       mojolpm::GetContext()->GetAndRemoveInstance<::mojo::Remote<{{mojom_type}}>>(input.id());
     } break;
 
@@ -272,16 +271,16 @@ bool HandleRemoteCall(const {{proto_type}}::RemoteCall& input) {
   return result;
 }
 
-bool HandleAssociatedRemoteCall(const {{proto_type}}::AssociatedRemoteCall& input) {
+bool HandleAssociatedRemoteAction(const {{proto_type}}::AssociatedRemoteAction& input) {
   bool result = true;
 
   switch (input.method_case()) {
 {%-     for method in interface.methods %}
-    case {{proto_type}}::AssociatedRemoteCall::k{{("m_" ~ method.name)|under_to_camel(digits_split=True)}}: {
+    case {{proto_type}}::AssociatedRemoteAction::k{{("m_" ~ method.name)|under_to_camel(digits_split=True)}}: {
       result = HandleAssociatedRemoteCall(input.id(), input.{{("m" ~ method.name)|camel_to_under}}());
     } break;
 {%-     endfor %}
-    case {{proto_type}}::AssociatedRemoteCall::kReset: {
+    case {{proto_type}}::AssociatedRemoteAction::kReset: {
       mojolpm::GetContext()->GetAndRemoveInstance<::mojo::AssociatedRemote<{{mojom_type}}>>(input.id());
     } break;
 
@@ -293,12 +292,12 @@ bool HandleAssociatedRemoteCall(const {{proto_type}}::AssociatedRemoteCall& inpu
   return result;
 }
 
-bool HandleResponse(
-    const {{proto_type}}::ReceiverResponse& input) {
+bool HandleReceiverAction(
+    const {{proto_type}}::ReceiverAction& input) {
   bool result = true;
   switch (input.response_case()) {
 {%-     for method in interface.methods %}
-    case {{proto_type}}::ReceiverResponse::k{{("m_" ~ method.name ~ "_response")|under_to_camel(digits_split=True)}}: {
+    case {{proto_type}}::ReceiverAction::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 %}
@@ -326,7 +325,6 @@ static void {{interface.name}}_{{method.name}}Callback(
 {{ util.add_instance(kind, 'param_' ~ name, False) }}
 {%-         endfor %}
   mojolpmdbg("{{interface.name}}.{{method.name}}Callback\n");
-  mojolpm::GetContext()->NextAction();
 }{{"\n"-}}
 {%-       endif %}
 template <typename T>

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

@@ -177,14 +177,14 @@ bool ToProto(
 {%-   set mojom_type = interface|get_qualified_name_for_kind(flatten_nested_kind=True) %}
 {%-   set proto_type = "::mojolpm" ~ (interface|get_qualified_name_for_kind(flatten_nested_kind=True)) %}
 {%-   if interface.methods %}
-bool HandleRemoteCall(
-  const {{proto_type}}::RemoteCall& input);
+bool HandleRemoteAction(
+  const {{proto_type}}::RemoteAction& input);
 
-bool HandleAssociatedRemoteCall(
-  const {{proto_type}}::AssociatedRemoteCall& input);
+bool HandleAssociatedRemoteAction(
+  const {{proto_type}}::AssociatedRemoteAction& input);
 
-bool HandleResponse(
-  const {{proto_type}}::ReceiverResponse& response);{{"\n"-}}
+bool HandleReceiverAction(
+  const {{proto_type}}::ReceiverAction& input);{{"\n"-}}
 {%-     for method in interface.methods %}
 bool HandleRemoteCall(
   uint32_t instance_id,

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

@@ -316,7 +316,7 @@ message {{interface.name}} {
 {%-   endfor%}
 
 {%-   if interface.methods|length %}
-  message RemoteCall {
+  message RemoteAction {
     required uint32 id = 1;
 
     oneof method {
@@ -327,7 +327,7 @@ message {{interface.name}} {
     }
   }
 
-  message AssociatedRemoteCall {
+  message AssociatedRemoteAction {
     required uint32 id = 1;
 
     oneof method {
@@ -338,7 +338,7 @@ message {{interface.name}} {
     }
   }
 
-  message ReceiverResponse {
+  message ReceiverAction {
     required uint32 id = 1;
 
     oneof response {