tools/android/loading: Archive tracks in LoadingTrace.

tools/android/loading: Archive tracks in LoadingTrace.

Committed: https://crrev.com/6cf3e7e1f3cb4bd595f3ddabcc126ead2e10ddf7
Cr-Commit-Position: refs/heads/master@{#370121}

[Benoit L, 2016-01-19 14:53:57]

lizeb@chromium.org changed reviewers:
+ mattcary@chromium.org

[mattcary, 2016-01-19 15:24:47]

lgtm

https://codereview.chromium.org/1606903002/diff/40001/tools/android/loading/l...
File tools/android/loading/loading_trace.py (right):

https://codereview.chromium.org/1606903002/diff/40001/tools/android/loading/l...
tools/android/loading/loading_trace.py:31: result = {'url': self.url,
'metadata': self.metadata,
Turn the keys into class constants to avoid typos?

https://codereview.chromium.org/1606903002/diff/40001/tools/android/loading/t...
File tools/android/loading/trace_recorder.py (right):

https://codereview.chromium.org/1606903002/diff/40001/tools/android/loading/t...
tools/android/loading/trace_recorder.py:63: 
two blank lines?

https://codereview.chromium.org/1606903002/diff/40001/tools/android/loading/t...
File tools/android/loading/tracing.py (right):

https://codereview.chromium.org/1606903002/diff/40001/tools/android/loading/t...
tools/android/loading/tracing.py:31: if connection:
Generally I try to minimize testing-specific tweaks like this. I've found it's
better to go through the slight pain of setting up an appropriate mock for a
unittest, then to have production code silently fail because one has forgotten
to initialize something.

But in this case it doesn't matter much yet, so I don't really care.

[Benoit L, 2016-01-19 15:54:50]

thanks!

https://codereview.chromium.org/1606903002/diff/40001/tools/android/loading/l...
File tools/android/loading/loading_trace.py (right):

https://codereview.chromium.org/1606903002/diff/40001/tools/android/loading/l...
tools/android/loading/loading_trace.py:31: result = {'url': self.url,
'metadata': self.metadata,
On 2016/01/19 15:24:47, mattcary wrote:
> Turn the keys into class constants to avoid typos?

Done.

https://codereview.chromium.org/1606903002/diff/40001/tools/android/loading/t...
File tools/android/loading/trace_recorder.py (right):

https://codereview.chromium.org/1606903002/diff/40001/tools/android/loading/t...
tools/android/loading/trace_recorder.py:63: 
On 2016/01/19 15:24:47, mattcary wrote:
> two blank lines?

Done.

https://codereview.chromium.org/1606903002/diff/40001/tools/android/loading/t...
File tools/android/loading/tracing.py (right):

https://codereview.chromium.org/1606903002/diff/40001/tools/android/loading/t...
tools/android/loading/tracing.py:31: if connection:
On 2016/01/19 15:24:47, mattcary wrote:
> Generally I try to minimize testing-specific tweaks like this. I've found it's
> better to go through the slight pain of setting up an appropriate mock for a
> unittest, then to have production code silently fail because one has forgotten
> to initialize something.
> 
> But in this case it doesn't matter much yet, so I don't really care.

I agree. But in this instance this is not for testing only. Since the tracks do
processing themselves, then we need to reconstruct them from a JSON file. In
this case, there is no valid connection. So a track need to be useful without
it. However this could be more elegant.
Thoughts?

[Benoit L, 2016-01-19 15:54:56]

The CQ bit was checked by lizeb@chromium.org

[Benoit L, 2016-01-19 15:54:56]

The patchset sent to the CQ was uploaded after l-g-t-m from
mattcary@chromium.org
Link to the patchset: https://codereview.chromium.org/1606903002/#ps60001
(title: "Address comments.")

[commit-bot: I haz the power, 2016-01-19 15:55:32]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1606903002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1606903002/60001

[commit-bot: I haz the power, 2016-01-19 16:13:28]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-01-19 16:14:23]

Description was changed from

==========
tools/android/loading: Archive tracks in LoadingTrace.
==========

to

==========
tools/android/loading: Archive tracks in LoadingTrace.

Committed: https://crrev.com/6cf3e7e1f3cb4bd595f3ddabcc126ead2e10ddf7
Cr-Commit-Position: refs/heads/master@{#370121}
==========

[commit-bot: I haz the power, 2016-01-19 16:14:23]

Patchset 4 (id:??) landed as
https://crrev.com/6cf3e7e1f3cb4bd595f3ddabcc126ead2e10ddf7
Cr-Commit-Position: refs/heads/master@{#370121}