Update histograms.xml with toolbar first draw histograms

Update histograms.xml with toolbar first draw histograms

4 histograms that have recently been added were not updated in histograms.xml
3/4 of these are a previous histogram being split into 3. We were only recording
this value for tabbed mode and we have started adding histograms for 3 different
modes of operation. These each have distinct UIs so the draw times should cluster
differently across them.

Committed: https://crrev.com/ef631726955a8d481e3703fa375831710f951ba9
Cr-Commit-Position: refs/heads/master@{#336180}

[Yusuf, 2015-06-23 22:52:31]

yusufo@chromium.org changed reviewers:
+ isherman@chromium.org

[Ilya Sherman, 2015-06-23 22:57:42]

LGTM % nits:

https://codereview.chromium.org/1204753003/diff/1/tools/metrics/histograms/hi...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/1204753003/diff/1/tools/metrics/histograms/hi...
tools/metrics/histograms/histograms.xml:17264: +</histogram>
Optional: You might want to use a histogram_suffixes element to reduce
redundancy.

https://codereview.chromium.org/1204753003/diff/1/tools/metrics/histograms/hi...
tools/metrics/histograms/histograms.xml:17281: +    Toolbar in DOCUMENT
mode(Tabs and apps together). This excludes activity
nit: "mode(" -> "mode ("

https://codereview.chromium.org/1204753003/diff/1/tools/metrics/histograms/hi...
tools/metrics/histograms/histograms.xml:50991: +  <int value="0" label="OTHER"/>
Is it worth distinguishing "other, identified" from "other, unidentified"
(assuming the latter is possible)?

https://codereview.chromium.org/1204753003/diff/1/tools/metrics/histograms/hi...
tools/metrics/histograms/histograms.xml:51002: +  <int value="11" label="GSA"/>
Optional nit: I'd suggest using Sentence case, i.e. "Other", "Gmail", etc.

[Yusuf, 2015-06-23 23:14:17]

https://codereview.chromium.org/1204753003/diff/1/tools/metrics/histograms/hi...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/1204753003/diff/1/tools/metrics/histograms/hi...
tools/metrics/histograms/histograms.xml:17264: +</histogram>
On 2015/06/23 22:57:42, Ilya Sherman wrote:
> Optional: You might want to use a histogram_suffixes element to reduce
> redundancy.

Done.

https://codereview.chromium.org/1204753003/diff/1/tools/metrics/histograms/hi...
tools/metrics/histograms/histograms.xml:17281: +    Toolbar in DOCUMENT
mode(Tabs and apps together). This excludes activity
On 2015/06/23 22:57:42, Ilya Sherman wrote:
> nit: "mode(" -> "mode ("

Done.

https://codereview.chromium.org/1204753003/diff/1/tools/metrics/histograms/hi...
tools/metrics/histograms/histograms.xml:50991: +  <int value="0" label="OTHER"/>
On 2015/06/23 22:57:42, Ilya Sherman wrote:
> Is it worth distinguishing "other, identified" from "other, unidentified"
> (assuming the latter is possible)?

Not really since this is mainly declared by the app itself and we don't do much
on our end about guessing. Other here means the app did not specify hence is
unidentified.

https://codereview.chromium.org/1204753003/diff/1/tools/metrics/histograms/hi...
tools/metrics/histograms/histograms.xml:51002: +  <int value="11" label="GSA"/>
On 2015/06/23 22:57:42, Ilya Sherman wrote:
> Optional nit: I'd suggest using Sentence case, i.e. "Other", "Gmail", etc.

Done.

[Yusuf, 2015-06-23 23:15:21]

yusufo@chromium.org changed reviewers:
+ dfalcantara@chromium.org

[Yusuf, 2015-06-23 23:15:21]

Also added a change to the string recorded in the code to conform to the
histogram suffix use.

dfalcantara@ for reviewing ToolbarManager.java change

[gone, 2015-06-23 23:16:14]

lgtm

[Yusuf, 2015-06-23 23:16:35]

The CQ bit was checked by yusufo@chromium.org

[Yusuf, 2015-06-23 23:16:36]

The patchset sent to the CQ was uploaded after l-g-t-m from
isherman@chromium.org
Link to the patchset: https://codereview.chromium.org/1204753003/#ps20001
(title: "Nits addressed and changed histogram name in code to conform to suffix
use")

[commit-bot: I haz the power, 2015-06-23 23:17:40]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1204753003/20001

[Ilya Sherman, 2015-06-23 23:41:03]

The CQ bit was unchecked by isherman@chromium.org

[Ilya Sherman, 2015-06-23 23:42:36]

https://codereview.chromium.org/1204753003/diff/1/tools/metrics/histograms/hi...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/1204753003/diff/1/tools/metrics/histograms/hi...
tools/metrics/histograms/histograms.xml:50991: +  <int value="0" label="OTHER"/>
On 2015/06/23 23:14:17, Yusuf wrote:
> On 2015/06/23 22:57:42, Ilya Sherman wrote:
> > Is it worth distinguishing "other, identified" from "other, unidentified"
> > (assuming the latter is possible)?
> 
> Not really since this is mainly declared by the app itself and we don't do
much
> on our end about guessing. Other here means the app did not specify hence is
> unidentified.

What is recorded if, say, Flipboard, decided to implement this, and identified
itself as "Flipboard"?  Is it the same as what would be recorded if Flipboard
didn't both to identify itself at all?

https://codereview.chromium.org/1204753003/diff/20001/chrome/android/java_sta...
File
chrome/android/java_staging/src/org/chromium/chrome/browser/toolbar/ToolbarManager.java
(right):

https://codereview.chromium.org/1204753003/diff/20001/chrome/android/java_sta...
chrome/android/java_staging/src/org/chromium/chrome/browser/toolbar/ToolbarManager.java:832:
RecordHistogram.recordTimesHistogram("MobileStartup.ToolbarFirstDrawTime_" +
activityName,
nit: You can use a dot with historam_suffixes -- you just specify separator="."

[Yusuf, 2015-06-24 19:42:24]

Will wait for another lgtm from isherman@ to submit. Just to make sure I address
any concerns about Other vs Unidentified.

https://codereview.chromium.org/1204753003/diff/1/tools/metrics/histograms/hi...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/1204753003/diff/1/tools/metrics/histograms/hi...
tools/metrics/histograms/histograms.xml:50991: +  <int value="0" label="OTHER"/>
On 2015/06/23 23:42:35, Ilya Sherman wrote:
> On 2015/06/23 23:14:17, Yusuf wrote:
> > On 2015/06/23 22:57:42, Ilya Sherman wrote:
> > > Is it worth distinguishing "other, identified" from "other, unidentified"
> > > (assuming the latter is possible)?
> > 
> > Not really since this is mainly declared by the app itself and we don't do
> much
> > on our end about guessing. Other here means the app did not specify hence is
> > unidentified.
> 
> What is recorded if, say, Flipboard, decided to implement this, and identified
> itself as "Flipboard"?  Is it the same as what would be recorded if Flipboard
> didn't both to identify itself at all?

Yes, right now it is the same. The spec says they are supposed to include their
package name in this if they choose to do so. And we have to do the bookkeeping
for the apps that include this. If a new one comes along we record as other.

I think changing this to separate the two cases would be useful in identifying
the volume of traffic that is not currently included but can be included with an
update. But not really easy to scale.

https://codereview.chromium.org/1204753003/diff/20001/chrome/android/java_sta...
File
chrome/android/java_staging/src/org/chromium/chrome/browser/toolbar/ToolbarManager.java
(right):

https://codereview.chromium.org/1204753003/diff/20001/chrome/android/java_sta...
chrome/android/java_staging/src/org/chromium/chrome/browser/toolbar/ToolbarManager.java:832:
RecordHistogram.recordTimesHistogram("MobileStartup.ToolbarFirstDrawTime_" +
activityName,
On 2015/06/23 23:42:36, Ilya Sherman wrote:
> nit: You can use a dot with historam_suffixes -- you just specify
separator="."

Thanks! Switched to doing that instead.

[Ilya Sherman, 2015-06-24 23:34:41]

LGTM

[Ilya Sherman, 2015-06-24 23:34:46]

The CQ bit was checked by isherman@chromium.org

[Ilya Sherman, 2015-06-24 23:34:46]

The patchset sent to the CQ was uploaded after l-g-t-m from
dfalcantara@chromium.org
Link to the patchset: https://codereview.chromium.org/1204753003/#ps40001
(title: "Added separator .")

[commit-bot: I haz the power, 2015-06-24 23:37:35]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1204753003/40001

[commit-bot: I haz the power, 2015-06-24 23:46:31]

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

[commit-bot: I haz the power, 2015-06-24 23:46:36]

Exceeded global retry quota

[Yusuf, 2015-06-24 23:47:49]

The CQ bit was checked by yusufo@chromium.org

[commit-bot: I haz the power, 2015-06-24 23:49:36]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1204753003/40001

[commit-bot: I haz the power, 2015-06-24 23:51:34]

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

[commit-bot: I haz the power, 2015-06-24 23:51:38]

Exceeded global retry quota

[Yusuf, 2015-06-25 16:59:47]

The CQ bit was checked by yusufo@chromium.org

[commit-bot: I haz the power, 2015-06-25 17:00:46]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1204753003/40001

[commit-bot: I haz the power, 2015-06-25 17:08:01]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-06-25 17:09:00]

Patchset 3 (id:??) landed as
https://crrev.com/ef631726955a8d481e3703fa375831710f951ba9
Cr-Commit-Position: refs/heads/master@{#336180}