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

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

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=258095

[Peter Kasting, 2014-03-18 19:28:12]

As requested on https://codereview.chromium.org/202993003/ , here are style
changes from that patch as a standalone patch.  The CL description lists
everything that's changed.

Compared to the original CL, I threw away all cases of adding vertical
whitespace, and I threw away all cases of introducing ternary operators -- both
of which you called out as unnecessary.  Please feel free to yowl if you still
feel there are things in here which are unhelpful.

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

lgtm!

("* Don't use at(), it buys nothing over [] when exceptions are off" in the cl
description isn't true though; at least in libstdc++, at() still does bounds
checking and calls abort() instead of throwing when it's built without
exceptions. Usually, libstdc++ is built with exceptions though and just its
clients are built without exceptions; in that case it still throws and the
client won't be able to catch that and then abort because of that – so at()
still gives you a guaranteed crash on out-of-bounds, which operator[] doesn't.)

https://codereview.chromium.org/203223008/diff/10001/base/test/trace_event_an...
File base/test/trace_event_analyzer.cc (right):

https://codereview.chromium.org/203223008/diff/10001/base/test/trace_event_an...
base/test/trace_event_analyzer.cc:146: bool result = GetArgAsString(name,
&arg_string);
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:15]

(I replied to the at() stuff back on the other CL.)

https://codereview.chromium.org/203223008/diff/10001/base/test/trace_event_an...
File base/test/trace_event_analyzer.cc (right):

https://codereview.chromium.org/203223008/diff/10001/base/test/trace_event_an...
base/test/trace_event_analyzer.cc:146: bool result = GetArgAsString(name,
&arg_string);
On 2014/03/19 10:29:21, Nico (on GMT time Mar 15 - 24) wrote:
> Not sure I love this change, as GetArgAsString doesn't guarantee to not touch
> arg_string when it returns false.

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.

[Peter Kasting, 2014-03-19 21:15:56]

Committed patchset #2 manually as r258095 (presubmit successful).