Revert "Add metadata at beginning of trace for discard buffer mode"

This reverts commit 1d2c953e.

Reason for revert: 
This is somewhat speculative, but ever since this change was introduced, bot "Builder Linux Tests (dbg)(1)" has been failing on every run. The CL for the failing test talks about Perfetto and tracing, so this CL seems like a likely culprit.

[12707:12707:0924/154115.904192:FATAL:privacy_filtering_check.cc(48)]  Unexpected field in TracePacket proto. IDs from root to child : 8
#0 0x7f66c614a14f base::debug::CollectStackTrace()
#1 0x7f66c5e7cecd base::debug::StackTrace::StackTrace()
#2 0x7f66c5e7ce88 base::debug::StackTrace::StackTrace()
#3 0x7f66c5ece649 logging::LogMessage::~LogMessage()
#4 0x5651bc1aac57 tracing::(anonymous namespace)::VerifyProtoRecursive()
#5 0x5651bc1aa9da tracing::(anonymous namespace)::VerifyProto()
#6 0x5651bc1aa7d2 tracing::PrivacyFilteringCheck::CheckProtoForUnexpectedFields()
#7 0x5651bbb57f38 content::ProtoBackgroundTracingTest_ProtoTraceReceived_Test::RunTestOnMainThread()
#8 0x5651bd505564 content::BrowserTestBase::ProxyRunTestOnMainThreadLoop()



Original change's description:
> Add metadata at beginning of trace for discard buffer mode
> 
> Trace metadata is almost always discarded in discard buffer mode of
> perfetto tracing. For this case, add metadata at beginning of the trace.
> Handles cases when generators are added while tracing.
> 
> Change-Id: I911794fba1e7b460ca1a7a3a54599a8c05678d75
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1812248
> Commit-Queue: ssid <ssid@chromium.org>
> Reviewed-by: Eric Seckler <eseckler@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#699493}

TBR=oysteine@chromium.org,ssid@chromium.org,eseckler@chromium.org

