Commit e1d1b3c9 authored by Gabriel Charette's avatar Gabriel Charette Committed by Commit Bot

[TraceLog] Fix uninitialized members

Also renamed the types to "Functions" instead of "Callbacks" as the
latter is confusing with a properly bound base::Callback where the
former clearly denotates a function pointer.

This lack of initialization resulted in an access violation instead of
using the non-overridden function when tracing is called in contexts
where the perfetto override isn't in place yet as part of
https://chromium-review.googlesource.com/c/chromium/src/+/1680893

We were simply lucky to get away with until now.

R=eseckler@chromium.org

Bug: 899897
Change-Id: I815fe5197888c32200cdf5166a0be4fbf79d355a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1721071
Auto-Submit: Gabriel Charette <gab@chromium.org>
Commit-Queue: Eric Seckler <eseckler@chromium.org>
Reviewed-by: default avatarEric Seckler <eseckler@chromium.org>
Cr-Commit-Position: refs/heads/master@{#681769}
parent f1d7448c
...@@ -331,12 +331,12 @@ void TraceLog::ThreadLocalEventBuffer::FlushWhileLocked() { ...@@ -331,12 +331,12 @@ void TraceLog::ThreadLocalEventBuffer::FlushWhileLocked() {
} }
void TraceLog::SetAddTraceEventOverrides( void TraceLog::SetAddTraceEventOverrides(
const AddTraceEventOverrideCallback& add_event_override, const AddTraceEventOverrideFunction& add_event_override,
const OnFlushCallback& on_flush_callback, const OnFlushFunction& on_flush_override,
const UpdateDurationCallback& update_duration_callback) { const UpdateDurationFunction& update_duration_override) {
add_trace_event_override_.store(add_event_override); add_trace_event_override_.store(add_event_override);
on_flush_callback_.store(on_flush_callback); on_flush_override_.store(on_flush_override);
update_duration_callback_.store(update_duration_callback); update_duration_override_.store(update_duration_override);
} }
struct TraceLog::RegisteredAsyncObserver { struct TraceLog::RegisteredAsyncObserver {
...@@ -1033,9 +1033,9 @@ void TraceLog::FlushCurrentThread(int generation, bool discard_events) { ...@@ -1033,9 +1033,9 @@ void TraceLog::FlushCurrentThread(int generation, bool discard_events) {
// This will flush the thread local buffer. // This will flush the thread local buffer.
delete thread_local_event_buffer_.Get(); delete thread_local_event_buffer_.Get();
auto on_flush_callback = on_flush_callback_.load(std::memory_order_relaxed); auto on_flush_override = on_flush_override_.load(std::memory_order_relaxed);
if (on_flush_callback) { if (on_flush_override) {
on_flush_callback(); on_flush_override();
} }
// Scheduler uses TRACE_EVENT macros when posting a task, which can lead // Scheduler uses TRACE_EVENT macros when posting a task, which can lead
...@@ -1432,10 +1432,10 @@ void TraceLog::UpdateTraceEventDurationExplicit( ...@@ -1432,10 +1432,10 @@ void TraceLog::UpdateTraceEventDurationExplicit(
#endif // OS_WIN #endif // OS_WIN
if (category_group_enabled_local & TraceCategory::ENABLED_FOR_RECORDING) { if (category_group_enabled_local & TraceCategory::ENABLED_FOR_RECORDING) {
auto update_duration_callback = auto update_duration_override =
update_duration_callback_.load(std::memory_order_relaxed); update_duration_override_.load(std::memory_order_relaxed);
if (update_duration_callback) { if (update_duration_override) {
update_duration_callback(handle, now, thread_now, thread_instruction_now); update_duration_override(handle, now, thread_now, thread_instruction_now);
return; return;
} }
} }
......
...@@ -196,11 +196,11 @@ class BASE_EXPORT TraceLog : public MemoryDumpProvider { ...@@ -196,11 +196,11 @@ class BASE_EXPORT TraceLog : public MemoryDumpProvider {
// Cancels tracing and discards collected data. // Cancels tracing and discards collected data.
void CancelTracing(const OutputCallback& cb); void CancelTracing(const OutputCallback& cb);
using AddTraceEventOverrideCallback = void (*)(TraceEvent*, using AddTraceEventOverrideFunction = void (*)(TraceEvent*,
bool thread_will_flush, bool thread_will_flush,
TraceEventHandle* handle); TraceEventHandle* handle);
using OnFlushCallback = void (*)(); using OnFlushFunction = void (*)();
using UpdateDurationCallback = using UpdateDurationFunction =
void (*)(TraceEventHandle handle, void (*)(TraceEventHandle handle,
const TimeTicks& now, const TimeTicks& now,
const ThreadTicks& thread_now, const ThreadTicks& thread_now,
...@@ -209,9 +209,9 @@ class BASE_EXPORT TraceLog : public MemoryDumpProvider { ...@@ -209,9 +209,9 @@ class BASE_EXPORT TraceLog : public MemoryDumpProvider {
// finished, i.e. must be callable until OutputCallback is called with // finished, i.e. must be callable until OutputCallback is called with
// has_more_events==false. // has_more_events==false.
void SetAddTraceEventOverrides( void SetAddTraceEventOverrides(
const AddTraceEventOverrideCallback& add_event_override, const AddTraceEventOverrideFunction& add_event_override,
const OnFlushCallback& on_flush_callback, const OnFlushFunction& on_flush_callback,
const UpdateDurationCallback& update_duration_callback); const UpdateDurationFunction& update_duration_callback);
// Called by TRACE_EVENT* macros, don't call this directly. // Called by TRACE_EVENT* macros, don't call this directly.
// The name parameter is a category group for example: // The name parameter is a category group for example:
...@@ -541,9 +541,9 @@ class BASE_EXPORT TraceLog : public MemoryDumpProvider { ...@@ -541,9 +541,9 @@ class BASE_EXPORT TraceLog : public MemoryDumpProvider {
MetadataFilterPredicate metadata_filter_predicate_; MetadataFilterPredicate metadata_filter_predicate_;
subtle::AtomicWord generation_; subtle::AtomicWord generation_;
bool use_worker_thread_; bool use_worker_thread_;
std::atomic<AddTraceEventOverrideCallback> add_trace_event_override_; std::atomic<AddTraceEventOverrideFunction> add_trace_event_override_{nullptr};
std::atomic<OnFlushCallback> on_flush_callback_; std::atomic<OnFlushFunction> on_flush_override_{nullptr};
std::atomic<UpdateDurationCallback> update_duration_callback_; std::atomic<UpdateDurationFunction> update_duration_override_{nullptr};
FilterFactoryForTesting filter_factory_for_testing_; FilterFactoryForTesting filter_factory_for_testing_;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment