Fix the benchmark app to handle real-world events

apps/benchmark/event.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "apps/benchmark/event.h"
 
 #include <map>
 #include <stack>
 
 #include "base/json/json_reader.h"
 #include "base/macros.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/values.h"
 
 namespace benchmark {
 
 Event::Event() {}
 
 Event::Event(EventType type,
              std::string name,
              std::string categories,
              base::TimeTicks timestamp,
              base::TimeDelta duration)
     : type(type),
       name(name),
       categories(categories),
       timestamp(timestamp),
       duration(duration) {}
 
 Event::~Event() {}
 
 namespace {
 // ID uniquely identifying an asynchronous event stack.
 struct AsyncEventStackId {
-  int id;
+  std::string id;
   std::string cat;
 };
 
 bool operator<(const AsyncEventStackId& t, const AsyncEventStackId& o) {
   return t.id < o.id || (t.id == o.id && t.cat < o.cat);
 }
 
 // ID uniquely identifying a duration event stack.
-typedef int DurationEventStackId;
+typedef std::string DurationEventStackId;
 
 // Makes a unique id for a duration event stack.
 bool GetDurationEventStackId(base::DictionaryValue* event_dict,
                              DurationEventStackId* durationId) {
-  if (!event_dict->GetInteger("tid", durationId)) {
-    LOG(ERROR) << "Incorrect trace event (missing tid).";
+  const base::Value* value;
+  if (!event_dict->Get("tid", &value)) {
+    LOG(ERROR) << "Incorrect duration trace event (missing tid): "
+               << *event_dict;
     return false;
   }
+
+  if (value->IsType(base::Value::TYPE_INTEGER)) {
+    int tid_int;
+    // We already verified the type, so it should be an integer.
+    DCHECK(value->GetAsInteger(&tid_int));

[ppi, 2015/10/01 13:31:16]

please extract the common part between this and the function below into a
helper.

[etiennej, 2015/10/01 13:41:47]

Done.

+    *durationId = base::IntToString(tid_int);
+  } else if (value->IsType(base::Value::TYPE_STRING)) {
+    DCHECK(value->GetAsString(durationId));
+  } else {
+    LOG(ERROR)
+        << "Incorrect duration trace event (tid not a string or integer): "
+        << *event_dict;
+  }
   return true;
 }
 
 // Makes a unique key for an async event stack.
 bool GetAsyncEventStackId(base::DictionaryValue* event_dict,
                           AsyncEventStackId* asyncId) {
-  if (!event_dict->GetInteger("id", &asyncId->id)) {
-    LOG(ERROR) << "Incorrect async trace event (missing id).";
+  const base::Value* value;
+  if (!event_dict->Get("id", &value)) {
+    LOG(ERROR) << "Incorrect async trace event (missing id): " << *event_dict;
     return false;
   }
 
-  // We can have an empty category, but it is still relevant for event merging,
-  // per the documentation.
+  if (value->IsType(base::Value::TYPE_INTEGER)) {
+    int id_int;
+    // We already verified the type, so it should be an integer.
+    DCHECK(value->GetAsInteger(&id_int));
+    asyncId->id = base::IntToString(id_int);
+  } else if (value->IsType(base::Value::TYPE_STRING)) {
+    DCHECK(value->GetAsString(&asyncId->id));
+  } else {
+    LOG(ERROR) << "Incorrect async trace event (id not a string or integer): "
+               << *event_dict;
+  }
+
+  // We can have an empty category, but it is still relevant for event
+  // merging per the documentation.
   std::string cat;
   event_dict->GetString("cat", &asyncId->cat);
 
   return true;
 }
 
 // Given a beginning event, registers its beginning for future merging.
 template <typename T>
 void RegisterEventBegin(
     T key,
@@ skipping 112 matching lines @@
     } else {
       // Skip all event types we do not handle.
       continue;
     }
 
     if (!event_dict->GetString("name", &event.name)) {
       LOG(ERROR) << "Incorrect trace event (no name)";
       return false;
     }
 
-    if (!event_dict->GetString("cat", &event.categories)) {

[ppi, 2015/10/01 13:31:16]

Why this change?

[etiennej, 2015/10/01 13:41:47]

Some real-world events do not have a category, yet we don't want to ignore them.

-      LOG(WARNING) << "Ignoring incorrect trace event (no categories)";
-      continue;
-    }
-
     double timestamp;
     if (!event_dict->GetDouble("ts", &timestamp)) {
       LOG(WARNING) << "Ingoring incorrect trace event (no timestamp)";
       continue;
     }
     event.timestamp = base::TimeTicks::FromInternalValue(timestamp);
 
     if (event.type == EventType::COMPLETE) {
       double duration;
       if (!event_dict->GetDouble("dur", &duration)) {
@@ skipping 30 matching lines @@
   }
 
   if (!JoinEvents(event_list))
     return false;
 
   if (!ParseEvents(event_list, result))
     return false;
   return true;
 }
 }  // namespace benchmark