pass script fetch start / end time to Transalte Element to implement real time latency monitoring

pass script fetch start / end time to Transalte Element to implement real time latency monitoring.
See https://go/chrome_translate_e2e_latency for detail.

BUG=638794
Committed: https://crrev.com/22e782e1834f4bbd7e12835210488c97f264951a
Cr-Commit-Position: refs/heads/master@{#413265}

[ftang, 2016-08-18 00:55:06]

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

[commit-bot: I haz the power, 2016-08-18 00:55:34]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[ftang, 2016-08-18 00:55:43]

Description was changed from

==========
pass script fetch start / end time to Transalte Element to implement real time
latency monitoring

BUG=638794
==========

to

==========
pass script fetch start / end time to Transalte Element to implement real time
latency monitoring.
See https://go/chrome_translate_e2e_latency for detail.

BUG=638794
==========

[commit-bot: I haz the power, 2016-08-18 02:03:49]

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

[commit-bot: I haz the power, 2016-08-18 02:03:50]

Dry run: This issue passed the CQ dry run.

[ftang, 2016-08-18 02:16:09]

ftang@chromium.org changed reviewers:
+ groby@chromium.org

[groby-ooo-7-16, 2016-08-18 22:35:04]

https://codereview.chromium.org/2251273002/diff/1/components/translate/core/b...
File components/translate/core/browser/resources/translate.js (right):

https://codereview.chromium.org/2251273002/diff/1/components/translate/core/b...
components/translate/core/browser/resources/translate.js:262: 'timeInfo':
gtTimeInfo,
key is 'timeInfo' here, 'gtTimeInfo' in test - maybe a typo?

https://codereview.chromium.org/2251273002/diff/1/components/translate/core/b...
File components/translate/core/browser/translate_script.cc (right):

https://codereview.chromium.org/2251273002/diff/1/components/translate/core/b...
components/translate/core/browser/translate_script.cc:148: &data_, "var
gtTimeInfo = {'fetchStart': %f, 'fetchEnd': %f};\n",
Question - wouldn't it make more sense to report just the delta? Or do we
actually need times?

[ftang, 2016-08-18 23:01:39]

For the detail of your question. see the design doc. Thank. 
go/chrome_translate_e2e_latency

https://codereview.chromium.org/2251273002/diff/1/components/translate/core/b...
File components/translate/core/browser/resources/translate.js (right):

https://codereview.chromium.org/2251273002/diff/1/components/translate/core/b...
components/translate/core/browser/resources/translate.js:262: 'timeInfo':
gtTimeInfo,
On 2016/08/18 22:35:04, groby wrote:
> key is 'timeInfo' here, 'gtTimeInfo' in test - maybe a typo?

the JavaScript variable in the test is gtTimeInfo. The value appended by the
translate_script.cc is also gtTimeInfo. This JS code pass that gtTimeInfo into
the  google.translate.TranslateService() inside an object keyed 'timeInfo'. The
'timeInfo' will be later read by js code in TranslateElement (see
https://critique.corp.google.com/#review/130658595/depot/google3/googledata/t...
)

https://codereview.chromium.org/2251273002/diff/1/components/translate/core/b...
File components/translate/core/browser/translate_script.cc (right):

https://codereview.chromium.org/2251273002/diff/1/components/translate/core/b...
components/translate/core/browser/translate_script.cc:148: &data_, "var
gtTimeInfo = {'fetchStart': %f, 'fetchEnd': %f};\n",
On 2016/08/18 22:35:04, groby wrote:
> Question - wouldn't it make more sense to report just the delta? Or do we
> actually need times?

I need time (at least one of them) so my js in the TranslateElement can
calculate the end-to-end latency by misusing the fetchStart. We also like to
know the delta between fetchStart and fetchEnd to see the fetch latency and also
factor out the effect by the browser cache. So... yes, we need time.

[groby-ooo-7-16, 2016-08-19 18:20:59]

lgtm

https://codereview.chromium.org/2251273002/diff/1/components/translate/core/b...
File components/translate/core/browser/resources/translate.js (right):

https://codereview.chromium.org/2251273002/diff/1/components/translate/core/b...
components/translate/core/browser/resources/translate.js:262: 'timeInfo':
gtTimeInfo,
On 2016/08/18 23:01:39, ftang wrote:
> On 2016/08/18 22:35:04, groby wrote:
> > key is 'timeInfo' here, 'gtTimeInfo' in test - maybe a typo?
> 
> the JavaScript variable in the test is gtTimeInfo. The value appended by the
> translate_script.cc is also gtTimeInfo. This JS code pass that gtTimeInfo into
> the  google.translate.TranslateService() inside an object keyed 'timeInfo'.
The
> 'timeInfo' will be later read by js code in TranslateElement (see
>
https://critique.corp.google.com/#review/130658595/depot/google3/googledata/t...
> ) 

Ah, I see - thank you for the explanation.

[ftang, 2016-08-19 20:33:23]

https://codereview.chromium.org/2251273002/diff/1/components/translate/core/b...
File components/translate/core/browser/resources/translate.js (right):

https://codereview.chromium.org/2251273002/diff/1/components/translate/core/b...
components/translate/core/browser/resources/translate.js:262: 'timeInfo':
gtTimeInfo,
On 2016/08/19 18:20:59, groby wrote:
> On 2016/08/18 23:01:39, ftang wrote:
> > On 2016/08/18 22:35:04, groby wrote:
> > > key is 'timeInfo' here, 'gtTimeInfo' in test - maybe a typo?
> > 
> > the JavaScript variable in the test is gtTimeInfo. The value appended by the
> > translate_script.cc is also gtTimeInfo. This JS code pass that gtTimeInfo
into
> > the  google.translate.TranslateService() inside an object keyed 'timeInfo'.
> The
> > 'timeInfo' will be later read by js code in TranslateElement (see
> >
>
https://critique.corp.google.com/#review/130658595/depot/google3/googledata/t...
> > ) 
> 
> Ah, I see - thank you for the explanation.

Acknowledged.

[ftang, 2016-08-19 20:33:29]

The CQ bit was checked by ftang@chromium.org

[commit-bot: I haz the power, 2016-08-19 20:33:57]

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, 2016-08-19 22:18:41]

