Move timeline and importers to use telemetry.value.TraceValue

Move timeline and importers to use telemetry.value.TraceValue

This (admittedly massive) patch actually gives us an ubertrace object: the
TraceValue. The key way to understand this CL is to read the trace.py file,
then read importer.py. The rest is cosmetic fallout from the signature changes.

A TraceValue contains the data from many tracers, and is always serializable. A
TimelineModel is constructed from a single TimelineValue. Internally, a
TraceValue is an in memory version of json of the following form:
  { traceEvents: ...
    inspectorTimelineEvnets: ...
    tabIds: ...
  }

Since a TracedValue has all the tracer data, the model and importer system was
updated so that instead of there being 1 importer per value, the importers
instead indicate which part of the TraceValue they can handle, if any.

BUG=356763

[nduca, 2014-08-05 12:22:34]

early preview. probably ready for review by Tuesday night.

[nduca, 2014-08-05 12:23:51]

+otherned

[nednguyen, 2014-08-05 14:34:34]

Looks solid to me. My only concern is I am not sure whether video can be fit
into TraceValue.

https://codereview.chromium.org/441873007/diff/1/tools/telemetry/telemetry/pa...
File tools/telemetry/telemetry/page/actions/action_runner_unittest.py (right):

https://codereview.chromium.org/441873007/diff/1/tools/telemetry/telemetry/pa...
tools/telemetry/telemetry/page/actions/action_runner_unittest.py:19: def
GetInteractionRecords(self, trace_value):
Something seems wrong with the indentation.

https://codereview.chromium.org/441873007/diff/1/tools/telemetry/telemetry/ti...
File tools/telemetry/telemetry/timeline/importer.py (right):

https://codereview.chromium.org/441873007/diff/1/tools/telemetry/telemetry/ti...
tools/telemetry/telemetry/timeline/importer.py:8: """Reads TracedValue and
populates timeline model with what it finds."""
nit: TraceValue

https://codereview.chromium.org/441873007/diff/1/tools/telemetry/telemetry/va...
File tools/telemetry/telemetry/value/trace.py (right):

https://codereview.chromium.org/441873007/diff/1/tools/telemetry/telemetry/va...
tools/telemetry/telemetry/value/trace.py:13: 
Can None be raw data?
For example: {1:None, 2:'foo'}

