Move CompletedCallback type to CallStackProfileBuilder class

Bug: 851163
Change-Id: I2346efb2fc9c7ac5e0be425ac9484babd6b6544d
Reviewed-on: https://chromium-review.googlesource.com/1123657Reviewed-by: Mike Wittman <wittman@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Commit-Queue: Xi Cheng <chengx@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572348}

base/profiler/stack_sampling_profiler.h

@@ -14,7 +14,6 @@
 
 #include "base/atomicops.h"
 #include "base/base_export.h"
-#include "base/callback.h"
 #include "base/files/file_path.h"
 #include "base/macros.h"
 #include "base/strings/string16.h"
@@ -270,18 +269,6 @@ class BASE_EXPORT StackSamplingProfiler {
     DISALLOW_COPY_AND_ASSIGN(ProfileBuilder);
   };
 
-  // The callback type used to collect a completed profile. The passed |profile|
-  // is move-only. Other threads, including the UI thread, may block on callback
-  // completion so this should run as quickly as possible.
-  //
-  // IMPORTANT NOTE: The callback is invoked on a thread the profiler
-  // constructs, rather than on the thread used to construct the profiler, and
-  // thus the callback must be callable on any thread. For threads with message
-  // loops that create StackSamplingProfilers, posting a task to the message
-  // loop with the moved (i.e. std::move) profile is the thread-safe callback
-  // implementation.
-  using CompletedCallback = Callback<void(CallStackProfile)>;
-
   // Creates a profiler for the CURRENT thread. An optional |test_delegate| can
   // be supplied by tests. The caller must ensure that this object gets
   // destroyed before the current thread exits.

chrome/common/thread_profiler.cc

@@ -15,9 +15,7 @@
 #include "base/threading/sequence_local_storage_slot.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "chrome/common/stack_sampling_configuration.h"
-#include "components/metrics/call_stack_profile_builder.h"
 #include "components/metrics/call_stack_profile_metrics_provider.h"
-#include "components/metrics/call_stack_profile_params.h"
 #include "components/metrics/child_call_stack_profile_collector.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/common/service_names.mojom.h"
@@ -221,7 +219,7 @@ ThreadProfiler::ThreadProfiler(
     ScheduleNextPeriodicCollection();
 }
 
