[Media Router] Add UMA metrics to record the load time for route controls

[Media Router] Add UMA metrics to record the load time for route controls

This CL adds a metric to measure how long it takes the WebUI route controls to
load after the route details view was opened. We also send the timestamp for
when the view was opened to the extensionview, so that we can also measure the
load time of the extensionview (done on the extension-side [1]).

[1] cl/157656839

BUG=707326
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2915983002
Cr-Commit-Position: refs/heads/master@{#477343}
Committed: https://chromium.googlesource.com/chromium/src/+/8acf7ed04acf24171f8928a26329a731955470e9

[takumif, 2017-06-01 00:37:47]

Description was changed from

==========
[Media Router] Add UMA metrics to record the load time for route controls

BUG=707326
==========

to

==========
[Media Router] Add UMA metrics to record the load time for route controls

BUG=707326
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[takumif, 2017-06-01 21:18:01]

Description was changed from

==========
[Media Router] Add UMA metrics to record the load time for route controls

BUG=707326
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
[Media Router] Add UMA metrics to record the load time for route controls

This CL adds a metric to measure how long it takes the WebUI route controls to
load after the route details view was opened. We also send the timestamp for
when the view was opened to the extensionview, so that we can also measure the
load time of the extensionview (done on the extension-side [1]).

[1] cl/157656839

BUG=707326
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[takumif, 2017-06-01 21:21:11]

Description was changed from

==========
[Media Router] Add UMA metrics to record the load time for route controls

This CL adds a metric to measure how long it takes the WebUI route controls to
load after the route details view was opened. We also send the timestamp for
when the view was opened to the extensionview, so that we can also measure the
load time of the extensionview (done on the extension-side [1]).

[1] cl/157656839

BUG=707326
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
[Media Router] Add UMA metrics to record the load time for route controls

This CL adds a metric to measure how long it takes the WebUI route controls to
load after the route details view was opened. We also send the timestamp for
when the view was opened to the extensionview, so that we can also measure the
load time of the extensionview (done on the extension-side [1]).

[1] cl/157656839

BUG=707326
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[takumif, 2017-06-01 21:49:03]

takumif@chromium.org changed reviewers:
+ asvitkine@chromium.org, imcheng@chromium.org

[takumif, 2017-06-01 21:49:04]

Please take a look at:
imcheng@: all
asvitkine@: histograms.xml
Thanks!

[imcheng, 2017-06-02 23:53:13]

https://codereview.chromium.org/2915983002/diff/20001/chrome/browser/resource...
File
chrome/browser/resources/media_router/elements/media_router_container/media_router_container.js
(right):

https://codereview.chromium.org/2915983002/diff/20001/chrome/browser/resource...
chrome/browser/resources/media_router/elements/media_router_container/media_router_container.js:2323:
this.routeDetailsOpenTime_ = new Date();
Use Date.now() to get back a number instead of a Date object.

Also, it would be a bit simpler if this could go into the constructor for
route-details.

https://codereview.chromium.org/2915983002/diff/20001/chrome/browser/resource...
File
chrome/browser/resources/media_router/elements/route_controls/route_controls.js
(right):

https://codereview.chromium.org/2915983002/diff/20001/chrome/browser/resource...
chrome/browser/resources/media_router/elements/route_controls/route_controls.js:78:
observer: 'onRouteDetailsOpenTimeSet_',
We can use maybeReportLoadTime_ here. Then we won't need
onRouteDetailsOpenTimeSet_. But then if we set openTime on route details during
construction, then I don't think we need the observer here.

https://codereview.chromium.org/2915983002/diff/20001/chrome/browser/resource...
chrome/browser/resources/media_router/elements/route_controls/route_controls.js:242:
this.initialLoadTime_ = new Date();
Date.now()

https://codereview.chromium.org/2915983002/diff/20001/chrome/browser/resource...
File
chrome/browser/resources/media_router/elements/route_details/extension_view_wrapper/extension_view_wrapper.js
(right):

https://codereview.chromium.org/2915983002/diff/20001/chrome/browser/resource...
chrome/browser/resources/media_router/elements/route_details/extension_view_wrapper/extension_view_wrapper.js:57:
return controllerPath = this.route.customControllerPath +
The "controllerPath = " seems redundant - remove?

https://codereview.chromium.org/2915983002/diff/20001/chrome/browser/resource...
chrome/browser/resources/media_router/elements/route_details/extension_view_wrapper/extension_view_wrapper.js:66:
/** @const */ var extensionview = this.$['custom-controller'];
Does it work if you replaced "var" with "const"?

[takumif, 2017-06-03 01:22:11]

Patchset #3 (id:40001) has been deleted

[takumif, 2017-06-03 01:29:23]

Patchset #3 (id:60001) has been deleted

[takumif, 2017-06-03 01:31:15]

Thanks for reviewing!

https://codereview.chromium.org/2915983002/diff/20001/chrome/browser/resource...
File
chrome/browser/resources/media_router/elements/media_router_container/media_router_container.js
(right):

https://codereview.chromium.org/2915983002/diff/20001/chrome/browser/resource...
chrome/browser/resources/media_router/elements/media_router_container/media_router_container.js:2323:
this.routeDetailsOpenTime_ = new Date();
On 2017/06/02 23:53:13, imcheng wrote:
> Use Date.now() to get back a number instead of a Date object.
> 
> Also, it would be a bit simpler if this could go into the constructor for
> route-details. 

Done.

https://codereview.chromium.org/2915983002/diff/20001/chrome/browser/resource...
File
chrome/browser/resources/media_router/elements/route_controls/route_controls.js
(right):

https://codereview.chromium.org/2915983002/diff/20001/chrome/browser/resource...
chrome/browser/resources/media_router/elements/route_controls/route_controls.js:78:
observer: 'onRouteDetailsOpenTimeSet_',
On 2017/06/02 23:53:13, imcheng wrote:
> We can use maybeReportLoadTime_ here. Then we won't need
> onRouteDetailsOpenTimeSet_. But then if we set openTime on route details
during
> construction, then I don't think we need the observer here.

Right, the observer is no longer necessary. Removed.

https://codereview.chromium.org/2915983002/diff/20001/chrome/browser/resource...
chrome/browser/resources/media_router/elements/route_controls/route_controls.js:242:
this.initialLoadTime_ = new Date();
On 2017/06/02 23:53:13, imcheng wrote:
> Date.now()

Done.

https://codereview.chromium.org/2915983002/diff/20001/chrome/browser/resource...
File
chrome/browser/resources/media_router/elements/route_details/extension_view_wrapper/extension_view_wrapper.js
(right):

https://codereview.chromium.org/2915983002/diff/20001/chrome/browser/resource...
chrome/browser/resources/media_router/elements/route_details/extension_view_wrapper/extension_view_wrapper.js:57:
return controllerPath = this.route.customControllerPath +
On 2017/06/02 23:53:13, imcheng wrote:
> The "controllerPath = " seems redundant - remove?

Done.

https://codereview.chromium.org/2915983002/diff/20001/chrome/browser/resource...
chrome/browser/resources/media_router/elements/route_details/extension_view_wrapper/extension_view_wrapper.js:66:
/** @const */ var extensionview = this.$['custom-controller'];
On 2017/06/02 23:53:13, imcheng wrote:
> Does it work if you replaced "var" with "const"?

The Closure compiler complains if you do that :(

[Alexei Svitkine (slow), 2017-06-05 14:35:21]

metrics lgtm

[takumif, 2017-06-05 17:22:18]

The CQ bit was checked by takumif@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-05 17:22:40]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[imcheng, 2017-06-05 17:44:26]

lgtm

https://codereview.chromium.org/2915983002/diff/20001/chrome/browser/resource...
File
chrome/browser/resources/media_router/elements/route_details/extension_view_wrapper/extension_view_wrapper.js
(right):

https://codereview.chromium.org/2915983002/diff/20001/chrome/browser/resource...
chrome/browser/resources/media_router/elements/route_details/extension_view_wrapper/extension_view_wrapper.js:66:
/** @const */ var extensionview = this.$['custom-controller'];
On 2017/06/03 01:31:14, takumif wrote:
> On 2017/06/02 23:53:13, imcheng wrote:
> > Does it work if you replaced "var" with "const"?
> 
> The Closure compiler complains if you do that :(

What's the error message?

[takumif, 2017-06-05 17:50:16]

https://codereview.chromium.org/2915983002/diff/20001/chrome/browser/resource...
File
chrome/browser/resources/media_router/elements/route_details/extension_view_wrapper/extension_view_wrapper.js
(right):

https://codereview.chromium.org/2915983002/diff/20001/chrome/browser/resource...
chrome/browser/resources/media_router/elements/route_details/extension_view_wrapper/extension_view_wrapper.js:66:
/** @const */ var extensionview = this.$['custom-controller'];
On 2017/06/05 17:44:26, imcheng wrote:
> On 2017/06/03 01:31:14, takumif wrote:
> > On 2017/06/02 23:53:13, imcheng wrote:
> > > Does it work if you replaced "var" with "const"?
> > 
> > The Closure compiler complains if you do that :(
> 
> What's the error message?

It simply says:
Found JavaScript style violations in
chrome/browser/resources/media_router/elements/route_details/extension_view_wrapper/extension_view_wrapper.js:
  line 61: Use /** @const */ var varName; instead of const varName;=

[commit-bot: I haz the power, 2017-06-05 18:12:53]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-05 18:12:54]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[takumif, 2017-06-05 19:45:22]

The CQ bit was checked by takumif@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-05 19:45:52]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-05 21:59:33]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-05 21:59:34]

