Implementation of newly designed sign in related histograms for Android

Implementation of newly designed sign in related histograms for Android.

This CL is dedicated to implement newly designed sign in related histograms (Signin.SigninStartedAccessPoint, Signin.SigninCompletedAccessPoint, Signin.SigninReason) for Android. Please refer https://docs.google.com/a/google.com/document/d/1-gXYAMXXgsJhk6jxO55RuYJ00JBGermevJZ0sIlk6ko/edit?usp=sharing for details.

BUG=532557
Committed: https://crrev.com/5a9e2b34d01056dfdc1a60ce418f5e9f884c9967
Cr-Commit-Position: refs/heads/master@{#373577}

[gogerald1, 2016-01-08 22:43:35]

Patchset #1 (id:1) has been deleted

[gogerald1, 2016-01-08 22:48:41]

Description was changed from

==========
head order


histograms

BUG=
==========

to

==========
Newly designed sign in related histograms for Android

This CL is dedicated to implement newly designed sign in related histograms
(Signin.SigninStartedAccessPoint, Signin.SigninCompletedAccessPoint,
Signin.SigninReason) for Android. Please refer for
https://docs.google.com/a/google.com/document/d/1-gXYAMXXgsJhk6jxO55RuYJ00JBG...
details.

BUG=532557
==========

[gogerald1, 2016-01-08 22:49:35]

Description was changed from

==========
Newly designed sign in related histograms for Android

This CL is dedicated to implement newly designed sign in related histograms
(Signin.SigninStartedAccessPoint, Signin.SigninCompletedAccessPoint,
Signin.SigninReason) for Android. Please refer for
https://docs.google.com/a/google.com/document/d/1-gXYAMXXgsJhk6jxO55RuYJ00JBG...
details.

BUG=532557
==========

to

==========
Newly designed sign in related histograms for Android

This CL is dedicated to implement newly designed sign in related histograms
(Signin.SigninStartedAccessPoint, Signin.SigninCompletedAccessPoint,
Signin.SigninReason) for Android. Please refer
https://docs.google.com/a/google.com/document/d/1-gXYAMXXgsJhk6jxO55RuYJ00JBG...
for details.

BUG=532557
==========

[gogerald1, 2016-01-08 22:53:33]

Description was changed from

==========
Newly designed sign in related histograms for Android

This CL is dedicated to implement newly designed sign in related histograms
(Signin.SigninStartedAccessPoint, Signin.SigninCompletedAccessPoint,
Signin.SigninReason) for Android. Please refer
https://docs.google.com/a/google.com/document/d/1-gXYAMXXgsJhk6jxO55RuYJ00JBG...
for details.

BUG=532557
==========

to

==========
Implementation of newly designed sign in related histograms for Android.

This CL is dedicated to implement newly designed sign in related histograms
(Signin.SigninStartedAccessPoint, Signin.SigninCompletedAccessPoint,
Signin.SigninReason) for Android. Please refer
https://docs.google.com/a/google.com/document/d/1-gXYAMXXgsJhk6jxO55RuYJ00JBG...
for details.

BUG=532557
==========

[gogerald1, 2016-01-08 23:01:37]

gogerald@chromium.org changed reviewers:
+ nyquist@chromium.org, rogerta@chromium.org

[gogerald1, 2016-01-08 23:01:37]

Hi Roger and Tommy, please help review this CL.

[Roger Tawa OOO till Jul 10th, 2016-01-11 19:59:46]

Hi Ganggui,

Maybe of the places implement the interfaces SigninStateObserver and
SigninFlowObserver.  You could implement an adapter for these interfaces so that
callers don't need to make the uma calls directly.

Also, those two interfaces are pretty similar:

https://code.google.com/p/chromium/codesearch#chromium/src/chrome/android/jav...

https://code.google.com/p/chromium/codesearch#chromium/src/chrome/android/jav...

And they're both declared in SigninManager. Don't know if there is a possibility
to get rid of one.

https://codereview.chromium.org/1578433002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/firstrun/FirstRunSignInProcessor.java
(right):

https://codereview.chromium.org/1578433002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/firstrun/FirstRunSignInProcessor.java:115:
SigninManager.ACCESS_POINT_MAX);
The code for started and completed should be moved to a common location do that
it is not duplicated.

