Clean up some API in trace_event_analyzer.h and add some getters.

base/test/trace_event_analyzer.h

 // Copyright (c) 2011 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.
 
 // Use trace_analyzer::Query and trace_analyzer::TraceAnalyzer to search for
 // specific trace events that were generated by the trace_event.h API.
 //
 // Basic procedure:
 // - Get trace events JSON string from base::debug::TraceLog.
 // - Create TraceAnalyzer with JSON string.
@@ skipping 105 matching lines @@
 
   bool SetFromJSON(const base::Value* event_value) WARN_UNUSED_RESULT;
 
   bool operator< (const TraceEvent& rhs) const {
     return timestamp < rhs.timestamp;
   }
 
   bool has_other_event() const { return other_event; }
 
   // Returns absolute duration in microseconds between this event and other
-  // event. Returns false if has_other_event() is false.
-  bool GetAbsTimeToOtherEvent(double* duration) const;
+  // event. Must have already verified that other_event exists by
+  // Query(EVENT_HAS_OTHER) or by calling has_other_event().
+  double GetAbsTimeToOtherEvent() const;

[nduca, 2011/11/03 19:05:53]

What happens if you forget to do this? DCHECK or a specific return value?

[jbates, 2011/11/03 19:34:39]

The behavior is to crash when other_event doesn't exist so that the test will
not accidentally pass.

 
   // Return the argument value if it exists and it is a string.
   bool GetArgAsString(const std::string& name, std::string* arg) const;
   // Return the argument value if it exists and it is a number.
   bool GetArgAsNumber(const std::string& name, double* arg) const;
 
+  // Check if argument exists and is string.
+  bool HasStringArg(const std::string& name) const;
+  // Check if argument exists and is number (double, int or bool).
+  bool HasNumberArg(const std::string& name) const;
+
+  // Get known existing arguments as specific types.
+  // Useful when you have already queried the argument with
+  // Query(HAS_NUMBER_ARG) or Query(HAS_STRING_ARG).
+  std::string GetKnownArgAsString(const std::string& name) const;
+  double GetKnownArgAsDouble(const std::string& name) const;
+  int GetKnownArgAsInt(const std::string& name) const;
+  bool GetKnownArgAsBool(const std::string& name) const;
+

[nduca, 2011/11/03 19:05:53]

I guess by moving to this CL, I didn't get your response to why you're doing
this and not

bool GetArg(name, value*)?

What's the behavior if GetKnownArgAsX fails?

[jbates, 2011/11/03 19:34:39]

What I realized (tried to explain it in the comment above -- let me know if it's
not clear) is that the typical usage of the Query API makes additional checking
here redundant and it's often inconvenient to require named return parameters
(it breaks the functional flow of code).

The behavior is to assert when the API is not used correctly (so that tests
don't accidentally pass). Consider std::vector: it allows you to access data at
particular indices and crashes if the index is out of bounds.

   // Process ID and Thread ID.
   ProcessThreadID thread;
 
   // Time since epoch in microseconds.
   // Stored as double to match its JSON representation.
   double timestamp;
 
   base::debug::TraceEventPhase phase;
 
   std::string category;
 
   std::string name;
 
   // All numbers and bool values from TraceEvent args are cast to double.
   // bool becomes 1.0 (true) or 0.0 (false).
   std::map<std::string, double> arg_numbers;
 
   std::map<std::string, std::string> arg_strings;
 
   // The other event associated with this event (or NULL).
   const TraceEvent* other_event;
 };
 
 // Pass these values to Query to compare with the corresponding member of a
-// TraceEvent.
+// TraceEvent. Unless otherwise specfied, the usage is Query(ENUM_MEMBER).
 enum TraceEventMember {
   EVENT_INVALID,
   // Use these to access the event members:
   EVENT_PID,
   EVENT_TID,
   // Return the timestamp of the event in microseconds since epoch.
   EVENT_TIME,
   // Return the absolute time between event and other event in microseconds.
   // Only works for events with associated BEGIN/END: Query(EVENT_HAS_OTHER).
   EVENT_DURATION,
   EVENT_PHASE,
   EVENT_CATEGORY,
   EVENT_NAME,
-  EVENT_HAS_ARG,
+
+  // Evaluates to true if arg exists and is a string.
+  // Usage: Query(EVENT_HAS_STRING_ARG, "arg_name")
+  EVENT_HAS_STRING_ARG,
+  // Evaluates to true if arg exists and is a number.
+  // Number arguments include types double, int and bool.
+  // Usage: Query(EVENT_HAS_NUMBER_ARG, "arg_name")
+  EVENT_HAS_NUMBER_ARG,
+  // Evaluates to arg value (string or number).
+  // Usage: Query(EVENT_ARG, "arg_name")
   EVENT_ARG,
   // Return true if associated event exists.
   // (Typically BEGIN for END or END for BEGIN).
   EVENT_HAS_OTHER,
-  // Use these to access the associated event's members:
+
+  // Access the associated other_event's members:
   OTHER_PID,
   OTHER_TID,
   OTHER_TIME,
   OTHER_PHASE,
   OTHER_CATEGORY,
   OTHER_NAME,
-  OTHER_HAS_ARG,
-  OTHER_ARG
+
+  // Evaluates to true if arg exists and is a string.
+  // Usage: Query(EVENT_HAS_STRING_ARG, "arg_name")
+  OTHER_HAS_STRING_ARG,
+  // Evaluates to true if arg exists and is a number.
+  // Number arguments include types double, int and bool.
+  // Usage: Query(EVENT_HAS_NUMBER_ARG, "arg_name")
+  OTHER_HAS_NUMBER_ARG,
+  // Evaluates to arg value (string or number).
+  // Usage: Query(EVENT_ARG, "arg_name")
+  OTHER_ARG,
 };
 
 class Query {
  public:
   // Compare with the given member.
   Query(TraceEventMember member);
 
   // Compare with the given member argument value.
   Query(TraceEventMember member, const std::string& arg_name);
 
@@ skipping 243 matching lines @@
   std::map<TraceEvent::ProcessThreadID, std::string> thread_names_;
   std::vector<TraceEvent> raw_events_;
   bool allow_assocation_changes_;
 
   DISALLOW_COPY_AND_ASSIGN(TraceAnalyzer);
 };
 
 }  // namespace trace_analyzer
 
 #endif  // BASE_TEST_TRACE_EVENT_ANALYZER_H_