[Android] Add AppMenu touch duration metric.

[Android] Add AppMenu touch duration metric.

We want to monitor touch duration on AppMenu to decide optimal timeout
for single tap vs dragging.

BUG=434501
Committed: https://crrev.com/52b622f3041de604765f5320b86b5558223e4172
Cr-Commit-Position: refs/heads/master@{#308663}

[Kibeom Kim (inactive), 2014-12-03 19:38:29]

kkimlabs@chromium.org changed reviewers:
+ asvitkine@chromium.org, aurimas@chromium.org, tedchoc@chromium.org

[Alexei Svitkine (slow), 2014-12-08 15:50:52]

https://codereview.chromium.org/763253005/diff/20001/tools/metrics/histograms...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/763253005/diff/20001/tools/metrics/histograms...
tools/metrics/histograms/histograms.xml:421: +<histogram
name="AppMenu.TouchDuration" units="milliseconds">
This looks like it's adding a new top-level prefix, which we generally try to
avoid doing too often.

Is there another prefix that this can use? Or do you expect a lot of metrics
being added under this prefix going forward?

https://codereview.chromium.org/763253005/diff/20001/tools/metrics/histograms...
tools/metrics/histograms/histograms.xml:424: +    Time difference between touch
down and touch up on the menu button.
Mention that it's Android specific.

[Ted C, 2014-12-10 17:28:55]

https://codereview.chromium.org/763253005/diff/20001/chrome/android/java/src/...
File
chrome/android/java/src/org/chromium/chrome/browser/appmenu/AppMenuDragHelper.java
(right):

https://codereview.chromium.org/763253005/diff/20001/chrome/android/java/src/...
chrome/android/java/src/org/chromium/chrome/browser/appmenu/AppMenuDragHelper.java:161:
nativeRecordTouchDuration(timeSinceDown);
this can't be triggered before native is loaded right?  I think we prevent the
menu in all those cases.

https://codereview.chromium.org/763253005/diff/20001/chrome/browser/android/a...
File chrome/browser/android/appmenu/app_menu_drag_helper.cc (right):

https://codereview.chromium.org/763253005/diff/20001/chrome/browser/android/a...
chrome/browser/android/appmenu/app_menu_drag_helper.cc:9: namespace appmenu {
These namespaces are way too specific.  I think in the chrome package, they are
promoting the removal of all namespaces, so I think you can just drop these (and
maybe make the name RecordAppMenuTouchDuration for name collision prevention).

https://codereview.chromium.org/763253005/diff/20001/chrome/browser/android/a...
chrome/browser/android/appmenu/app_menu_drag_helper.cc:16: void
RecordTouchDuration(JNIEnv* env, jclass jcaller, jlong time_ms) {
add "// static" above this.

[Kibeom Kim (inactive), 2014-12-15 22:14:30]

https://codereview.chromium.org/763253005/diff/20001/chrome/android/java/src/...
File
chrome/android/java/src/org/chromium/chrome/browser/appmenu/AppMenuDragHelper.java
(right):

https://codereview.chromium.org/763253005/diff/20001/chrome/android/java/src/...
chrome/android/java/src/org/chromium/chrome/browser/appmenu/AppMenuDragHelper.java:161:
nativeRecordTouchDuration(timeSinceDown);
On 2014/12/10 17:28:55, Ted C wrote:
> this can't be triggered before native is loaded right?  I think we prevent the
> menu in all those cases.

I tried commeting out ChromeBrowserInitializer#handlePostNativeStartup's task
posting, then menu button didn't work. So yeah you're right.

https://codereview.chromium.org/763253005/diff/20001/chrome/browser/android/a...
File chrome/browser/android/appmenu/app_menu_drag_helper.cc (right):

https://codereview.chromium.org/763253005/diff/20001/chrome/browser/android/a...
chrome/browser/android/appmenu/app_menu_drag_helper.cc:9: namespace appmenu {
On 2014/12/10 17:28:55, Ted C wrote:
> These namespaces are way too specific.  I think in the chrome package, they
are
> promoting the removal of all namespaces, so I think you can just drop these
(and
> maybe make the name RecordAppMenuTouchDuration for name collision prevention).

Done.

https://codereview.chromium.org/763253005/diff/20001/chrome/browser/android/a...
chrome/browser/android/appmenu/app_menu_drag_helper.cc:16: void
RecordTouchDuration(JNIEnv* env, jclass jcaller, jlong time_ms) {
On 2014/12/10 17:28:55, Ted C wrote:
> add "// static" above this.

Done.

https://codereview.chromium.org/763253005/diff/20001/tools/metrics/histograms...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/763253005/diff/20001/tools/metrics/histograms...
tools/metrics/histograms/histograms.xml:421: +<histogram
name="AppMenu.TouchDuration" units="milliseconds">
On 2014/12/08 15:50:52, asvitkine (OOO until Jan4) wrote:
> This looks like it's adding a new top-level prefix, which we generally try to
> avoid doing too often.
> 
> Is there another prefix that this can use? Or do you expect a lot of metrics
> being added under this prefix going forward?

Done.

https://codereview.chromium.org/763253005/diff/20001/tools/metrics/histograms...
tools/metrics/histograms/histograms.xml:424: +    Time difference between touch
down and touch up on the menu button.
On 2014/12/08 15:50:52, asvitkine (OOO until Jan4) wrote:
> Mention that it's Android specific.

Done.

[Kibeom Kim (inactive), 2014-12-15 22:18:10]

kkimlabs@chromium.org changed reviewers:
+ jwd@chromium.org

[Kibeom Kim (inactive), 2014-12-15 22:18:10]

jwd@ ptal : tools/metrics/histograms/histograms.xml (since asvitkine is OOO for
a while)

[Ted C, 2014-12-15 22:19:43]

android logic - lgtm

[jwd, 2014-12-16 19:08:04]

lgtm

[Kibeom Kim (inactive), 2014-12-16 19:52:54]

The CQ bit was checked by kkimlabs@chromium.org

[commit-bot: I haz the power, 2014-12-16 19:54:09]

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

[commit-bot: I haz the power, 2014-12-16 20:56:34]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2014-12-16 20:57:10]

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