-StackSamplingProfiler::CompletedCallback ThreadProfiler::GetReceiverCallback(
+CallStackProfileBuilder::CompletedCallback ThreadProfiler::GetReceiverCallback(
     const CallStackProfileParams& profile_params) {
   // TODO(wittman): Simplify the approach to getting the profiler callback
   // across CallStackProfileMetricsProvider and
@@ -247,14 +245,14 @@ StackSamplingProfiler::CompletedCallback ThreadProfiler::GetReceiverCallback(
 
 // static
 void ThreadProfiler::ReceiveStartupProfile(
-    const StackSamplingProfiler::CompletedCallback& receiver_callback,
+    const CallStackProfileBuilder::CompletedCallback& receiver_callback,
     StackSamplingProfiler::CallStackProfile profile) {
   receiver_callback.Run(std::move(profile));
 }
 
 // static
 void ThreadProfiler::ReceivePeriodicProfile(
-    const StackSamplingProfiler::CompletedCallback& receiver_callback,
+    const CallStackProfileBuilder::CompletedCallback& receiver_callback,
     scoped_refptr<base::SingleThreadTaskRunner> owning_thread_task_runner,
     base::WeakPtr<ThreadProfiler> thread_profiler,
     StackSamplingProfiler::CallStackProfile profile) {

chrome/common/thread_profiler.h

@@ -15,6 +15,7 @@
 #include "base/threading/thread.h"
 #include "base/threading/thread_checker.h"
 #include "base/time/time.h"
+#include "components/metrics/call_stack_profile_builder.h"
 #include "components/metrics/call_stack_profile_params.h"
 
 namespace service_manager {
@@ -92,19 +93,19 @@ class ThreadProfiler {
           scoped_refptr<base::SingleThreadTaskRunner>());
 
   // Gets the completed callback for the ultimate receiver of the profile.
-  base::StackSamplingProfiler::CompletedCallback GetReceiverCallback(
+  CallStackProfileBuilder::CompletedCallback GetReceiverCallback(
       const metrics::CallStackProfileParams& profile_params);
 
-  // Receives |profile| from the StackSamplingProfiler and forwards it on to
+  // Receives |profile| from the CallStackProfileBuilder and forwards it on to
   // the original |receiver_callback|.  Note that we must obtain and bind the
   // original receiver callback prior to the start of collection because the
   // collection start time is currently recorded when obtaining the callback in
   // some collection scenarios. The implementation contains a TODO to fix this.
   static void ReceiveStartupProfile(
-      const base::StackSamplingProfiler::CompletedCallback& receiver_callback,
+      const CallStackProfileBuilder::CompletedCallback& receiver_callback,
       base::StackSamplingProfiler::CallStackProfile profile);
   static void ReceivePeriodicProfile(
-      const base::StackSamplingProfiler::CompletedCallback& receiver_callback,
+      const CallStackProfileBuilder::CompletedCallback& receiver_callback,
       scoped_refptr<base::SingleThreadTaskRunner> owning_thread_task_runner,
       base::WeakPtr<ThreadProfiler> thread_profiler,
       base::StackSamplingProfiler::CallStackProfile profile);

components/metrics/BUILD.gn

@@ -6,8 +6,6 @@ import("//testing/test.gni")
 
 static_library("metrics") {
   sources = [
-    "call_stack_profile_builder.cc",
-    "call_stack_profile_builder.h",
     "call_stack_profile_metrics_provider.cc",
     "call_stack_profile_metrics_provider.h",
     "clean_exit_beacon.cc",
@@ -107,7 +105,7 @@ static_library("metrics") {
   ]
 
   public_deps = [
-    ":call_stack_profile_params",
+    ":call_stack_profile",
     "//third_party/metrics_proto",
   ]
 
@@ -258,8 +256,10 @@ static_library("single_sample_metrics") {
   ]
 }
 
-source_set("call_stack_profile_params") {
+source_set("call_stack_profile") {
   sources = [
+    "call_stack_profile_builder.cc",
+    "call_stack_profile_builder.h",
     "call_stack_profile_params.h",
   ]
 
@@ -285,6 +285,7 @@ source_set("child_call_stacks") {
     "child_call_stack_profile_collector.h",
   ]
   deps = [
+    ":call_stack_profile",
     "//components/metrics/public/interfaces:call_stack_mojo_bindings",
     "//services/service_manager/public/cpp",
   ]
@@ -360,7 +361,7 @@ source_set("unit_tests") {
   ]
 
   deps = [
-    ":call_stack_profile_params",
+    ":call_stack_profile",
     ":child_call_stacks",
     ":component_metrics",
     ":metrics",

components/metrics/call_stack_profile_builder.cc

@@ -11,7 +11,7 @@
 using StackSamplingProfiler = base::StackSamplingProfiler;
 
 CallStackProfileBuilder::CallStackProfileBuilder(
-    const StackSamplingProfiler::CompletedCallback& callback)
+    const CompletedCallback& callback)
     : callback_(callback) {}
 
 CallStackProfileBuilder::~CallStackProfileBuilder() = default;

components/metrics/call_stack_profile_builder.h

@@ -9,6 +9,8 @@
 
 #include <map>
 
+#include "base/callback.h"
+
 // CallStackProfileBuilder builds a CallStackProfile from the collected sampling
 // data.
 //
@@ -20,8 +22,20 @@
 class CallStackProfileBuilder
     : public base::StackSamplingProfiler::ProfileBuilder {
  public:
-  CallStackProfileBuilder(
-      const base::StackSamplingProfiler::CompletedCallback& callback);
+  // The callback type used to collect a completed profile. The passed
+  // CallStackProfile is move-only. Other threads, including the UI thread, may
+  // block on callback completion so this should run as quickly as possible.
+  //
+  // IMPORTANT NOTE: The callback is invoked on a thread the profiler
+  // constructs, rather than on the thread used to construct the profiler, and
+  // thus the callback must be callable on any thread. For threads with message
+  // loops that create CallStackProfileBuilders, posting a task to the message
+  // loop with the moved (i.e. std::move) profile is the thread-safe callback
+  // implementation.
+  using CompletedCallback =
+      base::Callback<void(base::StackSamplingProfiler::CallStackProfile)>;
+
+  CallStackProfileBuilder(const CompletedCallback& callback);
 
   ~CallStackProfileBuilder() override;
 
@@ -43,7 +57,7 @@ class CallStackProfileBuilder
   std::map<uintptr_t, size_t> module_index_;
 
   // Callback made when sampling a profile completes.
-  const base::StackSamplingProfiler::CompletedCallback callback_;
+  const CompletedCallback callback_;
 
   DISALLOW_COPY_AND_ASSIGN(CallStackProfileBuilder);
 };

components/metrics/call_stack_profile_metrics_provider.cc

@@ -414,7 +414,7 @@ CallStackProfileMetricsProvider::CallStackProfileMetricsProvider() {}
 
 CallStackProfileMetricsProvider::~CallStackProfileMetricsProvider() {}
 
-StackSamplingProfiler::CompletedCallback
+CallStackProfileBuilder::CompletedCallback
 CallStackProfileMetricsProvider::GetProfilerCallbackForBrowserProcess(
     const CallStackProfileParams& params) {
   // Ignore the profile if the collection is disabled. If the collection state

components/metrics/call_stack_profile_metrics_provider.h

@@ -9,6 +9,7 @@
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/profiler/stack_sampling_profiler.h"
+#include "components/metrics/call_stack_profile_builder.h"
 #include "components/metrics/call_stack_profile_params.h"
 #include "components/metrics/metrics_provider.h"
 
@@ -36,10 +37,11 @@ class CallStackProfileMetricsProvider : public MetricsProvider {
   CallStackProfileMetricsProvider();
   ~CallStackProfileMetricsProvider() override;
 
-  // Returns a callback for use with StackSamplingProfiler that sets up
+  // Returns a callback for use with CallStackProfileBuilder that sets up
   // parameters for general browser process sampling. The callback should be
-  // immediately passed to the StackSamplingProfiler, and should not be reused.
-  static base::StackSamplingProfiler::CompletedCallback
+  // immediately passed to the CallStackProfileBuilder, and should not be
+  // reused.
+  static CallStackProfileBuilder::CompletedCallback
   GetProfilerCallbackForBrowserProcess(const CallStackProfileParams& params);
 
   // Provides completed stack profile to the metrics provider. Intended for use

components/metrics/call_stack_profile_metrics_provider_unittest.cc

@@ -481,7 +481,7 @@ TEST_F(CallStackProfileMetricsProviderTest,
                                 CallStackProfileParams::MAIN_THREAD,
                                 CallStackProfileParams::PROCESS_STARTUP,
                                 CallStackProfileParams::MAY_SHUFFLE);
-  base::StackSamplingProfiler::CompletedCallback callback =
+  CallStackProfileBuilder::CompletedCallback callback =
       CallStackProfileMetricsProvider::GetProfilerCallbackForBrowserProcess(
           params);
   provider.OnRecordingDisabled();
@@ -507,7 +507,7 @@ TEST_F(CallStackProfileMetricsProviderTest,
                                 CallStackProfileParams::MAIN_THREAD,
                                 CallStackProfileParams::PROCESS_STARTUP,
                                 CallStackProfileParams::MAY_SHUFFLE);
-  base::StackSamplingProfiler::CompletedCallback callback =
+  CallStackProfileBuilder::CompletedCallback callback =
       CallStackProfileMetricsProvider::GetProfilerCallbackForBrowserProcess(
           params);
   provider.OnRecordingDisabled();
@@ -534,7 +534,7 @@ TEST_F(CallStackProfileMetricsProviderTest,
                                 CallStackProfileParams::MAIN_THREAD,
                                 CallStackProfileParams::PROCESS_STARTUP,
                                 CallStackProfileParams::MAY_SHUFFLE);
-  base::StackSamplingProfiler::CompletedCallback callback =
+  CallStackProfileBuilder::CompletedCallback callback =
       CallStackProfileMetricsProvider::GetProfilerCallbackForBrowserProcess(
           params);
   provider.OnRecordingEnabled();

components/metrics/child_call_stack_profile_collector.cc

@@ -38,7 +38,7 @@ ChildCallStackProfileCollector::ChildCallStackProfileCollector() {}
 
 ChildCallStackProfileCollector::~ChildCallStackProfileCollector() {}
 
-base::StackSamplingProfiler::CompletedCallback
+CallStackProfileBuilder::CompletedCallback
 ChildCallStackProfileCollector::GetProfilerCallback(
     const CallStackProfileParams& params,
     base::TimeTicks profile_start_time) {

components/metrics/child_call_stack_profile_collector.h

@@ -11,6 +11,7 @@
 #include "base/memory/ref_counted.h"
 #include "base/single_thread_task_runner.h"
 #include "base/synchronization/lock.h"
+#include "components/metrics/call_stack_profile_builder.h"
 #include "components/metrics/public/interfaces/call_stack_profile_collector.mojom.h"
 
 namespace service_manager {
@@ -41,7 +42,7 @@ namespace metrics {
 //       g_call_stack_profile_collector = LAZY_INSTANCE_INITIALIZER;
 //
 // Then, invoke CreateCompletedCallback() to generate the CompletedCallback to
-// pass when creating the StackSamplingProfiler.
+// pass when creating the CallStackProfileBuilder.
 //
 // When the mojo InterfaceProvider becomes available, provide it via
 // SetParentProfileCollector().
@@ -50,11 +51,11 @@ class ChildCallStackProfileCollector {
   ChildCallStackProfileCollector();
   ~ChildCallStackProfileCollector();
 
-  // Get a callback for use with StackSamplingProfiler that provides the
+  // Get a callback for use with CallStackProfileBuilder that provides the
   // completed profile to this object. The callback should be immediately passed
-  // to the StackSamplingProfiler, and should not be reused between
-  // StackSamplingProfilers. This function may be called on any thread.
-  base::StackSamplingProfiler::CompletedCallback GetProfilerCallback(
+  // to the CallStackProfileBuilder, and should not be reused between
+  // CallStackProfileBuilders. This function may be called on any thread.
+  CallStackProfileBuilder::CompletedCallback GetProfilerCallback(
       const CallStackProfileParams& params,
       base::TimeTicks profile_start_time);
 

components/metrics/public/cpp/OWNERS

@@ -2,3 +2,5 @@ per-file *.mojom=set noparent
 per-file *.mojom=file://ipc/SECURITY_OWNERS
 per-file *_struct_traits*.*=set noparent
 per-file *_struct_traits*.*=file://ipc/SECURITY_OWNERS
+per-file *.typemap=set noparent
+per-file *.typemap=file://ipc/SECURITY_OWNERS

components/metrics/public/cpp/call_stack_profile.typemap

@@ -12,7 +12,7 @@ traits_headers =
     [ "//components/metrics/public/cpp/call_stack_profile_struct_traits.h" ]
 deps = [
   "//base",
-  "//components/metrics:call_stack_profile_params",
+  "//components/metrics:call_stack_profile",
 ]
 type_mappings = [
   "metrics.mojom.CallStackModule=base::StackSamplingProfiler::Module",