perfetto: Stop tracing on threads more reliably

Moves resetting of TraceEventDataSource fields before flushing threads.
This way, the thread's flush no longer races with the field resets.

(If flushing occurs before the field resets, another thread may
theoretically recreate its ThreadLocalEventSink before the resets, too.)

Also flush the disabling thread in the same task, as a future task may
be executed on a different worker thread.

Bug: 903449
Change-Id: I13eb0d0a330ec66ffd826791f842c6fa7f8bd717
Reviewed-on: https://chromium-review.googlesource.com/c/1340325
Commit-Queue: oysteine <oysteine@chromium.org>
Reviewed-by: oysteine <oysteine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608972}

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

@@ -427,34 +427,43 @@ void TraceEventDataSource::StopTracing(
         RegisterTracedValueProtoWriter(false);
         TraceLog::GetInstance()->SetAddTraceEventOverride(nullptr, nullptr);
 
-        // TraceLog::CancelTracing will cause metadata events to be written;
-        // make sure we flush the TraceWriter for this thread (TraceLog will
-        // only call TraceEventDataSource::FlushCurrentThread for threads with
-        // a MessageLoop).
-        // TODO(oysteine): The perfetto service itself should be able to recover
-        // unreturned chunks so technically this can go away
-        // at some point, but seems needed for now.
-        FlushCurrentThread();
-
         if (data_source->stop_complete_callback_) {
           std::move(data_source->stop_complete_callback_).Run();
         }
       };
 
-  if (TraceLog::GetInstance()->IsEnabled()) {
-    // We call CancelTracing because we don't want/need TraceLog to do any of
-    // its own JSON serialization on its own.
+  bool was_enabled = TraceLog::GetInstance()->IsEnabled();
+  if (was_enabled) {
+    // Write metadata events etc.
+    TraceLog::GetInstance()->SetDisabled();
+  }
+
+  {
+    // Prevent recreation of ThreadLocalEventSinks after flush.
+    base::AutoLock lock(lock_);
+    DCHECK(producer_client_);
+    producer_client_ = nullptr;
+    target_buffer_ = 0;
+  }
+
+  if (was_enabled) {
+    // TraceLog::SetDisabled will cause metadata events to be written; make
+    // sure we flush the TraceWriter for this thread (TraceLog will only call
+    // TraceEventDataSource::FlushCurrentThread for threads with a MessageLoop).
+    // TODO(eseckler): Flush all worker threads.
+    // TODO(oysteine): The perfetto service itself should be able to recover
+    // unreturned chunks so technically this can go away at some point, but
+    // seems needed for now.
+    FlushCurrentThread();
+    
+    // Flush the remaining threads via TraceLog. We call CancelTracing because
+    // we don't want/need TraceLog to do any of its own JSON serialization.
     TraceLog::GetInstance()->CancelTracing(base::BindRepeating(
         on_tracing_stopped_callback, base::Unretained(this)));
   } else {
     on_tracing_stopped_callback(this, scoped_refptr<base::RefCountedString>(),
                                 false);
   }
-
-  base::AutoLock lock(lock_);
-  DCHECK(producer_client_);
-  producer_client_ = nullptr;
-  target_buffer_ = 0;
 }
 
 void TraceEventDataSource::Flush(