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

Violations:
* If any conditional arms have braces, all arms must have braces.
* Don't handle DCHECK failure.
* No else after return
* Use DCHECK instead of CHECK unless CHECK is truly necessary
* Declare variables in the most local scope possible
* Prefer (foo == 0) to (0 == foo)
* Avoid unnecessary vertical whitespace
* All lines of args should begin at the same position

Not violations:
* Use DCHECK_EQ() where possible for better error messages
* Split compound DCHECKs into separate ones so it's easier to see which failed
* Reverse conditionals where doing so allows removing braces and reducing
  indenting of more lines
* Rewrap to avoid splitting a string constant
* Don't use at(), it buys nothing over [] when exceptions are off
* delete [] -> delete[]
* Avoid unnecessary temps

BUG=none
TEST=none
R=thakis@chromium.org

Review URL: https://codereview.chromium.org/203223008

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@258095 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 63 insertions, 80 deletions

diff --git a/base/test/trace_event_analyzer.cc b/base/test/trace_event_analyzer.cc
index 4f8225f..73fa519 100644
--- a/base/test/trace_event_analyzer.cc
+++ b/base/test/trace_event_analyzer.cc
@@ -91,15 +91,15 @@ bool TraceEvent::SetFromJSON(const base::Value* event_value) {
     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.
@@ -143,34 +143,30 @@ bool TraceEvent::HasNumberArg(const std::string& name) const {
 
 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);
+  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
@@ -259,9 +255,10 @@ bool Query::Evaluate(const TraceEvent& event) const {
   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())
@@ -270,7 +267,7 @@ bool Query::Evaluate(const TraceEvent& event) const {
     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;
   }
@@ -362,8 +359,9 @@ bool Query::CompareAsString(const TraceEvent& event, bool* result) const {
 
 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))