https://codereview.chromium.org/441873007/diff/1/tools/telemetry/telemetry/va...
tools/telemetry/telemetry/value/trace.py:25: raise Exception('%s is not allowed
in TraceValue', raw)
Throwing exception at the leaf level may make it hard to see what went wrong if
we have a huge dictionary.

https://codereview.chromium.org/441873007/diff/1/tools/telemetry/telemetry/va...
tools/telemetry/telemetry/value/trace.py:29: # aare accessed by name, using the
following fixed field names.
nit: are

https://codereview.chromium.org/441873007/diff/1/tools/telemetry/telemetry/va...
tools/telemetry/telemetry/value/trace.py:45: class
RawTraceValueHelpersMixin(object):
Don't you need __init__(self) to define self._raw_data?

https://codereview.chromium.org/441873007/diff/1/tools/telemetry/telemetry/va...
tools/telemetry/telemetry/value/trace.py:69: 'value' : self._raw_data[v]
'value': v ?

https://codereview.chromium.org/441873007/diff/1/tools/telemetry/telemetry/va...
tools/telemetry/telemetry/value/trace.py:84: raw_data can be any of the three
standard trace_event formats:
So we will need something else for video and images?

https://codereview.chromium.org/441873007/diff/1/tools/telemetry/telemetry/va...
tools/telemetry/telemetry/value/trace.py:130: self._raw_data = {'traceEvents':
raw_data}
How about self._raw_data = { CHROME_TRACE_PART.rawFieldName : raw_data}?

[chrishenry, 2014-08-05 23:44:16]

https://codereview.chromium.org/441873007/diff/1/tools/telemetry/telemetry/va...
File tools/telemetry/telemetry/value/trace.py (right):

https://codereview.chromium.org/441873007/diff/1/tools/telemetry/telemetry/va...
tools/telemetry/telemetry/value/trace.py:30: class TraceValuePart(object):
Just TracePart? TraceValuePart is a bit ambiguous.

https://codereview.chromium.org/441873007/diff/1/tools/telemetry/telemetry/va...
tools/telemetry/telemetry/value/trace.py:32: self.rawFieldName = rawFieldName
nit: raw_field_name

https://codereview.chromium.org/441873007/diff/1/tools/telemetry/telemetry/va...
tools/telemetry/telemetry/value/trace.py:45: class
RawTraceValueHelpersMixin(object):
Does TraceValue and Builder really need to mixin this? Maybe they can just be
free functions.

https://codereview.chromium.org/441873007/diff/1/tools/telemetry/telemetry/va...
tools/telemetry/telemetry/value/trace.py:64: for k,v in
self._raw_data.iteritems():
nit: k, v (space after ,)

https://codereview.chromium.org/441873007/diff/1/tools/telemetry/telemetry/va...
tools/telemetry/telemetry/value/trace.py:68: 'name' : k,
nit: no space before :

https://codereview.chromium.org/441873007/diff/1/tools/telemetry/telemetry/va...
tools/telemetry/telemetry/value/trace.py:100: if not isinstance(raw_data,
basestring):
I think it's clearer to refactor TryInitFromRaw and this next section into
something like:

# Parse into json object if raw_value is a string.
if isinstance(..., basestring):
  json_data = _ParseFromString(...)
  # The parsed data isn't shared with anyone else, so we mark this value as
safely mutable.
  self._events_are_safely_mutable = True
else:
  json_data = raw_data

if isinstance(json_data, dict):
  ...
elif isinstance(json_data, list):
  ...
else:
  raise Exception('Unrecognized...')

https://codereview.chromium.org/441873007/diff/1/tools/telemetry/telemetry/va...
tools/telemetry/telemetry/value/trace.py:137: self._event_data_is_mutable = True
Unused? Or do you mean _events_are_safely_mutable?

https://codereview.chromium.org/441873007/diff/1/tools/telemetry/telemetry/va...
tools/telemetry/telemetry/value/trace.py:144: them to the model, changing them
subtly as they do so. If the TraceValue was
Do these importers modify the raw data in this TraceValue? You mentioned below
that TraceValue is supposed to be immutable, is that not true then?

https://codereview.chromium.org/441873007/diff/1/tools/telemetry/telemetry/va...
tools/telemetry/telemetry/value/trace.py:150: value came from
incomplete sentence?

https://codereview.chromium.org/441873007/diff/1/tools/telemetry/telemetry/va...
tools/telemetry/telemetry/value/trace.py:175: res = TraceValue()
nit: s/res/value/

https://codereview.chromium.org/441873007/diff/1/tools/telemetry/telemetry/va...
tools/telemetry/telemetry/value/trace.py:183: # out and doesn't write a timebase
that can be used to re-sync them.
Turn this into pydoc?

[slamm, 2014-08-12 23:11:59]

Drive by.

https://codereview.chromium.org/441873007/diff/1/tools/telemetry/telemetry/co...
File tools/telemetry/telemetry/core/backends/chrome/chrome_browser_backend.py
(right):

https://codereview.chromium.org/441873007/diff/1/tools/telemetry/telemetry/co...
tools/telemetry/telemetry/core/backends/chrome/chrome_browser_backend.py:282:
self._tracing_backend.StopTracing(builder)
builder -> trace_value_builder ?

https://codereview.chromium.org/441873007/diff/1/tools/telemetry/telemetry/co...
File
tools/telemetry/telemetry/core/platform/profiler/android_systrace_profiler.py
(right):

https://codereview.chromium.org/441873007/diff/1/tools/telemetry/telemetry/co...
tools/telemetry/telemetry/core/platform/profiler/android_systrace_profiler.py:58:
self._browser_backend.StopTracing(builder)
builder -> trace_result_builder ?

https://codereview.chromium.org/441873007/diff/1/tools/telemetry/telemetry/co...
File tools/telemetry/telemetry/core/platform/profiler/trace_profiler.py (right):

https://codereview.chromium.org/441873007/diff/1/tools/telemetry/telemetry/co...
tools/telemetry/telemetry/core/platform/profiler/trace_profiler.py:35:
self._browser_backend.StopTracing(builder)
builder -> trace_result_builder

https://codereview.chromium.org/441873007/diff/1/tools/telemetry/telemetry/ti...
File tools/telemetry/telemetry/timeline/model.py (right):

https://codereview.chromium.org/441873007/diff/1/tools/telemetry/telemetry/ti...
tools/telemetry/telemetry/timeline/model.py:245: importers.sort(lambda k:
k.import_order)
Are you meaning to use cmp or key here?

importers.sort(key=lambda k: k.import_order)

https://codereview.chromium.org/441873007/diff/1/tools/telemetry/telemetry/ti...
File tools/telemetry/telemetry/timeline/tab_id_importer.py (right):

https://codereview.chromium.org/441873007/diff/1/tools/telemetry/telemetry/ti...
tools/telemetry/telemetry/timeline/tab_id_importer.py:36: # trace buffer
overflow.    for process in self._model.GetAllProcesses():
Missing newline.

https://codereview.chromium.org/441873007/diff/1/tools/telemetry/telemetry/ti...
tools/telemetry/telemetry/timeline/tab_id_importer.py:52:
assert(timeline_markers[0].start_thread ==
Needs line continuation.

https://codereview.chromium.org/441873007/diff/1/tools/telemetry/telemetry/va...
File tools/telemetry/telemetry/value/trace.py (right):

https://codereview.chromium.org/441873007/diff/1/tools/telemetry/telemetry/va...
tools/telemetry/telemetry/value/trace.py:63: part_field_names =
set([p.rawFieldName for p in ALL_TRACE_VALUE_PARTS])
You can build the set with a generator expression instead of a list:

part_field_names = set(p.rawFieldName for p in ALL_TRACE_VALUE_PARTS)

https://codereview.chromium.org/441873007/diff/1/tools/telemetry/telemetry/va...
tools/telemetry/telemetry/value/trace.py:104: self._raw_data == {}
self._raw_data = {} ?
Same in _TryInitFromRaw.

https://codereview.chromium.org/441873007/diff/1/tools/telemetry/telemetry/va...
tools/telemetry/telemetry/value/trace.py:107: if raw_data[0] == '[' and
raw_data[-1] != ']':
Better?

if raw_data.startswith('[') and not raw_data.endswith(']'):
  if raw_data.endswith(','):
    raw_data = raw_data[:-1]
  raw_data += ']'