Change-Id: I98fdb75be1a0582ab1a196c66dd74213aeced90b
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1823861Reviewed-by: Finnur Thorarinsson <finnur@chromium.org>
Commit-Queue: Finnur Thorarinsson <finnur@chromium.org>
Cr-Commit-Position: refs/heads/master@{#699689}

content/browser/tracing/tracing_controller_browsertest.cc

@@ -234,6 +234,9 @@ class TracingControllerTest : public ContentBrowserTest {
     Navigate(shell());
 
     TracingControllerImpl* controller = TracingControllerImpl::GetInstance();
+    tracing::TraceEventAgent::GetInstance()->AddMetadataGeneratorFunction(
+        base::Bind(&TracingControllerTest::GenerateMetadataDict,
+                   base::Unretained(this)));
 
     {
       base::RunLoop run_loop;
@@ -263,9 +266,6 @@ class TracingControllerTest : public ContentBrowserTest {
 
       metadata_ = std::make_unique<base::DictionaryValue>();
       metadata_->SetString("not-whitelisted", "this_not_found");
-      tracing::TraceEventAgent::GetInstance()->AddMetadataGeneratorFunction(
-          base::Bind(&TracingControllerTest::GenerateMetadataDict,
-                     base::Unretained(this)));
 
       bool result = controller->StopTracing(trace_data_endpoint);
       ASSERT_TRUE(result);

services/tracing/public/cpp/perfetto/trace_event_data_source.h

@@ -20,7 +20,6 @@
 #include "base/trace_event/trace_config.h"
 #include "services/tracing/public/cpp/perfetto/perfetto_traced_process.h"
 #include "third_party/perfetto/protos/perfetto/trace/chrome/chrome_metadata.pbzero.h"
-#include "third_party/perfetto/protos/perfetto/trace/chrome/chrome_trace_event.pbzero.h"
 
 namespace perfetto {
 class StartupTraceWriter;
@@ -61,9 +60,7 @@ class COMPONENT_EXPORT(TRACING_CPP) TraceEventMetadataSource
       perfetto::protos::pbzero::ChromeMetadataPacket*,
       bool /* privacy_filtering_enabled */)>;
 
-  // Any callbacks passed here will be called when tracing. Note that if tracing
-  // is enabled while calling this method, the callback may be invoked
-  // directly.
+  // Any callbacks passed here will be called when tracing starts.
   void AddGeneratorFunction(JsonMetadataGeneratorFunction generator);
   // Same as above, but for filling in proto format.
   void AddGeneratorFunction(MetadataGeneratorFunction generator);
@@ -84,29 +81,15 @@ class COMPONENT_EXPORT(TRACING_CPP) TraceEventMetadataSource
   TraceEventMetadataSource();
   ~TraceEventMetadataSource() override;
 
-  void GenerateMetadata(
-      std::unique_ptr<std::vector<JsonMetadataGeneratorFunction>>
-          json_generators,
-      std::unique_ptr<std::vector<MetadataGeneratorFunction>> proto_generators);
-  void GenerateMetadataFromGenerator(
-      const MetadataGeneratorFunction& generator);
-  void GenerateJsonMetadataFromGenerator(
-      const JsonMetadataGeneratorFunction& generator,
-      perfetto::protos::pbzero::ChromeEventBundle* event_bundle);
+  void GenerateMetadata(std::unique_ptr<perfetto::TraceWriter> trace_writer);
   std::unique_ptr<base::DictionaryValue> GenerateTraceConfigMetadataDict();
 
-  // All members are protected by |lock_|.
-  base::Lock lock_;
   std::vector<JsonMetadataGeneratorFunction> json_generator_functions_;
   std::vector<MetadataGeneratorFunction> generator_functions_;
-
-  const scoped_refptr<base::SequencedTaskRunner> origin_task_runner_;
-
+  scoped_refptr<base::SequencedTaskRunner> origin_task_runner_;
   std::unique_ptr<perfetto::TraceWriter> trace_writer_;
   bool privacy_filtering_enabled_ = false;
   std::string chrome_config_;
-  std::unique_ptr<base::trace_event::TraceConfig> parsed_chrome_config_;
-  bool emit_metadata_at_start_ = false;
 
   DISALLOW_COPY_AND_ASSIGN(TraceEventMetadataSource);
 };

services/tracing/public/cpp/perfetto/trace_event_data_source_unittest.cc

@@ -486,76 +486,24 @@ void MetadataHasNamedValue(const google::protobuf::RepeatedPtrField<
   NOTREACHED();
 }
 
-std::unique_ptr<base::DictionaryValue> AddJsonMetadataGenerator() {
-  auto metadata = std::make_unique<base::DictionaryValue>();
-  metadata->SetInteger("foo_int", 42);
-  metadata->SetString("foo_str", "bar");
-  metadata->SetBoolean("foo_bool", true);
-
-  auto child_dict = std::make_unique<base::DictionaryValue>();
-  child_dict->SetString("child_str", "child_val");
-  metadata->Set("child_dict", std::move(child_dict));
-  return metadata;
-}
-
-TEST_F(TraceEventDataSourceTest, MetadataGeneratorBeforeTracing) {
-  auto* metadata_source = TraceEventMetadataSource::GetInstance();
-  metadata_source->AddGeneratorFunction(
-      base::BindRepeating(&AddJsonMetadataGenerator));
-
-  metadata_source->StartTracing(producer_client(),
-                                perfetto::DataSourceConfig());
-
-  base::RunLoop wait_for_stop;
-  metadata_source->StopTracing(wait_for_stop.QuitClosure());
-  wait_for_stop.Run();
-
-  auto metadata = producer_client()->GetChromeMetadata();
-  EXPECT_EQ(4, metadata.size());
-  MetadataHasNamedValue(metadata, "foo_int", 42);
-  MetadataHasNamedValue(metadata, "foo_str", "bar");
-  MetadataHasNamedValue(metadata, "foo_bool", true);
-
-  auto child_dict = std::make_unique<base::DictionaryValue>();
-  child_dict->SetString("child_str", "child_val");
-  MetadataHasNamedValue(metadata, "child_dict", *child_dict);
-}
-
-TEST_F(TraceEventDataSourceTest, MetadataGeneratorWhileTracing) {
-  auto* metadata_source = TraceEventMetadataSource::GetInstance();
-
-  metadata_source->StartTracing(producer_client(),
-                                perfetto::DataSourceConfig());
-  metadata_source->AddGeneratorFunction(
-      base::BindRepeating(&AddJsonMetadataGenerator));
-
-  base::RunLoop wait_for_stop;
-  metadata_source->StopTracing(wait_for_stop.QuitClosure());
-  wait_for_stop.Run();
-
-  auto metadata = producer_client()->GetChromeMetadata();
-  EXPECT_EQ(4, metadata.size());
-  MetadataHasNamedValue(metadata, "foo_int", 42);
-  MetadataHasNamedValue(metadata, "foo_str", "bar");
-  MetadataHasNamedValue(metadata, "foo_bool", true);
-
-  auto child_dict = std::make_unique<base::DictionaryValue>();
-  child_dict->SetString("child_str", "child_val");
-  MetadataHasNamedValue(metadata, "child_dict", *child_dict);
-}
-
-TEST_F(TraceEventDataSourceTest, MultipleMetadataGenerators) {
+TEST_F(TraceEventDataSourceTest, MetadataSourceBasicTypes) {
   auto* metadata_source = TraceEventMetadataSource::GetInstance();
   metadata_source->AddGeneratorFunction(base::BindRepeating([]() {
     auto metadata = std::make_unique<base::DictionaryValue>();
-    metadata->SetInteger("before_int", 42);
+    metadata->SetInteger("foo_int", 42);
+    metadata->SetString("foo_str", "bar");
+    metadata->SetBoolean("foo_bool", true);
+
+    auto child_dict = std::make_unique<base::DictionaryValue>();
+    child_dict->SetString("child_str", "child_val");
+    metadata->Set("child_dict", std::move(child_dict));
     return metadata;
   }));
 
+  CreateTraceEventDataSource();
+
   metadata_source->StartTracing(producer_client(),
                                 perfetto::DataSourceConfig());
-  metadata_source->AddGeneratorFunction(
-      base::BindRepeating(&AddJsonMetadataGenerator));
 
   base::RunLoop wait_for_stop;
   metadata_source->StopTracing(wait_for_stop.QuitClosure());
@@ -570,10 +518,6 @@ TEST_F(TraceEventDataSourceTest, MultipleMetadataGenerators) {
   auto child_dict = std::make_unique<base::DictionaryValue>();
   child_dict->SetString("child_str", "child_val");
   MetadataHasNamedValue(metadata, "child_dict", *child_dict);
-
-  metadata = producer_client()->GetChromeMetadata(1);
-  EXPECT_EQ(1, metadata.size());
-  MetadataHasNamedValue(metadata, "before_int", 42);
 }
 
 TEST_F(TraceEventDataSourceTest, BasicTraceEvent) {