Style changes split off of https://codereview.chromium.org/202993003/ .

base/test/trace_event_analyzer.cc

 // Copyright (c) 2012 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 "base/test/trace_event_analyzer.h"
 
 #include <algorithm>
 #include <math.h>
 #include <set>
 
@@ skipping 73 matching lines @@
     return false;
   }
 
   // For each argument, copy the type and create a trace_analyzer::TraceValue.
   for (base::DictionaryValue::Iterator it(*args); !it.IsAtEnd();
        it.Advance()) {
     std::string str;
     bool boolean = false;
     int int_num = 0;
     double double_num = 0.0;
-    if (it.value().GetAsString(&str))
+    if (it.value().GetAsString(&str)) {
       arg_strings[it.key()] = str;
-    else if (it.value().GetAsInteger(&int_num))
+    } else if (it.value().GetAsInteger(&int_num)) {
       arg_numbers[it.key()] = static_cast<double>(int_num);
-    else if (it.value().GetAsBoolean(&boolean))
+    } else if (it.value().GetAsBoolean(&boolean)) {
       arg_numbers[it.key()] = static_cast<double>(boolean ? 1 : 0);
-    else if (it.value().GetAsDouble(&double_num))
+    } else if (it.value().GetAsDouble(&double_num)) {
       arg_numbers[it.key()] = double_num;
-    else {
+    } else {
       LOG(WARNING) << "Value type of argument is not supported: " <<
           static_cast<int>(it.value().GetType());
       continue;  // Skip non-supported arguments.
     }
   }
 
   return true;
 }
 
 double TraceEvent::GetAbsTimeToOtherEvent() const {
@@ skipping 23 matching lines @@
 bool TraceEvent::HasStringArg(const std::string& name) const {
   return (arg_strings.find(name) != arg_strings.end());
 }
 
 bool TraceEvent::HasNumberArg(const std::string& name) const {
   return (arg_numbers.find(name) != arg_numbers.end());
 }
 
 std::string TraceEvent::GetKnownArgAsString(const std::string& name) const {
   std::string arg_string;
-  if (GetArgAsString(name, &arg_string))
-    return arg_string;
-  NOTREACHED();
-  return std::string();
+  bool result = GetArgAsString(name, &arg_string);

[Nico, 2014/03/19 10:29:21]

Not sure I love this change, as GetArgAsString doesn't guarantee to not touch
arg_string when it returns false.

[Peter Kasting, 2014/03/19 21:14:16]

True.  But the DCHECKs here say it can never return false in these cases, so
it's safe not to worry.  If it was possible, even in exceptional cases, for this
to return false here, we couldn't use DCHECK/NOTREACHED to begin with.

+  DCHECK(result);
+  return arg_string;
 }
 
 double TraceEvent::GetKnownArgAsDouble(const std::string& name) const {
   double arg_double;
-  if (GetArgAsNumber(name, &arg_double))
-    return arg_double;
-  NOTREACHED();
-  return 0;
+  bool result = GetArgAsNumber(name, &arg_double);
+  DCHECK(result);
+  return arg_double;
 }
 
 int TraceEvent::GetKnownArgAsInt(const std::string& name) const {
   double arg_double;
-  if (GetArgAsNumber(name, &arg_double))
-    return static_cast<int>(arg_double);
-  NOTREACHED();
-  return 0;
+  bool result = GetArgAsNumber(name, &arg_double);
+  DCHECK(result);
+  return static_cast<int>(arg_double);
 }
 
 bool TraceEvent::GetKnownArgAsBool(const std::string& name) const {
   double arg_double;
-  if (GetArgAsNumber(name, &arg_double))
-    return (arg_double != 0.0);
-  NOTREACHED();
-  return false;
+  bool result = GetArgAsNumber(name, &arg_double);
+  DCHECK(result);
+  return (arg_double != 0.0);
 }
 
 // QueryNode
 
 QueryNode::QueryNode(const Query& query) : query_(query) {
 }
 
 QueryNode::~QueryNode() {
 }
 
@@ skipping 68 matching lines @@
   bool is_bool = GetAsDouble(event, &bool_value);
   if (is_bool)
     return (bool_value != 0.0);
 
   // string is true if it is non-empty:
   std::string str_value;
   bool is_str = GetAsString(event, &str_value);
   if (is_str)
     return !str_value.empty();
 
-  DCHECK(type_ == QUERY_BOOLEAN_OPERATOR)
+  DCHECK_EQ(QUERY_BOOLEAN_OPERATOR, type_)
       << "Invalid query: missing boolean expression";
-  DCHECK(left_.get() && (right_.get() || is_unary_operator()));
+  DCHECK(left_.get());
+  DCHECK(right_.get() || is_unary_operator());
 
   if (is_comparison_operator()) {
     DCHECK(left().is_value() && right().is_value())
         << "Invalid query: comparison operator used between event member and "
            "value.";
     bool compare_result = false;
     if (CompareAsDouble(event, &compare_result))
       return compare_result;
-    else if (CompareAsString(event, &compare_result))
+    if (CompareAsString(event, &compare_result))
       return compare_result;
     return false;
   }
   // It's a logical operator.
   switch (operator_) {
     case OP_AND:
       return left().Evaluate(event) && right().Evaluate(event);
     case OP_OR:
       return left().Evaluate(event) || right().Evaluate(event);
     case OP_NOT:
@@ skipping 71 matching lines @@
       return true;
     default:
       NOTREACHED();
       return false;
   }
   return true;
 }
 
 bool Query::EvaluateArithmeticOperator(const TraceEvent& event,
                                        double* num) const {
-  DCHECK(type_ == QUERY_ARITHMETIC_OPERATOR);
-  DCHECK(left_.get() && (right_.get() || is_unary_operator()));
+  DCHECK_EQ(QUERY_ARITHMETIC_OPERATOR, type_);
+  DCHECK(left_.get());
+  DCHECK(right_.get() || is_unary_operator());
 
   double lhs = 0, rhs = 0;
   if (!left().GetAsDouble(event, &lhs))
     return false;
   if (!is_unary_operator() && !right().GetAsDouble(event, &rhs))
     return false;
 
   switch (operator_) {
     case OP_ADD:
       *num = lhs + rhs;
@@ skipping 41 matching lines @@
     case QUERY_STRING:
       *str = string_;
       return true;
     default:
       return false;
   }
 }
 
 bool Query::GetMemberValueAsDouble(const TraceEvent& event,
                                    double* num) const {
-  DCHECK(type_ == QUERY_EVENT_MEMBER);
+  DCHECK_EQ(QUERY_EVENT_MEMBER, type_);
 
   // This could be a request for a member of |event| or a member of |event|'s
   // associated event. Store the target event in the_event:
   const TraceEvent* the_event = (member_ < OTHER_PID) ?
       &event : event.other_event;
 
   // Request for member of associated event, but there is no associated event.
   if (!the_event)
     return false;
 
   switch (member_) {
     case EVENT_PID:
     case OTHER_PID:
       *num = static_cast<double>(the_event->thread.process_id);
       return true;
     case EVENT_TID:
     case OTHER_TID:
       *num = static_cast<double>(the_event->thread.thread_id);
       return true;
     case EVENT_TIME:
     case OTHER_TIME:
       *num = the_event->timestamp;
       return true;
     case EVENT_DURATION:
-      if (the_event->has_other_event()) {
-        *num = the_event->GetAbsTimeToOtherEvent();
-        return true;
-      }
-      return false;
+      if (!the_event->has_other_event())
+        return false;
+      *num = the_event->GetAbsTimeToOtherEvent();
+      return true;
     case EVENT_COMPLETE_DURATION:
-      if (the_event->phase == TRACE_EVENT_PHASE_COMPLETE) {
-        *num = the_event->duration;
-        return true;
-      }
-      return false;
+      if (the_event->phase != TRACE_EVENT_PHASE_COMPLETE)
+        return false;
+      *num = the_event->duration;
+      return true;
     case EVENT_PHASE:
     case OTHER_PHASE:
       *num = static_cast<double>(the_event->phase);
       return true;
     case EVENT_HAS_STRING_ARG:
     case OTHER_HAS_STRING_ARG:
       *num = (the_event->HasStringArg(string_) ? 1.0 : 0.0);
       return true;
     case EVENT_HAS_NUMBER_ARG:
     case OTHER_HAS_NUMBER_ARG:
@@ skipping 13 matching lines @@
       // return 1.0 (true) if the other event exists
       *num = event.other_event ? 1.0 : 0.0;
       return true;
     default:
       return false;
   }
 }
 
 bool Query::GetMemberValueAsString(const TraceEvent& event,
                                    std::string* str) const {
-  DCHECK(type_ == QUERY_EVENT_MEMBER);
+  DCHECK_EQ(QUERY_EVENT_MEMBER, type_);
 
   // This could be a request for a member of |event| or a member of |event|'s
   // associated event. Store the target event in the_event:
   const TraceEvent* the_event = (member_ < OTHER_PID) ?
       &event : event.other_event;
 
   // Request for member of associated event, but there is no associated event.
   if (!the_event)
     return false;
 
@@ skipping 218 matching lines @@
   Query match(Query::EventName() == Query::OtherName() &&
               Query::EventCategory() == Query::OtherCategory() &&
               Query::EventId() == Query::OtherId());
 
   AssociateEvents(begin, end, match);
 }
 
 void TraceAnalyzer::AssociateEvents(const Query& first,
                                     const Query& second,
                                     const Query& match) {
-  DCHECK(allow_assocation_changes_) << "AssociateEvents not allowed after "
-                                      "FindEvents";
+  DCHECK(allow_assocation_changes_)
+      << "AssociateEvents not allowed after FindEvents";
 
   // Search for matching begin/end event pairs. When a matching end is found,
   // it is associated with the begin event.
   std::vector<TraceEvent*> begin_stack;
   for (size_t event_index = 0; event_index < raw_events_.size();
        ++event_index) {
 
     TraceEvent& this_event = raw_events_[event_index];
 
     if (second.Evaluate(this_event)) {
@@ skipping 83 matching lines @@
     if (string_it != this_event.arg_strings.end())
       thread_names_[this_event.thread] = string_it->second;
   }
 }
 
 // TraceEventVector utility functions.
 
 bool GetRateStats(const TraceEventVector& events,
                   RateStats* stats,
                   const RateStatsOptions* options) {
-  CHECK(stats);
+  DCHECK(stats);
   // Need at least 3 events to calculate rate stats.
   const size_t kMinEvents = 3;
   if (events.size() < kMinEvents) {
     LOG(ERROR) << "Not enough events: " << events.size();
     return false;
   }
 
   std::vector<double> deltas;
   size_t num_deltas = events.size() - 1;
   for (size_t i = 0; i < num_deltas; ++i) {
@@ skipping 33 matching lines @@
   stats->standard_deviation_us =
       sqrt(sum_mean_offsets_squared / static_cast<double>(num_deltas - 1));
 
   return true;
 }
 
 bool FindFirstOf(const TraceEventVector& events,
                  const Query& query,
                  size_t position,
                  size_t* return_index) {
-  CHECK(return_index);
+  DCHECK(return_index);
   for (size_t i = position; i < events.size(); ++i) {
-    if (query.Evaluate(*events.at(i))) {
+    if (query.Evaluate(*events[i])) {
       *return_index = i;
       return true;
     }
   }
   return false;
 }
 
 bool FindLastOf(const TraceEventVector& events,
                 const Query& query,
                 size_t position,
                 size_t* return_index) {
-  CHECK(return_index);
+  DCHECK(return_index);
   if (events.empty())
     return false;
   position = (position < events.size()) ? position : events.size() - 1;
   for (;;) {
-    if (query.Evaluate(*events.at(position))) {
+    if (query.Evaluate(*events[position])) {
       *return_index = position;
       return true;
     }
     if (position == 0)
       return false;
     --position;
   }
   return false;
 }
 
 bool FindClosest(const TraceEventVector& events,
                  const Query& query,
                  size_t position,
                  size_t* return_closest,
                  size_t* return_second_closest) {
-  CHECK(return_closest);
+  DCHECK(return_closest);
   if (events.empty() || position >= events.size())
     return false;
   size_t closest = events.size();
   size_t second_closest = events.size();
   for (size_t i = 0; i < events.size(); ++i) {
     if (!query.Evaluate(*events.at(i)))
       continue;
     if (closest == events.size()) {
       closest = i;
       continue;
@@ skipping 27 matching lines @@
   end_position = (end_position < events.size()) ? end_position : events.size();
   size_t count = 0u;
   for (size_t i = begin_position; i < end_position; ++i) {
     if (query.Evaluate(*events.at(i)))
       ++count;
   }
   return count;
 }
 
 }  // namespace trace_analyzer