Description was changed from

==========
pass script fetch start / end time to Transalte Element to implement real time
latency monitoring.
See https://go/chrome_translate_e2e_latency for detail.

BUG=638794
==========

to

==========
pass script fetch start / end time to Transalte Element to implement real time
latency monitoring.
See https://go/chrome_translate_e2e_latency for detail.

BUG=638794
==========

[commit-bot: I haz the power, 2016-08-19 22:18:42]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-08-19 22:26:52]

Description was changed from

==========
pass script fetch start / end time to Transalte Element to implement real time
latency monitoring.
See https://go/chrome_translate_e2e_latency for detail.

BUG=638794
==========

to

==========
pass script fetch start / end time to Transalte Element to implement real time
latency monitoring.
See https://go/chrome_translate_e2e_latency for detail.

BUG=638794

Committed: https://crrev.com/22e782e1834f4bbd7e12835210488c97f264951a
Cr-Commit-Position: refs/heads/master@{#413265}
==========

[commit-bot: I haz the power, 2016-08-19 22:26:53]

Patchset 1 (id:??) landed as
https://crrev.com/22e782e1834f4bbd7e12835210488c97f264951a
Cr-Commit-Position: refs/heads/master@{#413265}

[yastrebov.alex, 2016-10-18 09:57:58]

On 2016/08/19 22:26:53, commit-bot: I haz the power wrote:
> Patchset 1 (id:??) landed as
> https://crrev.com/22e782e1834f4bbd7e12835210488c97f264951a
> Cr-Commit-Position: refs/heads/master@{#413265}

This created an regression:

https://bugs.chromium.org/p/chromium/issues/detail?id=656922