Dry run: This issue passed the CQ dry run.

[takumif, 2017-06-06 18:10:49]

The CQ bit was checked by takumif@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-06 18:11:32]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-06 18:16:31]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-06 18:16:32]

Dry run: This issue passed the CQ dry run.

[takumif, 2017-06-06 18:17:42]

The CQ bit was checked by takumif@chromium.org

[takumif, 2017-06-06 18:17:42]

The patchset sent to the CQ was uploaded after l-g-t-m from
imcheng@chromium.org, asvitkine@chromium.org
Link to the patchset: https://codereview.chromium.org/2915983002/#ps100001
(title: "Fix a test failure")

[commit-bot: I haz the power, 2017-06-06 18:18:15]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-06 18:24:05]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1496773062067980,
"parent_rev": "67095448eb212b9f8e09f071515669d9618e7534", "commit_rev":
"8acf7ed04acf24171f8928a26329a731955470e9"}

[commit-bot: I haz the power, 2017-06-06 18:24:14]

Description was changed from

==========
[Media Router] Add UMA metrics to record the load time for route controls

This CL adds a metric to measure how long it takes the WebUI route controls to
load after the route details view was opened. We also send the timestamp for
when the view was opened to the extensionview, so that we can also measure the
load time of the extensionview (done on the extension-side [1]).

[1] cl/157656839

BUG=707326
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
[Media Router] Add UMA metrics to record the load time for route controls

This CL adds a metric to measure how long it takes the WebUI route controls to
load after the route details view was opened. We also send the timestamp for
when the view was opened to the extensionview, so that we can also measure the
load time of the extensionview (done on the extension-side [1]).

[1] cl/157656839

BUG=707326
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2915983002
Cr-Commit-Position: refs/heads/master@{#477343}
Committed:
https://chromium.googlesource.com/chromium/src/+/8acf7ed04acf24171f8928a26329...
==========

[commit-bot: I haz the power, 2017-06-06 18:24:15]

Committed patchset #4 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/8acf7ed04acf24171f8928a26329...