Tracing: Convert the TracingControllerImpl to use the Perfetto Consumer API

Tracing_controller_impl.cc/.h now uses the Consumer Mojo interface rather
than the old Agent/Coordinator interface, allowing the latter to be deleted in
the next CL.

As part of this, the option to anonymize the data has to be moved from
the TraceConfig struct to be an explicit argument to StopTracing instead,
as the filtering/whitelisting done at JSON export time needs the full
unfiltered proto data available.

BUG=839086

Change-Id: Id73b73a1cdbc6c7a6c8d343c391136abc5914743
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1817207Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Erik Chen <erikchen@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Reviewed-by: ssid <ssid@chromium.org>
Reviewed-by: Eric Seckler <eseckler@chromium.org>
Commit-Queue: oysteine <oysteine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#705202}

android_webview/browser/tracing/aw_tracing_controller.cc

@@ -43,7 +43,7 @@ class AwTraceDataEndpoint
 
   void ReceivedTraceFinalContents() override {
     base::PostTask(FROM_HERE, {content::BrowserThread::UI},
-                   base::BindOnce(std::move(completed_callback_)));
+                   std::move(completed_callback_));
   }
 
   void ReceiveTraceChunk(std::unique_ptr<std::string> chunk) override {
@@ -88,20 +88,21 @@ bool AwTracingController::Start(JNIEnv* env,
       base::android::ConvertJavaStringToUTF8(env, jcategories);
   base::trace_event::TraceConfig trace_config(
       categories, static_cast<base::trace_event::TraceRecordMode>(jmode));
-  // Required for filtering out potential PII.
-  trace_config.EnableArgumentFilter();
   return content::TracingController::GetInstance()->StartTracing(
       trace_config, content::TracingController::StartTracingDoneCallback());
 }
 
 bool AwTracingController::StopAndFlush(JNIEnv* env,
                                        const JavaParamRef<jobject>& obj) {
+  // privacy_filtering_enabled=true is required for filtering out potential PII.
   return content::TracingController::GetInstance()->StopTracing(
       AwTraceDataEndpoint::Create(
           base::BindRepeating(&AwTracingController::OnTraceDataReceived,
                               weak_factory_.GetWeakPtr()),
           base::BindOnce(&AwTracingController::OnTraceDataComplete,
-                         weak_factory_.GetWeakPtr())));
+                         weak_factory_.GetWeakPtr())),
+      /*agent_label=*/"",
+      /*privacy_filtering_enabled=*/true);
 }
 
 void AwTracingController::OnTraceDataComplete() {

components/heap_profiling/supervisor.cc

@@ -125,7 +125,8 @@ void Supervisor::RequestTraceWithHeapDump(TraceFinishedCallback callback,
   }
 
   auto finished_dump_callback = base::BindOnce(
-      [](TraceFinishedCallback callback, bool success, uint64_t dump_guid) {
+      [](TraceFinishedCallback callback, bool anonymize, bool success,
+         uint64_t dump_guid) {
         // Once the trace has stopped, run |callback| on the UI thread.
         auto finish_sink_callback = base::BindOnce(
             [](TraceFinishedCallback callback,
@@ -137,13 +138,15 @@ void Supervisor::RequestTraceWithHeapDump(TraceFinishedCallback callback,
                              base::BindOnce(std::move(callback), true,
                                             std::move(result)));
             },
-            base::Passed(std::move(callback)));
+            std::move(callback));
         scoped_refptr<content::TracingController::TraceDataEndpoint> sink =
             content::TracingController::CreateStringEndpoint(
                 std::move(finish_sink_callback));
-        content::TracingController::GetInstance()->StopTracing(sink);
+        content::TracingController::GetInstance()->StopTracing(
+            sink,
+            /*agent_label=*/"", anonymize);
       },
-      std::move(callback));
+      std::move(callback), anonymize);
 
   auto trigger_memory_dump_callback = base::BindOnce(
       [](base::OnceCallback<void(bool success, uint64_t dump_guid)>

components/tracing/common/trace_startup_config.cc

@@ -72,8 +72,6 @@ TraceStartupConfig::GetDefaultBrowserStartupConfig() {
   // First 10k events at start are sufficient to debug startup traces.
   trace_config.SetTraceBufferSizeInEvents(10000);
   trace_config.SetProcessFilterConfig(process_config);
-  // Enable argument filter since we could be background tracing.
-  trace_config.EnableArgumentFilter();
   return trace_config;
 }
 
@@ -248,6 +246,8 @@ bool TraceStartupConfig::EnableFromBackgroundTracing() {
 
   SetBackgroundStartupTracingEnabled(false);
   trace_config_ = GetDefaultBrowserStartupConfig();
+  trace_config_.EnableArgumentFilter();
+
   is_enabled_ = true;
   session_owner_ = SessionOwner::kBackgroundTracing;
   should_trace_to_result_file_ = false;

content/browser/tracing/background_tracing_active_scenario.cc

@@ -244,7 +244,9 @@ class LegacyTracingSession
                 parent_scenario_->GetWeakPtr(), std::move(on_success))),
             true /* compress_with_background_priority */);
 