[gogerald1, 2016-01-21 19:20:52]

Patchset #2 (id:40001) has been deleted

[gogerald1, 2016-01-21 19:38:11]

Patchset #2 (id:60001) has been deleted

[gogerald1, 2016-01-21 19:42:44]

Patchset #2 (id:80001) has been deleted

[gogerald1, 2016-01-21 19:42:50]

Patchset #2 (id:100001) has been deleted

[gogerald1, 2016-01-21 19:45:22]

Patchset #2 (id:120001) has been deleted

[gogerald1, 2016-01-21 20:39:10]

Hi Roger,

I changed another way to implement it to make it neat. PTAL.

Thanks,
Ganggui,

https://codereview.chromium.org/1578433002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/firstrun/FirstRunSignInProcessor.java
(right):

https://codereview.chromium.org/1578433002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/firstrun/FirstRunSignInProcessor.java:115:
SigninManager.ACCESS_POINT_MAX);
On 2016/01/11 19:59:45, Roger Tawa wrote:
> The code for started and completed should be moved to a common location do
that
> it is not duplicated.

Done.

[Roger Tawa OOO till Jul 10th, 2016-01-21 20:51:16]

lgtm

[gogerald1, 2016-01-25 23:45:19]

On 2016/01/21 20:51:16, Roger Tawa wrote:
> lgtm

Friendly ping Tommy,

[nyquist, 2016-01-26 19:56:50]

https://codereview.chromium.org/1578433002/diff/140001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/signin/SigninManager.java
(right):

https://codereview.chromium.org/1578433002/diff/140001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/signin/SigninManager.java:53:
* signin_metrics.h.*/
Could these be auto-generated like other enums?

[gogerald1, 2016-01-26 22:55:10]

Patchset #3 (id:160001) has been deleted

[gogerald1, 2016-01-26 22:59:08]

gogerald@chromium.org changed reviewers:
+ sky@chromium.org

[gogerald1, 2016-01-26 22:59:08]

sky@chromium.org: Please review changes in 

chrome/BUILD.gn
chrome/chrome.gyp

https://codereview.chromium.org/1578433002/diff/140001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/signin/SigninManager.java
(right):

https://codereview.chromium.org/1578433002/diff/140001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/signin/SigninManager.java:53:
* signin_metrics.h.*/
On 2016/01/26 19:56:50, nyquist wrote:
> Could these be auto-generated like other enums?

Done.

[nyquist, 2016-01-26 23:09:38]

https://codereview.chromium.org/1578433002/diff/180001/chrome/BUILD.gn
File chrome/BUILD.gn (right):

