Perfetto: Fix for worker threads not finalizing proto packets

This caused trace events for these threads to go missing as
Perfetto was not able to recover them from the shared memory
chunks.

Also:
* Removed an unused variable from TraceEventDataSink,
* Read the same member out of a TraceEvent union as the
  one we write to (thread_id vs process_id).

R=primiano@chromium.org

Bug: 900669
Change-Id: I80d4e39b154b138a540770e227f1006fec0a91b2
Reviewed-on: https://chromium-review.googlesource.com/c/1311125Reviewed-by: Primiano Tucci <primiano@chromium.org>
Commit-Queue: oysteine <oysteine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604486}

base/trace_event/trace_event_impl.h

@@ -139,6 +139,7 @@ class BASE_EXPORT TraceEvent {
   ThreadTicks thread_timestamp() const { return thread_timestamp_; }
   char phase() const { return phase_; }
   int thread_id() const { return thread_id_; }
+  int process_id() const { return process_id_; }
   TimeDelta duration() const { return duration_; }
   TimeDelta thread_duration() const { return thread_duration_; }
   const char* scope() const { return scope_; }

base/trace_event/trace_log.cc

@@ -1303,7 +1303,9 @@ TraceEventHandle TraceLog::AddTraceEventWithThreadIdAndTimestamp(
                                bind_id, num_args, arg_names, arg_types,
                                arg_values, convertable_values, flags);
 
-    trace_event_override(&new_trace_event);
+    trace_event_override(
+        &new_trace_event,
+        /*thread_will_flush=*/thread_local_event_buffer != nullptr);
     return handle;
   }
 
@@ -1509,7 +1511,9 @@ void TraceLog::UpdateTraceEventDurationExplicit(
           trace_event_internal::kNoId /* id */,
           trace_event_internal::kNoId /* bind_id */, 0, nullptr, nullptr,
           nullptr, nullptr, TRACE_EVENT_FLAG_NONE);
-      trace_event_override(&new_trace_event);
+      trace_event_override(
+          &new_trace_event,
+          /*thread_will_flush=*/thread_local_event_buffer_.Get() != nullptr);
 
 #if defined(OS_ANDROID)
       new_trace_event.SendToATrace();
@@ -1561,7 +1565,7 @@ void TraceLog::AddMetadataEventWhileLocked(int thread_id,
     TraceEvent trace_event;
     InitializeMetadataEvent(&trace_event, thread_id, metadata_name, arg_name,
                             value);
-    trace_event_override(&trace_event);
+    trace_event_override(&trace_event, /*thread_will_flush=*/true);
   } else {
     InitializeMetadataEvent(
         AddEventToThreadSharedChunkWhileLocked(nullptr, false), thread_id,
@@ -1578,7 +1582,8 @@ void TraceLog::AddMetadataEventsWhileLocked() {
   // Move metadata added by |AddMetadataEvent| into the trace log.
   if (trace_event_override) {
     while (!metadata_events_.empty()) {
-      trace_event_override(metadata_events_.back().get());
+      trace_event_override(metadata_events_.back().get(),
+                           /*thread_will_flush=*/true);
       metadata_events_.pop_back();
     }
   } else {

base/trace_event/trace_log.h

@@ -185,7 +185,8 @@ class BASE_EXPORT TraceLog : public MemoryDumpProvider {
   // Cancels tracing and discards collected data.
   void CancelTracing(const OutputCallback& cb);
 
-  typedef void (*AddTraceEventOverrideCallback)(TraceEvent*);
+  typedef void (*AddTraceEventOverrideCallback)(TraceEvent*,
+                                                bool thread_will_flush);
   typedef void (*OnFlushCallback)();
   // The callback will be called up until the point where the flush is
   // finished, i.e. must be callable until OutputCallback is called with

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

@@ -101,9 +101,10 @@ void TraceEventMetadataSource::Flush(
 
 class TraceEventDataSource::ThreadLocalEventSink {
  public:
-  explicit ThreadLocalEventSink(
-      std::unique_ptr<perfetto::TraceWriter> trace_writer)
-      : trace_writer_(std::move(trace_writer)) {}
+  ThreadLocalEventSink(std::unique_ptr<perfetto::TraceWriter> trace_writer,
+                       bool thread_will_flush)
+      : trace_writer_(std::move(trace_writer)),
+        thread_will_flush_(thread_will_flush) {}
 
   ~ThreadLocalEventSink() {
     // Finalize the current message before posting the |trace_writer_| for
@@ -192,7 +193,8 @@ class TraceEventDataSource::ThreadLocalEventSink {
     // If the TRACE_EVENT_FLAG_COPY flag is set, the char* pointers aren't
     // necessarily valid after the TRACE_EVENT* call, and so we need to store
     // the string every time.
-    bool string_table_enabled = !(trace_event->flags() & TRACE_EVENT_FLAG_COPY);
+    bool string_table_enabled =
+        !(trace_event->flags() & TRACE_EVENT_FLAG_COPY) && thread_will_flush_;
     if (string_table_enabled) {
       name_index = GetStringTableIndexForString(trace_event->name());
       category_name_index =
@@ -231,8 +233,8 @@ class TraceEventDataSource::ThreadLocalEventSink {
     int process_id;
     int thread_id;
     if ((flags & TRACE_EVENT_FLAG_HAS_PROCESS_ID) &&
-        trace_event->thread_id() != base::kNullProcessId) {
-      process_id = trace_event->thread_id();
+        trace_event->process_id() != base::kNullProcessId) {
+      process_id = trace_event->process_id();
       thread_id = -1;
     } else {
       process_id = TraceLog::GetInstance()->process_id();
@@ -330,22 +332,29 @@ class TraceEventDataSource::ThreadLocalEventSink {
         (flags & TRACE_EVENT_FLAG_FLOW_IN)) {
       new_trace_event->set_bind_id(trace_event->bind_id());
     }
+
+    // If we know that the current thread will never send a Flush message
+    // (meaning it's a thread without a messageloop that TraceLog knows about),
+    // we need to finalize the packet right away so Perfetto can recover it.
+    if (!thread_will_flush_) {
+      event_bundle_ = ChromeEventBundleHandle();
+      trace_packet_handle_ = perfetto::TraceWriter::TracePacketHandle();
+    }
   }
 
   void Flush() {
     event_bundle_ = ChromeEventBundleHandle();
     trace_packet_handle_ = perfetto::TraceWriter::TracePacketHandle();
     trace_writer_->Flush();
-    token_ = "";
   }
 
  private:
   std::unique_ptr<perfetto::TraceWriter> trace_writer_;
+  const bool thread_will_flush_;
   ChromeEventBundleHandle event_bundle_;
   perfetto::TraceWriter::TracePacketHandle trace_packet_handle_;
   std::map<intptr_t, int> string_table_;
   int next_string_table_index_ = 0;
-  std::string token_;
 };
 
 namespace {
@@ -453,24 +462,26 @@ void TraceEventDataSource::Flush(
 }
 
 TraceEventDataSource::ThreadLocalEventSink*
-TraceEventDataSource::CreateThreadLocalEventSink() {
+TraceEventDataSource::CreateThreadLocalEventSink(bool thread_will_flush) {
   base::AutoLock lock(lock_);
 
   if (producer_client_) {
     return new ThreadLocalEventSink(
-        producer_client_->CreateTraceWriter(target_buffer_));
+        producer_client_->CreateTraceWriter(target_buffer_), thread_will_flush);
   } else {
     return nullptr;
   }
 }
 
 // static
-void TraceEventDataSource::OnAddTraceEvent(TraceEvent* trace_event) {
+void TraceEventDataSource::OnAddTraceEvent(TraceEvent* trace_event,
+                                           bool thread_will_flush) {
   auto* thread_local_event_sink =
       static_cast<ThreadLocalEventSink*>(ThreadLocalEventSinkSlot()->Get());
 
   if (!thread_local_event_sink) {
-    thread_local_event_sink = GetInstance()->CreateThreadLocalEventSink();
+    thread_local_event_sink =
+        GetInstance()->CreateThreadLocalEventSink(thread_will_flush);
     ThreadLocalEventSinkSlot()->Set(thread_local_event_sink);
   }
 

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

@@ -81,10 +81,11 @@ class COMPONENT_EXPORT(TRACING_CPP) TraceEventDataSource
   TraceEventDataSource();
   ~TraceEventDataSource() override;
 
-  ThreadLocalEventSink* CreateThreadLocalEventSink();
+  ThreadLocalEventSink* CreateThreadLocalEventSink(bool thread_will_flush);
 
   // Callback from TraceLog, can be called from any thread.
-  static void OnAddTraceEvent(base::trace_event::TraceEvent* trace_event);
+  static void OnAddTraceEvent(base::trace_event::TraceEvent* trace_event,
+                              bool thread_will_flush);
 
   base::Lock lock_;
   uint32_t target_buffer_ = 0;