-    TracingControllerImpl::GetInstance()->StopTracing(trace_data_endpoint);
+    TracingControllerImpl::GetInstance()->StopTracing(
+        trace_data_endpoint, "",
+        parent_scenario_->GetConfig()->requires_anonymized_data());
   }
 
   void AbortScenario(const base::RepeatingClosure& on_abort_callback) override {

content/browser/tracing/background_tracing_config_impl.cc

@@ -276,10 +276,6 @@ TraceConfig BackgroundTracingConfigImpl::GetTraceConfig() const {
     chrome_config.SetProcessFilterConfig(process_config);
   }
 
-  if (requires_anonymized_data_) {
-    chrome_config.EnableArgumentFilter();
-  }
-
   chrome_config.SetTraceBufferSizeInKb(GetMaximumTraceBufferSizeKb());
 
 #if defined(OS_ANDROID)

content/browser/tracing/background_tracing_manager_browsertest.cc

@@ -1589,7 +1589,7 @@ IN_PROC_BROWSER_TEST_F(BackgroundTracingManagerBrowserTest, RunStartupTracing) {
 
   EXPECT_TRUE(BackgroundTracingManager::GetInstance()->SetActiveScenario(
       std::move(config), trace_receiver_helper.get_receive_callback(),
-      BackgroundTracingManager::NO_DATA_FILTERING));
+      BackgroundTracingManager::ANONYMIZE_DATA));
 
   tracelog_helper.WaitForStartTracing();
   background_tracing_helper.WaitForTracingEnabled();
@@ -1598,9 +1598,6 @@ IN_PROC_BROWSER_TEST_F(BackgroundTracingManagerBrowserTest, RunStartupTracing) {
                   ->GetActiveScenarioForTesting()
                   ->GetConfig()
                   ->requires_anonymized_data());
-  EXPECT_TRUE(base::trace_event::TraceLog::GetInstance()
-                  ->GetCurrentTraceConfig()
-                  .IsArgumentFilterEnabled());
 
   // Since we specified a delay in the scenario, we should still be tracing
   // at this point.

content/browser/tracing/tracing_controller_browsertest.cc

