Upgrade analyze.py and related scripts to new world order.

Upgrade analyze.py and related scripts to new world order.

Changes analyze.py to use loading_trace methods. Moved old files to
deprecated/, I think we're all done with them but we may need them for
reference.

Updated loading_model graph drawing to account for new content-types;
what I've done seems sub-optimal & suggestions are welcome.

Fixed at least one minor bug (the request end units problem Benoit
found).

Committed: https://crrev.com/76303eb216a17c2f93f19593f8ba8b81472c29e4
Cr-Commit-Position: refs/heads/master@{#370715}

[mattcary, 2016-01-21 13:58:16]

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

[mattcary, 2016-01-21 13:58:16]

Hi Benoit and Colin:

This update here is what was used to generate the recent verge graph. As
mentioned, the node coloring can use tweaking. I think with this CL we can
declare that we've brought everything up to the new world order.

[blundell, 2016-01-21 14:11:38]

https://codereview.chromium.org/1619713002/diff/1/tools/android/loading/analy...
File tools/android/loading/analyze.py (right):

https://codereview.chromium.org/1619713002/diff/1/tools/android/loading/analy...
tools/android/loading/analyze.py:154: return loading_model.ResourceGraph(
these two lines are:

  return _ProcessJson(json.load(f))

right?

https://codereview.chromium.org/1619713002/diff/1/tools/android/loading/loadi...
File tools/android/loading/loading_model.py (right):

https://codereview.chromium.org/1619713002/diff/1/tools/android/loading/loadi...
tools/android/loading/loading_model.py:293: _CONTENT_KIND_TO_COLOR = {
Worth documenting what these mean?

https://codereview.chromium.org/1619713002/diff/1/tools/android/loading/loadi...
tools/android/loading/loading_model.py:575: if kind in
self._CONTENT_KIND_TO_COLOR:
hmm, also probably worth documenting the relationship between content_kind and
content_type given how this function works.

https://codereview.chromium.org/1619713002/diff/1/tools/android/loading/proce...
File tools/android/loading/processing.py (right):

https://codereview.chromium.org/1619713002/diff/1/tools/android/loading/proce...
tools/android/loading/processing.py:74: return
loading_model.ResourceGraph(loading_trace.LoadingTrace.FromJsonDict(
given the number of times that you're doing this, should we have a constructor
for LoadingTrace and/or ResourceGraph that takes in the filename and does the
work internally?

https://codereview.chromium.org/1619713002/diff/1/tools/android/loading/trace...
File tools/android/loading/trace_recorder.py (right):

https://codereview.chromium.org/1619713002/diff/1/tools/android/loading/trace...
tools/android/loading/trace_recorder.py:45: tracing_track =
tracing.TracingTrack(connection)
nit: either call this |trace| (which you can do now ;), or call the others
page_track and request_track

[Benoit L, 2016-01-21 14:14:38]

https://codereview.chromium.org/1619713002/diff/1/tools/android/loading/analy...
File tools/android/loading/analyze.py (right):

https://codereview.chromium.org/1619713002/diff/1/tools/android/loading/analy...
tools/android/loading/analyze.py:95: def _LogRequests(url, clear_cache=True,
local=False):
Can this move into trace_recorder?

https://codereview.chromium.org/1619713002/diff/1/tools/android/loading/devic...
File tools/android/loading/device_setup.py (right):

https://codereview.chromium.org/1619713002/diff/1/tools/android/loading/devic...
tools/android/loading/device_setup.py:107: 
nit: no blank line here.

https://codereview.chromium.org/1619713002/diff/1/tools/android/loading/devto...
File tools/android/loading/devtools_monitor.py (right):

https://codereview.chromium.org/1619713002/diff/1/tools/android/loading/devto...
tools/android/loading/devtools_monitor.py:172: """Clears device cache.
nit: s/device/browser/

https://codereview.chromium.org/1619713002/diff/1/tools/android/loading/loadi...
File tools/android/loading/loading_model.py (right):

https://codereview.chromium.org/1619713002/diff/1/tools/android/loading/loadi...
tools/android/loading/loading_model.py:296: 'image':           'orange',    #
This probably catches gifs?
it does.

https://codereview.chromium.org/1619713002/diff/1/tools/android/loading/trace...
File tools/android/loading/trace_recorder.py (right):

https://codereview.chromium.org/1619713002/diff/1/tools/android/loading/trace...
tools/android/loading/trace_recorder.py:47:
connection.SendAndIgnoreResponse('Network.clearBrowserCache', {})
Use the ClearCache() function introduced in devtools_monitor.

[mattcary, 2016-01-21 16:11:35]

https://codereview.chromium.org/1619713002/diff/1/tools/android/loading/analy...
File tools/android/loading/analyze.py (right):

https://codereview.chromium.org/1619713002/diff/1/tools/android/loading/analy...
tools/android/loading/analyze.py:95: def _LogRequests(url, clear_cache=True,
local=False):
On 2016/01/21 14:14:38, Benoit L wrote:
> Can this move into trace_recorder?

I think I already essentially did that by breaking out MonitorUrl. I cleaned up
the cache clearing (which I noticed was being done twice) but I think that's
about as simple as we want to make it given different contexts of outputting the
trace and setting up input.

https://codereview.chromium.org/1619713002/diff/1/tools/android/loading/analy...
tools/android/loading/analyze.py:154: return loading_model.ResourceGraph(
On 2016/01/21 14:11:38, blundell wrote:
> these two lines are:
> 
>   return _ProcessJson(json.load(f))
> 
> right?

All simplified by change to allow the ResourceGraph to take a dict, below.

https://codereview.chromium.org/1619713002/diff/1/tools/android/loading/devic...
File tools/android/loading/device_setup.py (right):

https://codereview.chromium.org/1619713002/diff/1/tools/android/loading/devic...
tools/android/loading/device_setup.py:107: 
On 2016/01/21 14:14:38, Benoit L wrote:
> nit: no blank line here.

Done.

https://codereview.chromium.org/1619713002/diff/1/tools/android/loading/devto...
File tools/android/loading/devtools_monitor.py (right):

https://codereview.chromium.org/1619713002/diff/1/tools/android/loading/devto...
tools/android/loading/devtools_monitor.py:172: """Clears device cache.
On 2016/01/21 14:14:38, Benoit L wrote:
> nit: s/device/browser/

Done.

https://codereview.chromium.org/1619713002/diff/1/tools/android/loading/loadi...
File tools/android/loading/loading_model.py (right):

https://codereview.chromium.org/1619713002/diff/1/tools/android/loading/loadi...
tools/android/loading/loading_model.py:293: _CONTENT_KIND_TO_COLOR = {
On 2016/01/21 14:11:38, blundell wrote:
> Worth documenting what these mean?

Done.

https://codereview.chromium.org/1619713002/diff/1/tools/android/loading/loadi...
tools/android/loading/loading_model.py:296: 'image':           'orange',    #
This probably catches gifs?
On 2016/01/21 14:14:38, Benoit L wrote:
> it does.

Then this isn't what we want. Well, let me land this so Colin can start using
it. I'm not sure what the priority of tweaking the graph node colors is anyway.

https://codereview.chromium.org/1619713002/diff/1/tools/android/loading/loadi...
tools/android/loading/loading_model.py:575: if kind in
self._CONTENT_KIND_TO_COLOR:
On 2016/01/21 14:11:38, blundell wrote:
> hmm, also probably worth documenting the relationship between content_kind and
> content_type given how this function works.

I don't really know how these mime-types work. I think content_kind is a general
classification (image, font, application) and type is something very specific,
but I'm not really sure.

https://codereview.chromium.org/1619713002/diff/1/tools/android/loading/proce...
File tools/android/loading/processing.py (right):

https://codereview.chromium.org/1619713002/diff/1/tools/android/loading/proce...
tools/android/loading/processing.py:74: return
loading_model.ResourceGraph(loading_trace.LoadingTrace.FromJsonDict(
On 2016/01/21 14:11:38, blundell wrote:
> given the number of times that you're doing this, should we have a constructor
> for LoadingTrace and/or ResourceGraph that takes in the filename and does the
> work internally?

I am going to have ResourceGraph take a json dict in the constructor. I like
keeping file opening as high up as possible as it makes testing & exception
handling a lot easier.

https://codereview.chromium.org/1619713002/diff/1/tools/android/loading/trace...
File tools/android/loading/trace_recorder.py (right):

https://codereview.chromium.org/1619713002/diff/1/tools/android/loading/trace...
tools/android/loading/trace_recorder.py:45: tracing_track =
tracing.TracingTrack(connection)
On 2016/01/21 14:11:38, blundell wrote:
> nit: either call this |trace| (which you can do now ;), or call the others
> page_track and request_track

Done.

https://codereview.chromium.org/1619713002/diff/1/tools/android/loading/trace...
tools/android/loading/trace_recorder.py:47:
connection.SendAndIgnoreResponse('Network.clearBrowserCache', {})
On 2016/01/21 14:14:38, Benoit L wrote:
> Use the ClearCache() function introduced in devtools_monitor.

Done.

[Benoit L, 2016-01-21 16:20:03]

lgtm, thank you!

https://codereview.chromium.org/1619713002/diff/1/tools/android/loading/analy...
File tools/android/loading/analyze.py (right):

https://codereview.chromium.org/1619713002/diff/1/tools/android/loading/analy...
tools/android/loading/analyze.py:95: def _LogRequests(url, clear_cache=True,
local=False):
On 2016/01/21 16:11:35, mattcary wrote:
> On 2016/01/21 14:14:38, Benoit L wrote:
> > Can this move into trace_recorder?
> 
> I think I already essentially did that by breaking out MonitorUrl. I cleaned
up
> the cache clearing (which I noticed was being done twice) but I think that's
> about as simple as we want to make it given different contexts of outputting
the
> trace and setting up input.

Acknowledged.

https://codereview.chromium.org/1619713002/diff/1/tools/android/loading/loadi...
File tools/android/loading/loading_model.py (right):

https://codereview.chromium.org/1619713002/diff/1/tools/android/loading/loadi...
tools/android/loading/loading_model.py:296: 'image':           'orange',    #
This probably catches gifs?
On 2016/01/21 16:11:35, mattcary wrote:
> On 2016/01/21 14:14:38, Benoit L wrote:
> > it does.
> 
> Then this isn't what we want. Well, let me land this so Colin can start using
> it. I'm not sure what the priority of tweaking the graph node colors is
anyway.

About as high as deciding on the bike shed color.

[mattcary, 2016-01-21 16:21:11]

The CQ bit was checked by mattcary@chromium.org

[commit-bot: I haz the power, 2016-01-21 16:21:46]

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

[commit-bot: I haz the power, 2016-01-21 16:55:27]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-01-21 16:56:22]

Description was changed from

==========
Upgrade analyze.py and related scripts to new world order.

Changes analyze.py to use loading_trace methods. Moved old files to
deprecated/, I think we're all done with them but we may need them for
reference.

Updated loading_model graph drawing to account for new content-types;
what I've done seems sub-optimal & suggestions are welcome.

Fixed at least one minor bug (the request end units problem Benoit
found).
==========

to

==========
Upgrade analyze.py and related scripts to new world order.

Changes analyze.py to use loading_trace methods. Moved old files to
deprecated/, I think we're all done with them but we may need them for
reference.

Updated loading_model graph drawing to account for new content-types;
what I've done seems sub-optimal & suggestions are welcome.

Fixed at least one minor bug (the request end units problem Benoit
found).

Committed: https://crrev.com/76303eb216a17c2f93f19593f8ba8b81472c29e4
Cr-Commit-Position: refs/heads/master@{#370715}
==========

[commit-bot: I haz the power, 2016-01-21 16:56:23]

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