Log NTP launch time until user can do a search.

Log NTP launch time until user can do a search.

BUG=672120
Committed: https://crrev.com/74766b3f02e5cfa36808e2d77429873d589df3f8
Cr-Commit-Position: refs/heads/master@{#437006}

[Michael van Ouwerkerk, 2016-12-06 17:03:18]

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

[Michael van Ouwerkerk, 2016-12-06 17:03:42]

mvanouwerkerk@chromium.org changed reviewers:
+ bauerb@chromium.org

[Michael van Ouwerkerk, 2016-12-06 17:03:42]

Bernhard, could you take a look please?

[commit-bot: I haz the power, 2016-12-06 17:03:53]

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, 2016-12-06 17:07:18]

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

[commit-bot: I haz the power, 2016-12-06 17:07:19]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Bernhard Bauer, 2016-12-06 17:20:43]

Neat! LGTM w/ nits:

https://codereview.chromium.org/2554003003/diff/1/chrome/android/java/src/org...
File chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageUma.java
(right):

https://codereview.chromium.org/2554003003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageUma.java:95:
// The ntp was loaded in a cold startup.
Nit: Capitalize NTP?

https://codereview.chromium.org/2554003003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageUma.java:205:
} else {
I think I would return after each of these.

[Michael van Ouwerkerk, 2016-12-06 17:33:09]

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

[commit-bot: I haz the power, 2016-12-06 17:33:34]

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

[Michael van Ouwerkerk, 2016-12-06 17:47:14]

https://codereview.chromium.org/2554003003/diff/1/chrome/android/java/src/org...
File chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageUma.java
(right):

https://codereview.chromium.org/2554003003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageUma.java:95:
// The ntp was loaded in a cold startup.
On 2016/12/06 17:20:43, Bernhard Bauer wrote:
> Nit: Capitalize NTP?

Done.

https://codereview.chromium.org/2554003003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageUma.java:205:
} else {
On 2016/12/06 17:20:43, Bernhard Bauer wrote:
> I think I would return after each of these.

Done.

[Michael van Ouwerkerk, 2016-12-06 17:48:39]

mvanouwerkerk@chromium.org changed reviewers:
+ asvitkine@chromium.org

[Michael van Ouwerkerk, 2016-12-06 17:48:39]

Alexei, could you take a look for histograms.xml please?

[Michael van Ouwerkerk, 2016-12-06 17:48:45]

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

[commit-bot: I haz the power, 2016-12-06 17:49:33]

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

[Alexei Svitkine (slow), 2016-12-06 18:00:54]

https://codereview.chromium.org/2554003003/diff/40001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2554003003/diff/40001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:38356: +    A histogram for the type of
load for the NTP, such as cold or warm start.
If a histogram is Java only, please mention so in the description.

Also, could you define what warm and cold mean in these cases?

[commit-bot: I haz the power, 2016-12-06 18:42:24]

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

[commit-bot: I haz the power, 2016-12-06 18:42:25]

Dry run: This issue passed the CQ dry run.

[Michael van Ouwerkerk, 2016-12-07 11:08:21]

All done, please take another look :-)

https://codereview.chromium.org/2554003003/diff/40001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2554003003/diff/40001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:38356: +    A histogram for the type of
load for the NTP, such as cold or warm start.
On 2016/12/06 18:00:54, Alexei Svitkine wrote:
> If a histogram is Java only, please mention so in the description.
> 
> Also, could you define what warm and cold mean in these cases?

Done.

[Michael van Ouwerkerk, 2016-12-07 11:08:25]

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

[commit-bot: I haz the power, 2016-12-07 11:08:49]

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, 2016-12-07 11:57:56]

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

[commit-bot: I haz the power, 2016-12-07 11:57:57]

Dry run: This issue passed the CQ dry run.

[Alexei Svitkine (slow), 2016-12-07 16:36:05]

lgtm, but please file a crbug for this change since it's non-trivial and
associate the CL with it via BUG=

https://codereview.chromium.org/2554003003/diff/60001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2554003003/diff/60001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:38357: +    warm start if the native
library is already loaded and initialized at the
Nit: Suggest copying this extra description to the other histograms you're
adding. Or alternatively reference this histogram from their descriptions for
details.

[Michael van Ouwerkerk, 2016-12-07 17:14:14]

Description was changed from

==========
Log NTP launch time until user can do a search.
==========

to

==========
Log NTP launch time until user can do a search.

BUG=672120
==========

[Michael van Ouwerkerk, 2016-12-07 17:15:51]

Thanks!

https://codereview.chromium.org/2554003003/diff/60001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2554003003/diff/60001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:38357: +    warm start if the native
library is already loaded and initialized at the
On 2016/12/07 16:36:05, Alexei Svitkine wrote:
> Nit: Suggest copying this extra description to the other histograms you're
> adding. Or alternatively reference this histogram from their descriptions for
> details.

Done.

[Michael van Ouwerkerk, 2016-12-07 17:15:55]

The CQ bit was checked by mvanouwerkerk@chromium.org

[Michael van Ouwerkerk, 2016-12-07 17:15:55]

The patchset sent to the CQ was uploaded after l-g-t-m from bauerb@chromium.org,
asvitkine@chromium.org
Link to the patchset: https://codereview.chromium.org/2554003003/#ps80001
(title: "Address review comments from asvitkine.")

[commit-bot: I haz the power, 2016-12-07 17:16:29]

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-12-07 18:09:24]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1481130955011370,
"parent_rev": "f09a8bbd330ca300f94383b729f21b34bdcac382", "commit_rev":
"523cd8fa1a1153908e6a304b09416e5f7b790702"}

[commit-bot: I haz the power, 2016-12-07 18:09:51]

Description was changed from

==========
Log NTP launch time until user can do a search.

BUG=672120
==========

to

==========
Log NTP launch time until user can do a search.

BUG=672120
==========

[commit-bot: I haz the power, 2016-12-07 18:09:53]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-12-07 18:11:54]

Description was changed from

==========
Log NTP launch time until user can do a search.

BUG=672120
==========

to

==========
Log NTP launch time until user can do a search.

BUG=672120

Committed: https://crrev.com/74766b3f02e5cfa36808e2d77429873d589df3f8
Cr-Commit-Position: refs/heads/master@{#437006}
==========

[commit-bot: I haz the power, 2016-12-07 18:11:55]

Patchset 5 (id:??) landed as
https://crrev.com/74766b3f02e5cfa36808e2d77429873d589df3f8
Cr-Commit-Position: refs/heads/master@{#437006}