[heap profiler] Refactor: make use of PostTaskWithTraitsAndReply

Get rid of an extra thread.

BUG=923459

Change-Id: I461ec8e437f0520274650a7922c29154fd57f23c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1573171
Commit-Queue: Alexei Filippov <alph@chromium.org>
Reviewed-by: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#652175}

components/services/heap_profiling/connection_manager.cc

@@ -7,7 +7,7 @@
 #include "base/bind.h"
 #include "base/json/string_escape.h"
 #include "base/metrics/histogram_macros.h"
-#include "base/threading/sequenced_task_runner_handle.h"
+#include "base/task/post_task.h"
 #include "components/services/heap_profiling/json_exporter.h"
 #include "components/services/heap_profiling/public/cpp/client.h"
 
@@ -85,8 +85,7 @@ struct ConnectionManager::Connection {
   uint32_t sampling_rate = 1;
 };
 
-ConnectionManager::ConnectionManager() : blocking_thread_("Blocking thread") {
-  blocking_thread_.Start();
+ConnectionManager::ConnectionManager() {
   metrics_timer_.Start(
       FROM_HERE, base::TimeDelta::FromHours(24),
       base::Bind(&ConnectionManager::ReportMetrics, base::Unretained(this)));
@@ -240,7 +239,7 @@ void ConnectionManager::HeapProfileRetrieved(
   }
 
   DCHECK(success);
-  DoDumpOneProcessForTracing(tracking, pid, process_type,
+  DoDumpOneProcessForTracing(std::move(tracking), pid, process_type,
                              strip_path_from_mapped_files, sampling_rate,
                              success, std::move(counts), std::move(context_map),
                              std::move(string_map));
@@ -289,55 +288,46 @@ void ConnectionManager::DoDumpOneProcessForTracing(
   ExportMemoryMapsAndV2StackTraceToJSON(&params, oss);
   std::string reply = oss.str();
   size_t reply_size = reply.size();
-
   next_id_ = params.next_id;
 
-  using FinishedCallback =
-      base::OnceCallback<void(mojo::ScopedSharedBufferHandle)>;
-  FinishedCallback finished_callback = base::BindOnce(
-      [](std::string reply, base::ProcessId pid,
-         scoped_refptr<DumpProcessesForTracingTracking> tracking,
-         mojo::ScopedSharedBufferHandle buffer) {
-        if (!buffer.is_valid()) {
-          DLOG(ERROR) << "Could not create Mojo shared buffer";
-        } else {
-          mojo::ScopedSharedBufferMapping mapping = buffer->Map(reply.size());
-          if (!mapping) {
-            DLOG(ERROR) << "Could not map Mojo shared buffer";
-          } else {
-            memcpy(mapping.get(), reply.c_str(), reply.size());
-
-            memory_instrumentation::mojom::SharedBufferWithSizePtr result =
-                memory_instrumentation::mojom::SharedBufferWithSize::New();
-            result->buffer = std::move(buffer);
-            result->size = reply.size();
-            result->pid = pid;
-            tracking->results.push_back(std::move(result));
-          }
-        }
-
-        // When all responses complete, issue done callback.
-        tracking->waiting_responses--;
-        if (tracking->waiting_responses == 0)
-          std::move(tracking->callback).Run(std::move(tracking->results));
-      },
-      std::move(reply), pid, tracking);
-
-  blocking_thread_.task_runner()->PostTask(
-      FROM_HERE, base::BindOnce(
-                     [](size_t size,
-                        scoped_refptr<base::SingleThreadTaskRunner> task_runner,
-                        FinishedCallback callback) {
-                       // This call will send a synchronous IPC to the browser
-                       // process.
-                       mojo::ScopedSharedBufferHandle buffer =
-                           mojo::SharedBufferHandle::Create(size);
-                       task_runner->PostTask(FROM_HERE,
-                                             base::BindOnce(std::move(callback),
-                                                            std::move(buffer)));
-                     },
-                     reply_size, base::ThreadTaskRunnerHandle::Get(),
-                     std::move(finished_callback)));
+  base::PostTaskWithTraitsAndReplyWithResult(
+      FROM_HERE,
+      {base::TaskPriority::BEST_EFFORT, base::MayBlock(),
+       base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN},
+      base::BindOnce(
+          [](size_t size) {
+            // This call sends a synchronous IPC to the browser process.
+            return mojo::SharedBufferHandle::Create(size);
+          },
+          reply_size),
+      base::BindOnce(
+          [](std::string reply, base::ProcessId pid,
+             scoped_refptr<DumpProcessesForTracingTracking> tracking,
+             mojo::ScopedSharedBufferHandle buffer) {
+            if (!buffer.is_valid()) {
+              DLOG(ERROR) << "Could not create Mojo shared buffer";
+            } else {
+              mojo::ScopedSharedBufferMapping mapping =
+                  buffer->Map(reply.size());
+              if (!mapping) {
+                DLOG(ERROR) << "Could not map Mojo shared buffer";
+              } else {
+                memcpy(mapping.get(), reply.c_str(), reply.size());
+
+                memory_instrumentation::mojom::SharedBufferWithSizePtr result =
+                    memory_instrumentation::mojom::SharedBufferWithSize::New();
+                result->buffer = std::move(buffer);
+                result->size = reply.size();
+                result->pid = pid;
+                tracking->results.push_back(std::move(result));
+              }
+            }
+
+            // When all responses complete, issue done callback.
+            if (--tracking->waiting_responses == 0)
+              std::move(tracking->callback).Run(std::move(tracking->results));
+          },
+          std::move(reply), pid, std::move(tracking)));
 }
 
 }  // namespace heap_profiling

components/services/heap_profiling/connection_manager.h

@@ -122,13 +122,7 @@ class ConnectionManager {
   // Every 24-hours, reports the types of profiled processes.
   base::RepeatingTimer metrics_timer_;
 
-  // To avoid deadlock, synchronous calls to the browser are made on a dedicated
-  // thread that does nothing else. Both the IO thread and connection-specific
-  // threads could potentially be processing messages from the browser process,
-  // which in turn could be blocked on sending more messages over the pipe.
-  base::Thread blocking_thread_;
-
-  // Must be last.
+  // Must be the last.
   base::WeakPtrFactory<ConnectionManager> weak_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(ConnectionManager);

components/services/heap_profiling/public/cpp/client.cc

@@ -6,9 +6,7 @@
 
 #include "base/allocator/allocator_interception_mac.h"
 #include "base/bind.h"
-#include "base/single_thread_task_runner.h"
 #include "base/task/post_task.h"
-#include "base/task/task_traits.h"
 #include "base/trace_event/malloc_dump_provider.h"
 #include "build/build_config.h"
 #include "components/services/heap_profiling/public/cpp/sampling_profiler_wrapper.h"
@@ -20,23 +18,6 @@
 
 namespace heap_profiling {
 
-namespace {
-
-#if defined(OS_ANDROID) && BUILDFLAG(CAN_UNWIND_WITH_CFI_TABLE) && \
-    defined(OFFICIAL_BUILD)
-void EnsureCFIInitializedOnBackgroundThread(
-    scoped_refptr<base::SingleThreadTaskRunner> callback_task_runner,
-    base::OnceClosure callback) {
-  bool can_unwind =
-      base::trace_event::CFIBacktraceAndroid::GetInitializedInstance()
-          ->can_unwind_stack_frames();
-  DCHECK(can_unwind);
-  callback_task_runner->PostTask(FROM_HERE, std::move(callback));
-}
-#endif
-
-}  // namespace
-
 Client::Client() : sampling_profiler_(new SamplingProfilerWrapper()) {}
 
 Client::~Client() {
@@ -67,17 +48,18 @@ void Client::StartProfiling(mojom::ProfilingParamsPtr params) {
     defined(OFFICIAL_BUILD)
   // On Android the unwinder initialization requires file reading before
   // initializing shim. So, post task on background thread.
-  auto init_callback =
+  base::PostTaskWithTraitsAndReply(
+      FROM_HERE,
+      {base::TaskPriority::BEST_EFFORT, base::MayBlock(),
+       base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN},
+      base::BindOnce([]() {
+        bool can_unwind =
+            base::trace_event::CFIBacktraceAndroid::GetInitializedInstance()
+                ->can_unwind_stack_frames();
+        DCHECK(can_unwind);
+      }),
       base::BindOnce(&Client::StartProfilingInternal,
-                     weak_factory_.GetWeakPtr(), std::move(params));
-
-  auto background_task = base::BindOnce(&EnsureCFIInitializedOnBackgroundThread,
-                                        base::ThreadTaskRunnerHandle::Get(),
-                                        std::move(init_callback));
-  base::PostTaskWithTraits(FROM_HERE,
-                           {base::TaskPriority::BEST_EFFORT, base::MayBlock(),
-                            base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN},
-                           std::move(background_task));
+                     weak_factory_.GetWeakPtr(), std::move(params)));
 #else
   StartProfilingInternal(std::move(params));
 #endif