🔍 Add first run dialog for selecting search engine

🔍 Add first run dialog for selecting search engine

* Adds a new DefaultSearchEngineFirstRunFragment that's
  immediately after data saver and before account sign-in.
  It was put here because the sign-in logic is scary and
  doesn't normally allow a user to back up into it, which
  a user could do if the search engine dialog came after
  it.

* Updates/adds tests to see if the search engine dialog
  appears in the correct spot and allows recording a
  search engine.

Screenshots: go/eabws

BUG=712836, 712833
Review-Url: https://codereview.chromium.org/2844323003
Cr-Commit-Position: refs/heads/master@{#469291}
Committed: https://chromium.googlesource.com/chromium/src/+/5d6d7b17572cdebafb10e694db58eccc787e384f

[gone, 2017-04-28 04:52:11]

Not ready for review.  Just uploaded as a "hey, this is coming".

[gone, 2017-05-03 02:03:27]

Description was changed from

==========
🔍 Add first run dialog for selecting search engine

Screenshots =
https://drive.google.com/corp/drive/u/0/folders/0B7c8ZkXVwskDcWlEdXdOVWctZjQ

BUG=712836,712833
==========

to

==========
🔍 Add first run dialog for selecting search engine

* Adds a new DefaultSearchEngineFirstRunFragment that's
  immediately after data saver and before account sign-in.
  It was put here because the sign-in logic is scary and
  doesn't normally allow a user to back up into it, which
  a user could do if the search engine dialog came after
  it.

* Updates/adds tests to see if the search engine dialog
  appears in the correct spot and allows recording a
  search engine.

Screenshots: go/eabws

BUG=712836,712833
==========

[gone, 2017-05-03 02:07:45]

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

[gone, 2017-05-03 02:11:38]

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

[commit-bot: I haz the power, 2017-05-03 02:12:18]

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, 2017-05-03 03:24:13]

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

[commit-bot: I haz the power, 2017-05-03 03:24:14]

Dry run: This issue passed the CQ dry run.

[Ted C, 2017-05-04 00:16:57]

https://codereview.chromium.org/2844323003/diff/60001/chrome/android/java/res...
File chrome/android/java/res/layout/default_search_engine_first_run_fragment.xml
(right):

https://codereview.chromium.org/2844323003/diff/60001/chrome/android/java/res...
chrome/android/java/res/layout/default_search_engine_first_run_fragment.xml:16:
<FrameLayout
Why do we need this?  Any reason FirstRunChooserView isn't here instead?

https://codereview.chromium.org/2844323003/diff/60001/chrome/android/java/res...
chrome/android/java/res/layout/default_search_engine_first_run_fragment.xml:48:
<View
I assume this can't use the Divider style? 
https://cs.chromium.org/chromium/src/chrome/android/java/res/values-v17/style...

https://codereview.chromium.org/2844323003/diff/60001/chrome/android/java/res...
chrome/android/java/res/layout/default_search_engine_first_run_fragment.xml:71:
android:paddingStart="@dimen/signin_chooser_padding"
use android:padding= to replace these four?

https://codereview.chromium.org/2844323003/diff/60001/chrome/android/javatest...
File
chrome/android/javatests/src/org/chromium/chrome/browser/firstrun/FirstRunIntegrationTest.java
(right):

https://codereview.chromium.org/2844323003/diff/60001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/firstrun/FirstRunIntegrationTest.java:236:
private String mSelectedEngine;
move to top?

[gone, 2017-05-04 00:28:12]

https://codereview.chromium.org/2844323003/diff/60001/chrome/android/java/res...
File chrome/android/java/res/layout/default_search_engine_first_run_fragment.xml
(right):

https://codereview.chromium.org/2844323003/diff/60001/chrome/android/java/res...
chrome/android/java/res/layout/default_search_engine_first_run_fragment.xml:16:
<FrameLayout
On 2017/05/04 00:16:56, Ted C wrote:
> Why do we need this?  Any reason FirstRunChooserView isn't here instead?

FirstRunChooserView asserts its layout parameters are match_parent.  Don't know
why, I just pulled it out into its own class -_-

https://codereview.chromium.org/2844323003/diff/60001/chrome/android/java/res...
chrome/android/java/res/layout/default_search_engine_first_run_fragment.xml:48:
<View
On 2017/05/04 00:16:56, Ted C wrote:
> I assume this can't use the Divider style? 
>
https://cs.chromium.org/chromium/src/chrome/android/java/res/values-v17/style...

Was intentionally not breaking from the sign in page layout.  I'll change both. 
Doubt anyone will notice the difference between 8% black and google_grey_300.

https://codereview.chromium.org/2844323003/diff/60001/chrome/android/java/res...
chrome/android/java/res/layout/default_search_engine_first_run_fragment.xml:71:
android:paddingStart="@dimen/signin_chooser_padding"
On 2017/05/04 00:16:56, Ted C wrote:
> use android:padding= to replace these four?

