Avoid dangling pointers in TraceEvent::Reset().

This patch fixed the base::trace_event::TraceEvent::Reset()
method to ensure that it never creates dangling pointers.

This can happen in the following case:

- TraceEvent::Initialize() is called on an instance, with
  TRACE_EVENT_FLAG_COPY set in the |flags| argument.

  This will copy argument names, copyable string values,
  as well as the name and scope into a single heap
  allocated buffer backed by |parameter_copy_storage_|,
  and will also adjust all internal pointer fields to
  point to it.

- TraceEvent::Reset() is called on the same instance,
  this frees the storage area, but before this CL did not
  update the internal pointers, who were now dangling
  into heap-free memory!

- Later, some code will iterate over the arguments with
  a loop like:

    for (int i = 0;
         i < kTraceMaxNumArgs && arg_names_[i] != nullptr;
         ++i) {
      ...
    }

  The assumption being that an arg_names_[i] value of
  nullptr indicates the end of list. Unfortunately, in
  the case above, this will read completely invalid
  values from memory.

+ Fix TraceEvent::MoveFrom() to call other->Reset() to ensure
  that the source instance is left in consistent state.

I believe this is the source of flakiness on many tests
related to TraceEvent, and hope this fixes it.

BUG=905624,899813
R=oystene@chromium.org,primiano@chromium.org,alexilin@chromium.org,pkl@chromium.org

Change-Id: I63cbadc728130cddc68b8c92b28e1e3f584793f4
Reviewed-on: https://chromium-review.googlesource.com/c/1340308
Commit-Queue: David Turner <digit@chromium.org>
Reviewed-by: Peter Lee <pkl@chromium.org>
Reviewed-by: oysteine <oysteine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609207}

base/trace_event/trace_event_impl.cc

@@ -83,6 +83,8 @@ void TraceEvent::MoveFrom(std::unique_ptr<TraceEvent> other) {
     arg_values_[i] = other->arg_values_[i];
     convertable_values_[i] = std::move(other->convertable_values_[i]);
   }
+
+  other->Reset();
 }
 
 void TraceEvent::Initialize(
@@ -183,7 +185,20 @@ void TraceEvent::Reset() {
   // Only reset fields that won't be initialized in Initialize(), or that may
   // hold references to other objects.
   duration_ = TimeDelta::FromInternalValue(-1);
+
+  // The following pointers might point into parameter_copy_storage_ so
+  // must be reset to nullptr first.
+  for (int i = 0; i < kTraceMaxNumArgs; ++i) {
+    arg_names_[i] = nullptr;
+    arg_values_[i].as_uint = 0u;
+    arg_types_[i] = TRACE_VALUE_TYPE_UINT;
+  }
+  scope_ = nullptr;
+  name_ = nullptr;
+
+  // It is now safe to reset the storage area.
   parameter_copy_storage_.reset();
+
   for (int i = 0; i < kTraceMaxNumArgs; ++i)
     convertable_values_[i].reset();
 }