@@ -224,10 +224,8 @@ class TracingControllerTest : public ContentBrowserTest {
           base::BindOnce(&TracingControllerTest::StartTracingDoneCallbackTest,
                          base::Unretained(this), run_loop.QuitClosure());
 
-      TraceConfig config = TraceConfig();
-      config.EnableArgumentFilter();
-
-      bool result = controller->StartTracing(config, std::move(callback));
+      bool result =
+          controller->StartTracing(TraceConfig(), std::move(callback));
       ASSERT_TRUE(result);
       run_loop.Run();
       EXPECT_EQ(enable_recording_done_callback_count(), 1);
@@ -248,7 +246,9 @@ class TracingControllerTest : public ContentBrowserTest {
           base::Bind(&TracingControllerTest::GenerateMetadataDict,
                      base::Unretained(this)));
 
-      bool result = controller->StopTracing(trace_data_endpoint);
+      bool result =
+          controller->StopTracing(trace_data_endpoint, /*agent_label=*/"",
+                                  /*privacy_filtering_enabled=*/true);
       ASSERT_TRUE(result);
       run_loop.Run();
       EXPECT_EQ(disable_recording_done_callback_count(), 1);

content/browser/tracing/tracing_controller_impl.cc

@@ -40,7 +40,7 @@
 #include "net/base/network_change_notifier.h"
 #include "services/service_manager/public/cpp/connector.h"
 #include "services/tracing/public/cpp/perfetto/java_heap_profiler/java_heap_profiler.h"
-#include "services/tracing/public/cpp/perfetto/perfetto_traced_process.h"
+#include "services/tracing/public/cpp/perfetto/perfetto_config.h"
 #include "services/tracing/public/cpp/trace_event_agent.h"
 #include "services/tracing/public/cpp/traced_process_impl.h"
 #include "services/tracing/public/cpp/tracing_features.h"
@@ -217,21 +217,17 @@ void TracingControllerImpl::AddAgents() {
 }
 
 void TracingControllerImpl::ConnectToServiceIfNeeded() {
-  if (!coordinator_) {
+  if (!consumer_host_) {
     GetSystemConnector()->BindInterface(tracing::mojom::kServiceName,
-                                        &coordinator_);
-    coordinator_.set_connection_error_handler(base::BindOnce(
+                                        &consumer_host_);
+    consumer_host_.set_connection_error_handler(base::BindOnce(
         [](TracingControllerImpl* controller) {
-          controller->coordinator_.reset();
+          controller->consumer_host_.reset();
         },
         base::Unretained(this)));
   }
 }
 
-void TracingControllerImpl::DisconnectFromService() {
-  coordinator_ = nullptr;
-}
-
 // Can be called on any thread.
 std::unique_ptr<base::DictionaryValue>
 TracingControllerImpl::GenerateMetadataDict() {
@@ -394,15 +390,27 @@ bool TracingControllerImpl::StartTracing(
   trace_config_ =
       std::make_unique<base::trace_event::TraceConfig>(trace_config);
 
+  DCHECK(!tracing_session_host_);
   ConnectToServiceIfNeeded();
-  coordinator_->StartTracing(
-      trace_config.ToString(),
-      base::BindOnce(
-          [](StartTracingDoneCallback callback, bool success) {
-            if (!callback.is_null())
-              std::move(callback).Run();
-          },
-          std::move(callback)));
+
+  perfetto::TraceConfig perfetto_config = tracing::GetDefaultPerfettoConfig(
+      trace_config, /*requires_anonymized_data=*/false);
+
+  mojo::PendingRemote<tracing::mojom::TracingSessionClient>
+      tracing_session_client;
+  binding_.Bind(tracing_session_client.InitWithNewPipeAndPassReceiver());
+  binding_.set_connection_error_handler(base::BindOnce(
+      &TracingControllerImpl::OnTracingFailed, base::Unretained(this)));
+
+  consumer_host_->EnableTracing(
+      mojo::MakeRequest(&tracing_session_host_),
+      std::move(tracing_session_client), std::move(perfetto_config),
+      tracing::mojom::TracingClientPriority::kUserInitiated);
+  tracing_session_host_.set_connection_error_handler(base::BindOnce(
+      &TracingControllerImpl::OnTracingFailed, base::Unretained(this)));
+
+  start_tracing_callback_ = std::move(callback);
+
   // TODO(chiniforooshan): The actual success value should be sent by the
   // callback asynchronously.
   return true;
@@ -512,8 +520,9 @@ bool TracingControllerImpl::StopTracing(
 
 bool TracingControllerImpl::StopTracing(
     const scoped_refptr<TraceDataEndpoint>& trace_data_endpoint,
-    const std::string& agent_label) {
-  if (!IsTracing() || drainer_ || !coordinator_)
+    const std::string& agent_label,
+    bool privacy_filtering_enabled) {
+  if (!IsTracing() || drainer_ || !tracing_session_host_)
     return false;
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
@@ -521,26 +530,36 @@ bool TracingControllerImpl::StopTracing(
   base::trace_event::TraceLog::GetInstance()->AddClockSyncMetadataEvent();
 #endif
 
+  // Setting the argument filter is no longer supported just in the TraceConfig;
+  // clients of the TracingController that need filtering need to pass that
+  // option to StopTracing directly as an argument. This is due to Perfetto-
+  // based tracing requiring this filtering to be done during serialization
+  // time and not during tracing time.
+  // TODO(oysteine): Remove the config option once the legacy IPC layer is
+  // removed.
+  CHECK(privacy_filtering_enabled || !trace_config_->IsArgumentFilterEnabled());
+
   tracing::TraceStartupConfig::GetInstance()->SetDisabled();
   trace_data_endpoint_ = std::move(trace_data_endpoint);
   is_data_complete_ = false;
-  is_metadata_available_ = false;
-  mojo::DataPipe data_pipe;
-  drainer_.reset(
-      new mojo::DataPipeDrainer(this, std::move(data_pipe.consumer_handle)));
-  if (agent_label.empty()) {
-    // Stop and flush all agents.
-    coordinator_->StopAndFlush(
-        std::move(data_pipe.producer_handle),
-        base::BindRepeating(&TracingControllerImpl::OnMetadataAvailable,
-                            base::Unretained(this)));
-  } else {
-    // Stop all and flush a particular agent.
-    coordinator_->StopAndFlushAgent(
-        std::move(data_pipe.producer_handle), agent_label,
-        base::BindRepeating(&TracingControllerImpl::OnMetadataAvailable,
-                            base::Unretained(this)));
+  read_buffers_complete_ = false;
+
+  mojo::ScopedDataPipeProducerHandle producer_handle;
+  mojo::ScopedDataPipeConsumerHandle consumer_handle;
+  MojoResult result =
+      mojo::CreateDataPipe(nullptr, &producer_handle, &consumer_handle);
+  if (result != MOJO_RESULT_OK) {
+    CompleteFlush();
+    return true;
   }
+
+  drainer_.reset(new mojo::DataPipeDrainer(this, std::move(consumer_handle)));
+
+  tracing_session_host_->DisableTracingAndEmitJson(
+      agent_label, std::move(producer_handle), privacy_filtering_enabled,
+      base::BindOnce(&TracingControllerImpl::OnReadBuffersComplete,
+                     base::Unretained(this)));
+
   // TODO(chiniforooshan): Is the return value used anywhere?
   return true;
 }
@@ -549,12 +568,14 @@ bool TracingControllerImpl::GetTraceBufferUsage(
     GetTraceBufferUsageCallback callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
-  ConnectToServiceIfNeeded();
-  coordinator_->RequestBufferUsage(base::BindOnce(
+  if (!tracing_session_host_) {
+    std::move(callback).Run(0.0, 0);
+    return true;
+  }
+
+  tracing_session_host_->RequestBufferUsage(base::BindOnce(
       [](GetTraceBufferUsageCallback callback, bool success, float percent_full,
-         uint32_t approximate_count) {
-        std::move(callback).Run(percent_full, approximate_count);
-      },
+         bool data_loss) { std::move(callback).Run(percent_full, 0); },
       std::move(callback)));
   // TODO(chiniforooshan): The actual success value should be sent by the
   // callback asynchronously.
@@ -565,15 +586,15 @@ bool TracingControllerImpl::IsTracing() {
   return trace_config_ != nullptr;
 }
 
-void TracingControllerImpl::RegisterTracingUI(TracingUI* tracing_ui) {
-  DCHECK(tracing_uis_.find(tracing_ui) == tracing_uis_.end());
-  tracing_uis_.insert(tracing_ui);
+void TracingControllerImpl::OnTracingEnabled() {
+  if (start_tracing_callback_)
+    std::move(start_tracing_callback_).Run();
 }
 
-void TracingControllerImpl::UnregisterTracingUI(TracingUI* tracing_ui) {
-  auto it = tracing_uis_.find(tracing_ui);
-  DCHECK(it != tracing_uis_.end());
-  tracing_uis_.erase(it);
+void TracingControllerImpl::OnTracingDisabled() {}
+
+void TracingControllerImpl::OnTracingFailed() {
+  CompleteFlush();
 }
 
 void TracingControllerImpl::OnDataAvailable(const void* data,
@@ -586,42 +607,24 @@ void TracingControllerImpl::OnDataAvailable(const void* data,
 }
 
 void TracingControllerImpl::CompleteFlush() {
-  if (trace_data_endpoint_) {
+  if (trace_data_endpoint_)
     trace_data_endpoint_->ReceivedTraceFinalContents();
-  }
-  filtered_metadata_.reset(nullptr);
+
   trace_data_endpoint_ = nullptr;
   trace_config_ = nullptr;
   drainer_ = nullptr;
+  tracing_session_host_.reset();
+  binding_.Close();
 }
 
 void TracingControllerImpl::OnDataComplete() {
   is_data_complete_ = true;
-  if (is_metadata_available_)
+  if (read_buffers_complete_)
     CompleteFlush();
 }
 
-void TracingControllerImpl::OnMetadataAvailable(base::Value metadata) {
-  DCHECK(!filtered_metadata_);
-  is_metadata_available_ = true;
-  base::trace_event::MetadataFilterPredicate metadata_filter;
-  if (trace_config_->IsArgumentFilterEnabled()) {
-    metadata_filter = base::trace_event::TraceLog::GetInstance()
-                          ->GetMetadataFilterPredicate();
-  }
-  if (metadata_filter.is_null()) {
-    filtered_metadata_ = base::DictionaryValue::From(
-        base::Value::ToUniquePtrValue(std::move(metadata)));
-  } else {
-    filtered_metadata_ = std::make_unique<base::DictionaryValue>();
-    for (auto it : metadata.DictItems()) {
-      if (metadata_filter.Run(it.first)) {
-        filtered_metadata_->SetKey(it.first, std::move(it.second));
-      } else {
-        filtered_metadata_->SetKey(it.first, base::Value("__stripped__"));
-      }
-    }
-  }
+void TracingControllerImpl::OnReadBuffersComplete() {
+  read_buffers_complete_ = true;
   if (is_data_complete_)
     CompleteFlush();
 }

content/browser/tracing/tracing_controller_impl.h

@@ -15,8 +15,9 @@
 #include "base/timer/timer.h"
 #include "content/common/content_export.h"
 #include "content/public/browser/tracing_controller.h"
+#include "mojo/public/cpp/bindings/binding.h"
 #include "mojo/public/cpp/system/data_pipe_drainer.h"
-#include "services/tracing/public/mojom/tracing.mojom.h"
+#include "services/tracing/public/mojom/perfetto_service.mojom.h"
 
 namespace base {
 
@@ -35,14 +36,14 @@ namespace content {
 
 class PerfettoFileTracer;
 class TracingDelegate;
-class TracingUI;
 
 class TracingControllerImpl : public TracingController,
-                              public mojo::DataPipeDrainer::Client {
+                              public mojo::DataPipeDrainer::Client,
+                              public tracing::mojom::TracingSessionClient {
  public:
   // Create an endpoint for dumping the trace data to a callback.
   CONTENT_EXPORT static scoped_refptr<TraceDataEndpoint> CreateCallbackEndpoint(
-      base::OnceCallback<void(std::unique_ptr<std::string>)> callback);
+      CompletionCallback callback);
 
   CONTENT_EXPORT static scoped_refptr<TraceDataEndpoint>
   CreateCompressedStringEndpoint(scoped_refptr<TraceDataEndpoint> endpoint,
@@ -59,12 +60,16 @@ class TracingControllerImpl : public TracingController,
                     StartTracingDoneCallback callback) override;
   bool StopTracing(const scoped_refptr<TraceDataEndpoint>& endpoint) override;
   bool StopTracing(const scoped_refptr<TraceDataEndpoint>& endpoint,
-                   const std::string& agent_label) override;
+                   const std::string& agent_label,
+                   bool privacy_filtering_enabled = false) override;
   bool GetTraceBufferUsage(GetTraceBufferUsageCallback callback) override;
   bool IsTracing() override;
 
-  void RegisterTracingUI(TracingUI* tracing_ui);
-  void UnregisterTracingUI(TracingUI* tracing_ui);
+  // tracing::mojom::TracingSessionClient implementation:
+  void OnTracingEnabled() override;
+  void OnTracingDisabled() override;
+
+  void OnTracingFailed();
 
   // For unittests.
   CONTENT_EXPORT void SetTracingDelegateForTesting(
@@ -92,14 +97,13 @@ class TracingControllerImpl : public TracingController,
   ~TracingControllerImpl() override;
   void AddAgents();
   void ConnectToServiceIfNeeded();
-  void DisconnectFromService();
   std::unique_ptr<base::DictionaryValue> GenerateMetadataDict();
 
   // mojo::DataPipeDrainer::Client
   void OnDataAvailable(const void* data, size_t num_bytes) override;
   void OnDataComplete() override;
 
-  void OnMetadataAvailable(base::Value metadata);
+  void OnReadBuffersComplete();
 
   void CompleteFlush();
 
@@ -111,16 +115,18 @@ class TracingControllerImpl : public TracingController,
   base::FilePath GetStartupTraceFileName() const;
 
   std::unique_ptr<PerfettoFileTracer> perfetto_file_tracer_;
-  tracing::mojom::CoordinatorPtr coordinator_;
+  tracing::mojom::ConsumerHostPtr consumer_host_;
+  tracing::mojom::TracingSessionHostPtr tracing_session_host_;
+  mojo::Binding<tracing::mojom::TracingSessionClient> binding_{this};
+  StartTracingDoneCallback start_tracing_callback_;
+
   std::vector<std::unique_ptr<tracing::BaseAgent>> agents_;
   std::unique_ptr<TracingDelegate> delegate_;
   std::unique_ptr<base::trace_event::TraceConfig> trace_config_;
   std::unique_ptr<mojo::DataPipeDrainer> drainer_;
   scoped_refptr<TraceDataEndpoint> trace_data_endpoint_;
-  std::unique_ptr<base::DictionaryValue> filtered_metadata_;
-  std::set<TracingUI*> tracing_uis_;
   bool is_data_complete_ = false;
-  bool is_metadata_available_ = false;
+  bool read_buffers_complete_ = false;
 
   base::FilePath startup_trace_file_;
   // This timer initiates trace file saving.

content/public/browser/tracing_controller.h

@@ -114,7 +114,8 @@ class TracingController {
       const scoped_refptr<TraceDataEndpoint>& trace_data_endpoint) = 0;
   virtual bool StopTracing(
       const scoped_refptr<TraceDataEndpoint>& trace_data_endpoint,
-      const std::string& agent_label) = 0;
+      const std::string& agent_label,
+      bool privacy_filtering_enabled = false) = 0;
 
   // Get the maximum across processes of trace buffer percent full state.
   // When the TraceBufferUsage value is determined, the callback is