InspectorTimeline improvements

InspectorTimeline improvements

- Split out inspector-neutral parts of InspectorTimeline in preparation for
  about:tracing-based timeline
- Use event.name instead of type, which is in keeping with trace-viewer
- Use ms on units since the rest of telemetry uses seconds, but
  for precision reasons, ms makes sense to stick with
- Move arguments to event.args so that params get consistent treatment
  e.g. event.foo['bar'] is less consistent than event.args['foo']['bar']
- Improved test coverage of model, events, parsing
- event.self_time which represents
- drop events lacking start- or end- times

R=bulach@chromium.org
NOTRY=True

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

[nduca, 2013-01-09 10:56:33]

ptal

[bulach, 2013-01-09 11:17:17]

lgtm, good stuff! just some nits, and a suggestion to clarify the new method a
bit..

https://codereview.chromium.org/11818024/diff/1/tools/telemetry/telemetry/ins...
File tools/telemetry/telemetry/inspector_timeline.py (right):

https://codereview.chromium.org/11818024/diff/1/tools/telemetry/telemetry/ins...
tools/telemetry/telemetry/inspector_timeline.py:65: if msg.get('params',
{}).get('record'):
maybe:
record = msg.get('params', {}).get('record')
if record:
  ..
  68: s/msg['params']['record']/record/

https://codereview.chromium.org/11818024/diff/1/tools/telemetry/telemetry/ins...
tools/telemetry/telemetry/inspector_timeline.py:99: childs_parent =
parent_timeline_event
can 95-99 be outside the loop?
also, if I'm reading this right, it may be a good thing to add a docstring
saying that it mutates parent_timeline_event?
or perhaps expose a method with a single param that delegate to an internal one
with a parent_timeline_event?

https://codereview.chromium.org/11818024/diff/1/tools/telemetry/telemetry/ins...
File tools/telemetry/telemetry/inspector_timeline_unittest.py (right):

https://codereview.chromium.org/11818024/diff/1/tools/telemetry/telemetry/ins...
tools/telemetry/telemetry/inspector_timeline_unittest.py:53: None,
_SAMPLE_MESSAGE)
nit: indent+2 here and 56-57 below.

https://codereview.chromium.org/11818024/diff/1/tools/telemetry/telemetry/ins...
tools/telemetry/telemetry/inspector_timeline_unittest.py:54:
self.assertTrue(root_event != None)
I guess != None is redundant

https://codereview.chromium.org/11818024/diff/1/tools/telemetry/telemetry/tim...
File tools/telemetry/telemetry/timeline_event.py (right):

https://codereview.chromium.org/11818024/diff/1/tools/telemetry/telemetry/tim...
tools/telemetry/telemetry/timeline_event.py:1: # Copyright (c) 2012 The Chromium
Authors. All rights reserved.
happy new year! :)

https://codereview.chromium.org/11818024/diff/1/tools/telemetry/telemetry/tim...
tools/telemetry/telemetry/timeline_event.py:3: # found in the LICENSE file.
nit: \n

https://codereview.chromium.org/11818024/diff/1/tools/telemetry/telemetry/tim...
tools/telemetry/telemetry/timeline_event.py:19: """Time spent in this function
less any time spent in child events"""
nit: end with .

https://codereview.chromium.org/11818024/diff/1/tools/telemetry/telemetry/tim...
File tools/telemetry/telemetry/timeline_event_unittest.py (right):

https://codereview.chromium.org/11818024/diff/1/tools/telemetry/telemetry/tim...
tools/telemetry/telemetry/timeline_event_unittest.py:3: # found in the LICENSE
file.
nit: \n

https://codereview.chromium.org/11818024/diff/1/tools/telemetry/telemetry/tim...
tools/telemetry/telemetry/timeline_event_unittest.py:24:
self.assertEquals(a.self_time_ms, 1.75)
nit: # 2 - 0.25

https://codereview.chromium.org/11818024/diff/1/tools/telemetry/telemetry/tim...
tools/telemetry/telemetry/timeline_event_unittest.py:26: 
nit: extra \n

https://codereview.chromium.org/11818024/diff/1/tools/telemetry/telemetry/tim...
File tools/telemetry/telemetry/timeline_model.py (right):

https://codereview.chromium.org/11818024/diff/1/tools/telemetry/telemetry/tim...
tools/telemetry/telemetry/timeline_model.py:3: # found in the LICENSE file.
nit: \n

[nduca, 2013-01-09 11:55:33]

(notry because none of this code actually is affected by trybots)

[commit-bot: I haz the power, 2013-01-09 11:55:54]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nduca@chromium.org/11818024/7001

[commit-bot: I haz the power, 2013-01-09 11:56:44]

Change committed as 175782