Make account rows in settings tappable

Make account rows in settings tappable

This CL adds click listeners to account rows in Chrome settings on Android.
Tapping the row opens Android settings for the corresponding account.

BUG=722907
Review-Url: https://codereview.chromium.org/2912933003
Cr-Commit-Position: refs/heads/master@{#476373}
Committed: https://chromium.googlesource.com/chromium/src/+/a4b7c0e0a04f46e3668d20b935fc39429597ce7e

[bsazonov, 2017-05-30 17:22:27]

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

[commit-bot: I haz the power, 2017-05-30 17:22:59]

Dry run: CQ is trying da patch.

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

[bsazonov, 2017-05-30 17:31:18]

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

[bsazonov, 2017-05-30 17:31:19]

Dan, please take a look. Side note: this CL will add dividers between account
rows (because rows become selectable), CL to fix it is already in review:
https://codereview.chromium.org/2914513002/.

[commit-bot: I haz the power, 2017-05-30 18:16:36]

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

[commit-bot: I haz the power, 2017-05-30 18:16:38]

Dry run: This issue passed the CQ dry run.

[bsazonov, 2017-05-31 14:33:45]

On 2017/05/30 17:31:19, bsazonov wrote:
> Dan, please take a look. Side note: this CL will add dividers between account
> rows (because rows become selectable), CL to fix it is already in review:
> https://codereview.chromium.org/2914513002/.

CL to fix dividers has landed.

[gone, 2017-05-31 17:34:10]

https://codereview.chromium.org/2912933003/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/signin/AccountManagementFragment.java
(right):

https://codereview.chromium.org/2912933003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/signin/AccountManagementFragment.java:114:
public static final String ACCOUNT_SETTINGS_ACTION =
"android.settings.ACCOUNT_SYNC_SETTINGS";
These don't need to be public.

https://codereview.chromium.org/2912933003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/signin/AccountManagementFragment.java:439:
startActivity(intent);
In general, use IntentUtils.safeStartActivity().  This catches any potential
crashes and prevents Clank from crashing if Android doesn't like the Intent.

[bsazonov, 2017-06-01 11:16:28]

https://codereview.chromium.org/2912933003/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/signin/AccountManagementFragment.java
(right):

https://codereview.chromium.org/2912933003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/signin/AccountManagementFragment.java:114:
public static final String ACCOUNT_SETTINGS_ACTION =
"android.settings.ACCOUNT_SYNC_SETTINGS";
On 2017/05/31 17:34:10, dfalcantara wrote:
> These don't need to be public.

Good point. Made all the string constants in this class private.

https://codereview.chromium.org/2912933003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/signin/AccountManagementFragment.java:439:
startActivity(intent);
On 2017/05/31 17:34:10, dfalcantara wrote:
> In general, use IntentUtils.safeStartActivity().  This catches any potential
> crashes and prevents Clank from crashing if Android doesn't like the Intent.

Didn't know about IntentUtils, thanks. Replaced startActivity with
IntentUtils.safeStartActivity and removed resolveActivity check before it (looks
like signin code is the only place where this pattern is used).

[bsazonov, 2017-06-01 11:16:38]

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

[commit-bot: I haz the power, 2017-06-01 11:16:59]

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-06-01 11:42:01]

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

[commit-bot: I haz the power, 2017-06-01 11:42:02]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)

[bsazonov, 2017-06-01 14:08:02]

Patchset #2 (id:20001) has been deleted

[bsazonov, 2017-06-01 14:08:06]

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

[commit-bot: I haz the power, 2017-06-01 14:08:44]

Dry run: CQ is trying da patch.

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

[bsazonov, 2017-06-01 14:09:58]

https://codereview.chromium.org/2912933003/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/signin/AccountManagementFragment.java
(right):

https://codereview.chromium.org/2912933003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/signin/AccountManagementFragment.java:114:
public static final String ACCOUNT_SETTINGS_ACTION =
"android.settings.ACCOUNT_SYNC_SETTINGS";
On 2017/05/31 17:34:10, dfalcantara wrote:
> These don't need to be public.

Hm, it turns out that we need preference keys to be public for UI tests. Made
ACCOUNT_SETTINGS_* strings private.

[commit-bot: I haz the power, 2017-06-01 14:45:24]

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

[commit-bot: I haz the power, 2017-06-01 14:45:26]

Dry run: This issue passed the CQ dry run.

[gone, 2017-06-01 17:27:05]

lgtm

https://codereview.chromium.org/2912933003/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/signin/AccountManagementFragment.java
(right):

https://codereview.chromium.org/2912933003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/signin/AccountManagementFragment.java:114:
public static final String ACCOUNT_SETTINGS_ACTION =
"android.settings.ACCOUNT_SYNC_SETTINGS";
On 2017/06/01 14:09:58, bsazonov wrote:
> On 2017/05/31 17:34:10, dfalcantara wrote:
> > These don't need to be public.
> 
> Hm, it turns out that we need preference keys to be public for UI tests. Made
> ACCOUNT_SETTINGS_* strings private.

Ah, didn't see that the PREF_SIGN_OUT already existed.  Mea culpa.

[bsazonov, 2017-06-01 18:49:22]

The CQ bit was checked by bsazonov@chromium.org

[commit-bot: I haz the power, 2017-06-01 18:50:18]

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-06-01 18:55:19]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1496342962517140,
"parent_rev": "56e1ea730204dbc043f58b14fec3cb6dfb6a4739", "commit_rev":
"a4b7c0e0a04f46e3668d20b935fc39429597ce7e"}

[commit-bot: I haz the power, 2017-06-01 18:55:29]

Description was changed from

==========
Make account rows in settings tappable

This CL adds click listeners to account rows in Chrome settings on Android.
Tapping the row opens Android settings for the corresponding account.

BUG=722907
==========

to

==========
Make account rows in settings tappable

This CL adds click listeners to account rows in Chrome settings on Android.
Tapping the row opens Android settings for the corresponding account.

BUG=722907

Review-Url: https://codereview.chromium.org/2912933003
Cr-Commit-Position: refs/heads/master@{#476373}
Committed:
https://chromium.googlesource.com/chromium/src/+/a4b7c0e0a04f46e3668d20b935fc...
==========

[commit-bot: I haz the power, 2017-06-01 18:55:31]

Committed patchset #2 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/a4b7c0e0a04f46e3668d20b935fc...