@@ -425,7 +423,7 @@ bool Query::GetAsString(const TraceEvent& event, std::string* str) const {
 
 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:
@@ -450,17 +448,15 @@ bool Query::GetMemberValueAsDouble(const TraceEvent& event,
       *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);
@@ -494,7 +490,7 @@ bool Query::GetMemberValueAsDouble(const TraceEvent& event,
 
 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:
@@ -733,8 +729,8 @@ void TraceAnalyzer::AssociateAsyncBeginEndEvents() {
 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.
@@ -838,7 +834,7 @@ void TraceAnalyzer::ParseMetadata() {
 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) {
@@ -892,9 +888,9 @@ 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;
     }
@@ -906,12 +902,12 @@ 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;
     }
@@ -927,7 +923,7 @@ bool FindClosest(const TraceEventVector& events,
                  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();
diff --git a/base/win/event_trace_consumer_unittest.cc b/base/win/event_trace_consumer_unittest.cc
index e46f564..ac14260 100644
--- a/base/win/event_trace_consumer_unittest.cc
+++ b/base/win/event_trace_consumer_unittest.cc
@@ -43,10 +43,9 @@ class TestConsumer: public EtwTraceConsumerBase<TestConsumer> {
   }
 
   void ClearQueue() {
-    EventQueue::const_iterator it(events_.begin()), end(events_.end());
-
-    for (; it != end; ++it) {
-      delete [] it->MofData;
+    for (EventQueue::const_iterator it(events_.begin()), end(events_.end());
+         it != end; ++it) {
+      delete[] it->MofData;
     }
 
     events_.clear();
@@ -56,7 +55,7 @@ class TestConsumer: public EtwTraceConsumerBase<TestConsumer> {
     events_.push_back(*event);
     EVENT_TRACE& back = events_.back();
 
-    if (NULL != event->MofData && 0 != event->MofLength) {
+    if (event->MofData != NULL && event->MofLength != 0) {
       back.MofData = new char[event->MofLength];
       memcpy(back.MofData, event->MofData, event->MofLength);
     }
@@ -112,14 +111,12 @@ TEST_F(EtwTraceConsumerBaseTest, Initialize) {
 
 TEST_F(EtwTraceConsumerBaseTest, OpenRealtimeSucceedsWhenNoSession) {
   TestConsumer consumer_;
-
   ASSERT_HRESULT_SUCCEEDED(
       consumer_.OpenRealtimeSession(session_name_.c_str()));
 }
 
 TEST_F(EtwTraceConsumerBaseTest, ConsumerImmediateFailureWhenNoSession) {
   TestConsumer consumer_;
-
   ASSERT_HRESULT_SUCCEEDED(
       consumer_.OpenRealtimeSession(session_name_.c_str()));
   ASSERT_HRESULT_FAILED(consumer_.Consume());
@@ -131,22 +128,18 @@ class EtwTraceConsumerRealtimeTest: public EtwTraceConsumerBaseTest {
  public:
   virtual void SetUp() {
     EtwTraceConsumerBaseTest::SetUp();
-
     ASSERT_HRESULT_SUCCEEDED(
         consumer_.OpenRealtimeSession(session_name_.c_str()));
   }
 
   virtual void TearDown() {
     consumer_.Close();
-
     EtwTraceConsumerBaseTest::TearDown();
   }
 
   DWORD ConsumerThread() {
     ::SetEvent(consumer_ready_.Get());
-
-    HRESULT hr = consumer_.Consume();
-    return hr;
+    return consumer_.Consume();
   }
 
   static DWORD WINAPI ConsumerThreadMainProc(void* arg) {
@@ -157,9 +150,9 @@ class EtwTraceConsumerRealtimeTest: public EtwTraceConsumerBaseTest {
   HRESULT StartConsumerThread() {
     consumer_ready_.Set(::CreateEvent(NULL, TRUE, FALSE, NULL));
     EXPECT_TRUE(consumer_ready_ != NULL);
-    consumer_thread_.Set(::CreateThread(NULL, 0, ConsumerThreadMainProc,
-        this, 0, NULL));
-    if (NULL == consumer_thread_.Get())
+    consumer_thread_.Set(::CreateThread(NULL, 0, ConsumerThreadMainProc, this,
+                                        0, NULL));
+    if (consumer_thread_.Get() == NULL)
       return HRESULT_FROM_WIN32(::GetLastError());
 
     HRESULT hr = S_OK;
@@ -173,13 +166,12 @@ class EtwTraceConsumerRealtimeTest: public EtwTraceConsumerBaseTest {
       case WAIT_OBJECT_0 + 1: {
           // The thread finished. This may race with the event, so check
           // explicitly for the event here, before concluding there's trouble.
-          if (WAIT_OBJECT_0 == ::WaitForSingleObject(consumer_ready_, 0))
+          if (::WaitForSingleObject(consumer_ready_, 0) == WAIT_OBJECT_0)
             return S_OK;
           DWORD exit_code = 0;
           if (::GetExitCodeThread(consumer_thread_, &exit_code))
             return exit_code;
-          else
-            return HRESULT_FROM_WIN32(::GetLastError());
+          return HRESULT_FROM_WIN32(::GetLastError());
           break;
         }
       default:
@@ -192,7 +184,7 @@ class EtwTraceConsumerRealtimeTest: public EtwTraceConsumerBaseTest {
 
   // Waits for consumer_ thread to exit, and returns its exit code.
   HRESULT JoinConsumerThread() {
-    if (WAIT_OBJECT_0 != ::WaitForSingleObject(consumer_thread_, INFINITE))
+    if (::WaitForSingleObject(consumer_thread_, INFINITE) != WAIT_OBJECT_0)
       return HRESULT_FROM_WIN32(::GetLastError());
 
     DWORD exit_code = 0;
@@ -211,10 +203,8 @@ class EtwTraceConsumerRealtimeTest: public EtwTraceConsumerBaseTest {
 
 TEST_F(EtwTraceConsumerRealtimeTest, ConsumerReturnsWhenSessionClosed) {
   EtwTraceController controller;
-
-  HRESULT hr = controller.StartRealtimeSession(session_name_.c_str(),
-                                               100 * 1024);
-  if (hr == E_ACCESSDENIED) {
+  if (controller.StartRealtimeSession(session_name_.c_str(), 100 * 1024) ==
+      E_ACCESSDENIED) {
     VLOG(1) << "You must be an administrator to run this test on Vista";
     return;
   }
@@ -224,7 +214,6 @@ TEST_F(EtwTraceConsumerRealtimeTest, ConsumerReturnsWhenSessionClosed) {
 
   // Wait around for the consumer_ thread a bit.
   ASSERT_EQ(WAIT_TIMEOUT, ::WaitForSingleObject(consumer_thread_, 50));
-
   ASSERT_HRESULT_SUCCEEDED(controller.Stop(NULL));
 
   // The consumer_ returns success on session stop.
@@ -234,16 +223,16 @@ TEST_F(EtwTraceConsumerRealtimeTest, ConsumerReturnsWhenSessionClosed) {
 namespace {
 
 // {57E47923-A549-476f-86CA-503D57F59E62}
-DEFINE_GUID(kTestEventType,
-  0x57e47923, 0xa549, 0x476f, 0x86, 0xca, 0x50, 0x3d, 0x57, 0xf5, 0x9e, 0x62);
+DEFINE_GUID(
+    kTestEventType,
+    0x57e47923, 0xa549, 0x476f, 0x86, 0xca, 0x50, 0x3d, 0x57, 0xf5, 0x9e, 0x62);
 
 }  // namespace
 
 TEST_F(EtwTraceConsumerRealtimeTest, ConsumeEvent) {
   EtwTraceController controller;
-  HRESULT hr = controller.StartRealtimeSession(session_name_.c_str(),
-                                               100 * 1024);
-  if (hr == E_ACCESSDENIED) {
+  if (controller.StartRealtimeSession(session_name_.c_str(), 100 * 1024) ==
+      E_ACCESSDENIED) {
     VLOG(1) << "You must be an administrator to run this test on Vista";
     return;
   }
@@ -256,12 +245,10 @@ TEST_F(EtwTraceConsumerRealtimeTest, ConsumeEvent) {
 
   // Start the consumer_.
   ASSERT_HRESULT_SUCCEEDED(StartConsumerThread());
-
   ASSERT_EQ(0, TestConsumer::events_.size());
 
   EtwMofEvent<1> event(kTestEventType, 1, TRACE_LEVEL_ERROR);
   EXPECT_EQ(ERROR_SUCCESS, provider.Log(&event.header));
-
   EXPECT_EQ(WAIT_OBJECT_0, ::WaitForSingleObject(TestConsumer::sank_event_,
                                                  INFINITE));
   ASSERT_HRESULT_SUCCEEDED(controller.Stop(NULL));
@@ -306,8 +293,8 @@ class EtwTraceConsumerDataTest: public EtwTraceConsumerBaseTest {
       return hr;
 
     // Enable our provider.
-    EXPECT_HRESULT_SUCCEEDED(controller.EnableProvider(test_provider_,
-        TRACE_LEVEL_VERBOSE, 0xFFFFFFFF));
+    EXPECT_HRESULT_SUCCEEDED(controller.EnableProvider(
+        test_provider_, TRACE_LEVEL_VERBOSE, 0xFFFFFFFF));
 
     EtwTraceProvider provider(test_provider_);
     // Then register our provider, means we get a session handle immediately.
@@ -374,7 +361,7 @@ TEST_F(EtwTraceConsumerDataTest, RoundTrip) {
     return;
   }
   ASSERT_HRESULT_SUCCEEDED(hr) << "RoundTripEvent failed";
-  ASSERT_TRUE(NULL != trace);
+  ASSERT_TRUE(trace != NULL);
   ASSERT_EQ(sizeof(kData), trace->MofLength);
   ASSERT_STREQ(kData, reinterpret_cast<const char*>(trace->MofData));
 }