Done.

https://codereview.chromium.org/2844323003/diff/60001/chrome/android/javatest...
File
chrome/android/javatests/src/org/chromium/chrome/browser/firstrun/FirstRunIntegrationTest.java
(right):

https://codereview.chromium.org/2844323003/diff/60001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/firstrun/FirstRunIntegrationTest.java:236:
private String mSelectedEngine;
On 2017/05/04 00:16:57, Ted C wrote:
> move to top?

Done.

[gone, 2017-05-04 00:28:17]

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

[commit-bot: I haz the power, 2017-05-04 00:28:56]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Ted C, 2017-05-04 00:42:43]

lgtm

https://codereview.chromium.org/2844323003/diff/80001/chrome/android/java/res...
File chrome/android/java/res/layout/account_signin_view.xml (left):

https://codereview.chromium.org/2844323003/diff/80001/chrome/android/java/res...
chrome/android/java/res/layout/account_signin_view.xml:51:
android:background="@color/signin_border_line_color"
can we remove this color from the colors.xml file?

[gone, 2017-05-04 01:25:34]

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

[gone, 2017-05-04 01:25:36]

+ilya for histograms.xml

https://codereview.chromium.org/2844323003/diff/80001/chrome/android/java/res...
File chrome/android/java/res/layout/account_signin_view.xml (left):

https://codereview.chromium.org/2844323003/diff/80001/chrome/android/java/res...
chrome/android/java/res/layout/account_signin_view.xml:51:
android:background="@color/signin_border_line_color"
On 2017/05/04 00:42:43, Ted C wrote:
> can we remove this color from the colors.xml file?

Nope.  It's used a few times for other dividers below.  I just swapped them all
over.

[gone, 2017-05-04 01:25:52]

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

[commit-bot: I haz the power, 2017-05-04 01:26:26]

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, 2017-05-04 02:38:42]

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

[commit-bot: I haz the power, 2017-05-04 02:38:43]

Dry run: This issue passed the CQ dry run.

[Ilya Sherman, 2017-05-04 04:12:35]

histograms.xml lgtm

[gone, 2017-05-04 04:14:34]

The CQ bit was checked by dfalcantara@chromium.org

[gone, 2017-05-04 04:14:35]

The patchset sent to the CQ was uploaded after l-g-t-m from tedchoc@chromium.org
Link to the patchset: https://codereview.chromium.org/2844323003/#ps100001
(title: "Replace dividers")

[commit-bot: I haz the power, 2017-05-04 04:14:47]

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, 2017-05-04 04:22:45]

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

[commit-bot: I haz the power, 2017-05-04 04:22:47]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[gone, 2017-05-04 06:24:16]

The CQ bit was checked by dfalcantara@chromium.org

[gone, 2017-05-04 06:24:16]

The patchset sent to the CQ was uploaded after l-g-t-m from
tedchoc@chromium.org, isherman@chromium.org
Link to the patchset: https://codereview.chromium.org/2844323003/#ps120001
(title: "Rebase")

[commit-bot: I haz the power, 2017-05-04 06:24:33]

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, 2017-05-04 07:32:30]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1493879056357320,
"parent_rev": "c17b37ab36a216c3923da51e382d531efc61c35e", "commit_rev":
"5d6d7b17572cdebafb10e694db58eccc787e384f"}

[commit-bot: I haz the power, 2017-05-04 07:32:42]

Description was changed from

==========
🔍 Add first run dialog for selecting search engine

* Adds a new DefaultSearchEngineFirstRunFragment that's
  immediately after data saver and before account sign-in.
  It was put here because the sign-in logic is scary and
  doesn't normally allow a user to back up into it, which
  a user could do if the search engine dialog came after
  it.

* Updates/adds tests to see if the search engine dialog
  appears in the correct spot and allows recording a
  search engine.

Screenshots: go/eabws

BUG=712836,712833
==========

to

==========
🔍 Add first run dialog for selecting search engine

* Adds a new DefaultSearchEngineFirstRunFragment that's
  immediately after data saver and before account sign-in.
  It was put here because the sign-in logic is scary and
  doesn't normally allow a user to back up into it, which
  a user could do if the search engine dialog came after
  it.

* Updates/adds tests to see if the search engine dialog
  appears in the correct spot and allows recording a
  search engine.

Screenshots: go/eabws

BUG=712836,712833

Review-Url: https://codereview.chromium.org/2844323003
Cr-Commit-Position: refs/heads/master@{#469291}
Committed:
https://chromium.googlesource.com/chromium/src/+/5d6d7b17572cdebafb10e694db58...
==========

[commit-bot: I haz the power, 2017-05-04 07:32:43]

Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/5d6d7b17572cdebafb10e694db58...