Upstream FirstRunActivity and friends.

Upstream FirstRunActivity and friends.

BUG=428842
Committed: https://crrev.com/6638f6c44a74e4fc788970f6aa94771724634c80
Cr-Commit-Position: refs/heads/master@{#320175}

[aurimas (slooooooooow), 2015-03-03 23:35:34]

aurimas@chromium.org changed reviewers:
+ newt@chromium.org

[aurimas (slooooooooow), 2015-03-03 23:35:35]

newt: PTAL

[newt (away), 2015-03-04 03:47:08]

Are there any changes to the files that have been newly added upstream?

Also, need to update logo images.

https://chromiumcodereview.appspot.com/978653002/diff/1/chrome/android/java/r...
File chrome/android/java/res/layout/fre_page.xml (right):

https://chromiumcodereview.appspot.com/978653002/diff/1/chrome/android/java/r...
chrome/android/java/res/layout/fre_page.xml:2: <!-- Copyright 2014 The Chromium
Authors. All rights reserved.
2015

https://chromiumcodereview.appspot.com/978653002/diff/1/chrome/android/java/r...
File chrome/android/java/res/layout/fre_tosanduma.xml (right):

https://chromiumcodereview.appspot.com/978653002/diff/1/chrome/android/java/r...
chrome/android/java/res/layout/fre_tosanduma.xml:3: Copyright 2011 The Chromium
Authors. All rights reserved.
2015

https://chromiumcodereview.appspot.com/978653002/diff/1/chrome/android/java/s...
File
chrome/android/java/src/org/chromium/chrome/browser/ChromiumApplication.java
(right):

https://chromiumcodereview.appspot.com/978653002/diff/1/chrome/android/java/s...
chrome/android/java/src/org/chromium/chrome/browser/ChromiumApplication.java:62:
public abstract String getFirstRunActivityName();
I think it's also fine to make this non-abstract. Then you don't have to
override it in ChromeShellApplication, and it can simplify later upstreaming a
bit.

[aurimas (slooooooooow), 2015-03-10 22:16:51]

Updated logo to use chromium logo and private repo will have the correct chrome
icon

https://chromiumcodereview.appspot.com/978653002/diff/1/chrome/android/java/r...
File chrome/android/java/res/layout/fre_page.xml (right):

https://chromiumcodereview.appspot.com/978653002/diff/1/chrome/android/java/r...
chrome/android/java/res/layout/fre_page.xml:2: <!-- Copyright 2014 The Chromium
Authors. All rights reserved.
On 2015/03/04 at 03:47:07, newt wrote:
> 2015

Done

https://chromiumcodereview.appspot.com/978653002/diff/1/chrome/android/java/r...
File chrome/android/java/res/layout/fre_tosanduma.xml (right):

https://chromiumcodereview.appspot.com/978653002/diff/1/chrome/android/java/r...
chrome/android/java/res/layout/fre_tosanduma.xml:3: Copyright 2011 The Chromium
Authors. All rights reserved.
On 2015/03/04 at 03:47:08, newt wrote:
> 2015

Done

https://chromiumcodereview.appspot.com/978653002/diff/1/chrome/android/java/s...
File
chrome/android/java/src/org/chromium/chrome/browser/ChromiumApplication.java
(right):

https://chromiumcodereview.appspot.com/978653002/diff/1/chrome/android/java/s...
chrome/android/java/src/org/chromium/chrome/browser/ChromiumApplication.java:62:
public abstract String getFirstRunActivityName();
On 2015/03/04 at 03:47:08, newt wrote:
> I think it's also fine to make this non-abstract. Then you don't have to
override it in ChromeShellApplication, and it can simplify later upstreaming a
bit.

Done

[newt (away), 2015-03-10 22:26:06]

lgtm

[aurimas (slooooooooow), 2015-03-10 22:28:22]

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

[aurimas (slooooooooow), 2015-03-10 22:28:23]

+isherman: please take a look at changes in tools/metrics/*

[Ilya Sherman, 2015-03-10 23:59:31]

https://chromiumcodereview.appspot.com/978653002/diff/40001/chrome/browser/an...
File chrome/browser/android/metrics/uma_bridge.cc (right):

https://chromiumcodereview.appspot.com/978653002/diff/40001/chrome/browser/an...
chrome/browser/android/metrics/uma_bridge.cc:16: namespace {
nit: Please leave a blank line after this one.

https://chromiumcodereview.appspot.com/978653002/diff/40001/chrome/browser/an...
chrome/browser/android/metrics/uma_bridge.cc:38: for (int i = 0; i <
MOBILE_FRE_FINISH_STATE_BOUNDARY; i++)
nit: ++i

https://chromiumcodereview.appspot.com/978653002/diff/40001/chrome/browser/an...
chrome/browser/android/metrics/uma_bridge.cc:41: }
nit: Please leave a blank line after this one.

https://chromiumcodereview.appspot.com/978653002/diff/40001/chrome/browser/an...
chrome/browser/android/metrics/uma_bridge.cc:111:
UMA_HISTOGRAM_CUSTOM_ENUMERATION("MobileFre.FinishState",
Why CUSTOM_ENUMERATION, rather than just regular ENUMERATION?

https://chromiumcodereview.appspot.com/978653002/diff/40001/chrome/browser/an...
chrome/browser/android/metrics/uma_bridge.cc:155: }
Would it make sense to use base/android/record_histogram.cc, rather than adding
new JNI bindings specifically for these histograms?

https://chromiumcodereview.appspot.com/978653002/diff/40001/tools/metrics/his...
File tools/metrics/histograms/histograms.xml (right):

https://chromiumcodereview.appspot.com/978653002/diff/40001/tools/metrics/his...
tools/metrics/histograms/histograms.xml:53799: +  </summary>
Note that enum <summary>s are only shown on hover; hence, they're typically
omitted, or folded into the histogram summary when they are important for users
to see.  That said, if you prefer to keep this, that's fine.

[aurimas (slooooooooow), 2015-03-11 18:49:23]

Fixed up your nits as well as removed MobileFreFinishState as that histogram is
no longer used (confirmed with aruslan@)

https://chromiumcodereview.appspot.com/978653002/diff/40001/chrome/browser/an...
File chrome/browser/android/metrics/uma_bridge.cc (right):

https://chromiumcodereview.appspot.com/978653002/diff/40001/chrome/browser/an...
chrome/browser/android/metrics/uma_bridge.cc:16: namespace {
On 2015/03/10 at 23:59:30, Ilya Sherman wrote:
> nit: Please leave a blank line after this one.

Done

https://chromiumcodereview.appspot.com/978653002/diff/40001/chrome/browser/an...
chrome/browser/android/metrics/uma_bridge.cc:38: for (int i = 0; i <
MOBILE_FRE_FINISH_STATE_BOUNDARY; i++)
On 2015/03/10 at 23:59:30, Ilya Sherman wrote:
> nit: ++i

Done

https://chromiumcodereview.appspot.com/978653002/diff/40001/chrome/browser/an...
chrome/browser/android/metrics/uma_bridge.cc:41: }
On 2015/03/10 at 23:59:30, Ilya Sherman wrote:
> nit: Please leave a blank line after this one.

Done

[Ilya Sherman, 2015-03-11 19:20:20]

Thanks.  LGTM.

https://chromiumcodereview.appspot.com/978653002/diff/60001/tools/metrics/his...
File tools/metrics/histograms/histograms.xml (right):

https://chromiumcodereview.appspot.com/978653002/diff/60001/tools/metrics/his...
tools/metrics/histograms/histograms.xml:14683: +<histogram
name="MobileFre.SignInChoice" enum="MobileFreSignInChoice">
FWIW, I find "Fre" to be a very non-obvious acronym.  I'd much prefer to see
"FirstRunExperience".

[aurimas (slooooooooow), 2015-03-11 19:22:11]

https://chromiumcodereview.appspot.com/978653002/diff/60001/tools/metrics/his...
File tools/metrics/histograms/histograms.xml (right):

https://chromiumcodereview.appspot.com/978653002/diff/60001/tools/metrics/his...
tools/metrics/histograms/histograms.xml:14683: +<histogram
name="MobileFre.SignInChoice" enum="MobileFreSignInChoice">
On 2015/03/11 at 19:20:20, Ilya Sherman wrote:
> FWIW, I find "Fre" to be a very non-obvious acronym.  I'd much prefer to see
"FirstRunExperience".

Sadly, this is used for several releases already and defined in internal version
of histograms.xml so I cannot rename it :'(

[aurimas (slooooooooow), 2015-03-11 20:59:20]

The CQ bit was checked by aurimas@chromium.org

[aurimas (slooooooooow), 2015-03-11 20:59:20]

The patchset sent to the CQ was uploaded after l-g-t-m from newt@chromium.org
Link to the patchset: https://chromiumcodereview.appspot.com/978653002/#ps60001
(title: "Fix nits + remove MobileFreFinishState as it is no longer relevant")

[commit-bot: I haz the power, 2015-03-11 21:00:32]

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

[commit-bot: I haz the power, 2015-03-11 23:05:03]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-03-11 23:05:56]

Patchset 4 (id:??) landed as
https://crrev.com/6638f6c44a74e4fc788970f6aa94771724634c80
Cr-Commit-Position: refs/heads/master@{#320175}