Signin: Reuse existing "update gms" dialogs when possible.

Signin: Reuse existing "update gms" dialogs when possible.

Currently, |updateAccounts()| is sometimes called multiple times in
AccountSigninView which results in multiple "update GMS" dialogs being
generated, and the user must click back three times for the dialogs to
disappear.

This CL changes signin to re-use existing dialogs where possible and
always remove old dialogs before creating a new one.

BUG=679851
Review-Url: https://codereview.chromium.org/2626083004
Cr-Commit-Position: refs/heads/master@{#443349}
Committed: https://chromium.googlesource.com/chromium/src/+/5a7ffae71d0dcf646d26ef202f8d1882587724c7

[estevenson, 2017-01-11 19:35:11]

estevenson@chromium.org changed reviewers:
+ bauerb@chromium.org

[estevenson, 2017-01-11 19:35:12]

ptal Bernhard!

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

https://codereview.chromium.org/2626083004/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/signin/AccountSigninView.java:340:
boolean cancelable = !SigninManager.get(getContext()).isForceSigninEnabled();
I couldn't figure out if this can change during the lifetime of
|mGooglePlayServicesUpdateErrorHandler|, any thoughts?

[estevenson, 2017-01-11 19:36:29]

Description was changed from

==========
Signin: Reuse existing "update gms dialogs" when possible.

Currently, |updateAccounts()| is sometimes called multiple times in
AccountSigninView which results in multiple "update GMS" dialogs being
generated, and the user must click back three times for the dialogs to
disappear.

This CL changes signin to re-use existing dialogs where possible and
always remove old dialogs before creating a new one.

BUG=679851
==========

to

==========
Signin: Reuse existing "update gms dialogs" when possible.

Currently, |updateAccounts()| is sometimes called multiple times in
AccountSigninView which results in multiple "update GMS" dialogs being
generated, and the user must click back three times for the dialogs to
disappear.

This CL changes signin to re-use existing dialogs where possible and
always remove old dialogs before creating a new one.

BUG=679851
==========

[estevenson, 2017-01-11 19:36:55]

Description was changed from

==========
Signin: Reuse existing "update gms dialogs" when possible.

Currently, |updateAccounts()| is sometimes called multiple times in
AccountSigninView which results in multiple "update GMS" dialogs being
generated, and the user must click back three times for the dialogs to
disappear.

This CL changes signin to re-use existing dialogs where possible and
always remove old dialogs before creating a new one.

BUG=679851
==========

to

==========
Signin: Reuse existing "update gms" dialogs when possible.

Currently, |updateAccounts()| is sometimes called multiple times in
AccountSigninView which results in multiple "update GMS" dialogs being
generated, and the user must click back three times for the dialogs to
disappear.

This CL changes signin to re-use existing dialogs where possible and
always remove old dialogs before creating a new one.

BUG=679851
==========

[Bernhard Bauer, 2017-01-12 10:14:33]

lgtm

https://codereview.chromium.org/2626083004/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/externalauth/UserRecoverableErrorHandler.java
(right):

https://codereview.chromium.org/2626083004/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/externalauth/UserRecoverableErrorHandler.java:141:
* Error code from Google Play Services.
Nit: "Last error code [...]"?

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

https://codereview.chromium.org/2626083004/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/signin/AccountSigninView.java:340:
boolean cancelable = !SigninManager.get(getContext()).isForceSigninEnabled();
On 2017/01/11 19:35:12, estevenson wrote:
> I couldn't figure out if this can change during the lifetime of
> |mGooglePlayServicesUpdateErrorHandler|, any thoughts?

According to
https://cs.chromium.org/chromium/src/components/policy/resources/policy_templ...,
changing the policy while the brower is running is not supported.

[estevenson, 2017-01-12 19:52:33]

https://codereview.chromium.org/2626083004/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/externalauth/UserRecoverableErrorHandler.java
(right):

https://codereview.chromium.org/2626083004/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/externalauth/UserRecoverableErrorHandler.java:141:
* Error code from Google Play Services.
On 2017/01/12 10:14:33, Bernhard Bauer wrote:
> Nit: "Last error code [...]"?

Done.

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

https://codereview.chromium.org/2626083004/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/signin/AccountSigninView.java:340:
boolean cancelable = !SigninManager.get(getContext()).isForceSigninEnabled();
On 2017/01/12 10:14:33, Bernhard Bauer wrote:
> On 2017/01/11 19:35:12, estevenson wrote:
> > I couldn't figure out if this can change during the lifetime of
> > |mGooglePlayServicesUpdateErrorHandler|, any thoughts?
> 
> According to
>
https://cs.chromium.org/chromium/src/components/policy/resources/policy_templ...,
> changing the policy while the brower is running is not supported. 

Ahh cool, thanks!

[estevenson, 2017-01-12 19:52:43]

The CQ bit was checked by estevenson@chromium.org

[estevenson, 2017-01-12 19:52:44]

The patchset sent to the CQ was uploaded after l-g-t-m from bauerb@chromium.org
Link to the patchset: https://codereview.chromium.org/2626083004/#ps20001
(title: ""Error code" -> "Last error code"")

[commit-bot: I haz the power, 2017-01-12 19:53:36]

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-01-12 20:44:20]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1484250763883060,
"parent_rev": "d8a99a8083a992e00dc0316cc621ad9f0d9b6699", "commit_rev":
"5a7ffae71d0dcf646d26ef202f8d1882587724c7"}

[commit-bot: I haz the power, 2017-01-12 20:44:50]

Description was changed from

==========
Signin: Reuse existing "update gms" dialogs when possible.

Currently, |updateAccounts()| is sometimes called multiple times in
AccountSigninView which results in multiple "update GMS" dialogs being
generated, and the user must click back three times for the dialogs to
disappear.

This CL changes signin to re-use existing dialogs where possible and
always remove old dialogs before creating a new one.

BUG=679851
==========

to

==========
Signin: Reuse existing "update gms" dialogs when possible.

Currently, |updateAccounts()| is sometimes called multiple times in
AccountSigninView which results in multiple "update GMS" dialogs being
generated, and the user must click back three times for the dialogs to
disappear.

This CL changes signin to re-use existing dialogs where possible and
always remove old dialogs before creating a new one.

BUG=679851

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

[commit-bot: I haz the power, 2017-01-12 20:44:51]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/5a7ffae71d0dcf646d26ef202f8d...