https://codereview.chromium.org/1578433002/diff/180001/chrome/BUILD.gn#newcod...
chrome/BUILD.gn:946: java_cpp_enum("signin_metrics_enum_javagen") {
Do you need to depend on this in chrome/android/BUILD.gn as well?

https://codereview.chromium.org/1578433002/diff/180001/chrome/BUILD.gn#newcod...
chrome/BUILD.gn:948: "../components/signin/core/browser/signin_metrics.h",
It's a little bit odd to me that this does not live i n//components/signing, but
I guess that's fine since others are doing the same thing.

[gogerald1, 2016-01-27 16:25:05]

Patchset #4 (id:200001) has been deleted

[gogerald1, 2016-01-27 16:50:39]

PTAL.

https://codereview.chromium.org/1578433002/diff/180001/chrome/BUILD.gn
File chrome/BUILD.gn (right):

https://codereview.chromium.org/1578433002/diff/180001/chrome/BUILD.gn#newcod...
chrome/BUILD.gn:946: java_cpp_enum("signin_metrics_enum_javagen") {
On 2016/01/26 23:09:37, nyquist wrote:
> Do you need to depend on this in chrome/android/BUILD.gn as well?

Done.

https://codereview.chromium.org/1578433002/diff/180001/chrome/BUILD.gn#newcod...
chrome/BUILD.gn:948: "../components/signin/core/browser/signin_metrics.h",
On 2016/01/26 23:09:37, nyquist wrote:
> It's a little bit odd to me that this does not live i n//components/signing,
but
> I guess that's fine since others are doing the same thing.

Yes, I guess the reason is the generated enums are used in org.chromium.chrome.*
but not org.chromium.components.*.

[gogerald1, 2016-02-01 17:59:00]

Friendly ping Tommy!

[nyquist, 2016-02-03 17:47:09]

lgtm

[sky, 2016-02-04 17:17:14]

LGTM

[gogerald1, 2016-02-04 17:21:56]

The CQ bit was checked by gogerald@chromium.org

[gogerald1, 2016-02-04 17:21:56]

The patchset sent to the CQ was uploaded after l-g-t-m from
rogerta@chromium.org, nyquist@chromium.org
Link to the patchset: https://codereview.chromium.org/1578433002/#ps240001
(title: "rebase")

[commit-bot: I haz the power, 2016-02-04 17:22:23]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1578433002/240001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1578433002/240001

[commit-bot: I haz the power, 2016-02-04 17:40:30]

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

[commit-bot: I haz the power, 2016-02-04 17:40:30]

Try jobs failed on following builders:
  android_arm64_dbg_recipe on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_clang_dbg_recipe on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  linux_android_rel_ng on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[gogerald1, 2016-02-04 17:58:47]

The CQ bit was checked by gogerald@chromium.org

[gogerald1, 2016-02-04 17:58:48]

The patchset sent to the CQ was uploaded after l-g-t-m from
nyquist@chromium.org, rogerta@chromium.org, sky@chromium.org
Link to the patchset: https://codereview.chromium.org/1578433002/#ps260001
(title: "import RecordHistogram in SigninManager")

[commit-bot: I haz the power, 2016-02-04 17:59:06]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1578433002/260001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1578433002/260001

[commit-bot: I haz the power, 2016-02-04 19:06:10]

Description was changed from

==========
Implementation of newly designed sign in related histograms for Android.

This CL is dedicated to implement newly designed sign in related histograms
(Signin.SigninStartedAccessPoint, Signin.SigninCompletedAccessPoint,
Signin.SigninReason) for Android. Please refer
https://docs.google.com/a/google.com/document/d/1-gXYAMXXgsJhk6jxO55RuYJ00JBG...
for details.

BUG=532557
==========

to

==========
Implementation of newly designed sign in related histograms for Android.

This CL is dedicated to implement newly designed sign in related histograms
(Signin.SigninStartedAccessPoint, Signin.SigninCompletedAccessPoint,
Signin.SigninReason) for Android. Please refer
https://docs.google.com/a/google.com/document/d/1-gXYAMXXgsJhk6jxO55RuYJ00JBG...
for details.

BUG=532557
==========

[commit-bot: I haz the power, 2016-02-04 19:06:11]

Committed patchset #6 (id:260001)

[commit-bot: I haz the power, 2016-02-04 19:07:12]

Description was changed from

==========
Implementation of newly designed sign in related histograms for Android.

This CL is dedicated to implement newly designed sign in related histograms
(Signin.SigninStartedAccessPoint, Signin.SigninCompletedAccessPoint,
Signin.SigninReason) for Android. Please refer
https://docs.google.com/a/google.com/document/d/1-gXYAMXXgsJhk6jxO55RuYJ00JBG...
for details.

BUG=532557
==========

to

==========
Implementation of newly designed sign in related histograms for Android.

This CL is dedicated to implement newly designed sign in related histograms
(Signin.SigninStartedAccessPoint, Signin.SigninCompletedAccessPoint,
Signin.SigninReason) for Android. Please refer
https://docs.google.com/a/google.com/document/d/1-gXYAMXXgsJhk6jxO55RuYJ00JBG...
for details.

BUG=532557

Committed: https://crrev.com/5a9e2b34d01056dfdc1a60ce418f5e9f884c9967
Cr-Commit-Position: refs/heads/master@{#373577}
==========

[commit-bot: I haz the power, 2016-02-04 19:07:13]

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