This is really bad. We need to fix this for M59 (if not for M58).

Sorry, just to make sure I understand: you're tapping on the auth error row at the top of the Sync settings page, expecting it to bring up the OS-level flow for re-auth, and it's not doing so (i.e. tapping it does nothing). Is that right?

I actually don't think this is that severe of an issue. IIRC, we knew about this when we launched the revamp to our settings UI (+Ganggui to confirm that's correct). This is still strictly better than what we used to show in our error UI, which also wasn't tappable. And you will always still have the Android system notification in your notification tray that you can tap on to start the re-auth flow.

My understanding was that it was impossible to bring up the MM flow for re-auth from Chrome UI. If that's not impossible, we should certainly try to add that in as soon as possible, to make the experience from Chrome that much better. But I wouldn't treat this as a P1, personally.

I thought we would mostly depend on the system UI for re-auth.

Yeah, we depend on the system UI for the actual re-auth itself. I think the question here is whether the user should be able to tap on the auth error they see in the Chrome Sync settings UI, and have that bring up the system UI.

I think ideally that would be the behavior, but don't see it as a major deficiency if the user has to tap on the notification in the system tray in order to bring up the re-auth UI. If it's technically feasible to implement though, then I don't see a reason not to.

I talked to Boris, and it should be technically possible to bring up the reauth UI by requesting a token with GoogleAuthUtil.getToken() (as opposed to getTokenWithNotification()), and then launching the recovery intent when that fails. I would also like that ability for issue 697491, as Chrome is in an unusable state without it.

Got it, sounds good to me. So are you blocked for Issue 697491 until we make this change?

I can work on my change independently, but until this issue is fixed, the dialog won't actually start reauth, which removes the obvious way for users to get out of that state.

Boris, let me know if I can help with this!

Got it, makes sense. Chatted with Boris during our weekly today, and he's on it.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6e26494d0caa38a608ecd3c7852a58387dcaaf96

commit 6e26494d0caa38a608ecd3c7852a58387dcaaf96
Author: bsazonov <bsazonov@chromium.org>
Date: Fri Mar 10 19:10:56 2017

Fixed permission check in SystemAccountManagerDelegate.updateCredentials

Checking hasGetAccountsPermission before calling android.AccountManager.updateCredentials
is irrelevant, because this call never required GET_ACCOUNTS permission.
Added check for MANAGE_ACCOUNTS permission that was required before
Android M. On M+ it doesn't require any special permission.

BUG= 699050 

Review-Url: https://codereview.chromium.org/2745533002
Cr-Commit-Position: refs/heads/master@{#456124}

[modify] https://crrev.com/6e26494d0caa38a608ecd3c7852a58387dcaaf96/components/signin/core/browser/android/java/src/org/chromium/components/signin/SystemAccountManagerDelegate.java

Boris, after this fix, will tapping on the auth error in sync settings automatically bring up the system UI for re-authentication?

Current fix works for Android N/M, but doesn't work for KitKat (something inside AccountManager throws an exception). I've found the possible cause. Reopening the bug as a reminder.

Yes, after this fix tapping the "Sync error" card should bring up the password input screen.

Terrific, just tested on Canary and it works as expected :) Thanks Boris!

Just for reference, here is the error I receive on KitKat:

W/AccountAuthenticator( 7175): updateCredentials(Account {name=cipartest@gmail.com, type=com.google},android)
W/AccountAuthenticator( 7175): java.lang.NullPointerException
W/AccountAuthenticator( 7175):  at android.os.Bundle.putAll(Bundle.java:313)
W/AccountAuthenticator( 7175):  at android.content.Intent.putExtras(Intent.java:6145)
W/AccountAuthenticator( 7175):  at com.google.android.gsf.loginservice.GoogleLoginService$AccountAuthenticatorImpl.multiProcessHopFix(GoogleLoginService.java:541)
W/AccountAuthenticator( 7175):  at com.google.android.gsf.loginservice.GoogleLoginService$AccountAuthenticatorImpl.updateCredentials(GoogleLoginService.java:580)
W/AccountAuthenticator( 7175):  at android.accounts.AbstractAccountAuthenticator$Transport.updateCredentials(AbstractAccountAuthenticator.java:220)
W/AccountAuthenticator( 7175):  at android.accounts.IAccountAuthenticator$Stub.onTransact(IAccountAuthenticator.java:147)
W/AccountAuthenticator( 7175):  at android.os.Binder.execTransact(Binder.java:404)
W/AccountAuthenticator( 7175):  at dalvik.system.NativeStart.run(Native Method)
E/cr_Auth (20269): Error while update credentials: 
E/cr_Auth (20269): android.accounts.AuthenticatorException: updateCredentials failed
E/cr_Auth (20269):      at android.accounts.AccountManager.convertErrorToException(AccountManager.java:1726)
E/cr_Auth (20269):      at android.accounts.AccountManager.access$400(AccountManager.java:144)
E/cr_Auth (20269):      at android.accounts.AccountManager$AmsTask$Response.onError(AccountManager.java:1572)
E/cr_Auth (20269):      at android.accounts.IAccountManagerResponse$Stub.onTransact(IAccountManagerResponse.java:69)
E/cr_Auth (20269):      at android.os.Binder.execTransact(Binder.java:404)
E/cr_Auth (20269):      at dalvik.system.NativeStart.run(Native Method)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/255225462a687af278ab3118122638816dd4a212

commit 255225462a687af278ab3118122638816dd4a212
Author: bsazonov <bsazonov@chromium.org>
Date: Tue Mar 14 16:24:02 2017

Fixed SystemAccountManagerDelegate.updateCredentials on KitKat

AccountManager.updateCredentials on Android KitKat fails because of
NullPointerException in authenticator. Passing empty Bundle instead of null
fixes the problem.

BUG= 699050 

Review-Url: https://codereview.chromium.org/2751733002
Cr-Commit-Position: refs/heads/master@{#456723}

[modify] https://crrev.com/255225462a687af278ab3118122638816dd4a212/components/signin/core/browser/android/java/src/org/chromium/components/signin/SystemAccountManagerDelegate.java

Thanks Boris! Bernhard - do you need this merged back to 58 for any reason?

Yes, that would be good.

OK. Boris - can you please verify your latest fix (for Android K) on the next Canary to make sure it works? Assuming all is well, please add the "Merge-Request-58" label to request a merge for your two CLs back to 58.

Let me know if you have any questions about how the merge process works :)

Works like a charm. Done. I shouldn't change status to "Fixed", should I?

Your change meets the bar and is auto-approved for M58. Please go ahead and merge the CL to branch 3029 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Nope, only set it to "Fixed" after we've merged it.

You'll need to get a committer to help you merge the changes back to 58 (now that it's been approved). Once that happens, feel free to set this as "Fixed."

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9609fe5d7016961497955f2d59b691e84fb92402

commit 9609fe5d7016961497955f2d59b691e84fb92402
Author: Jerome Lebel <jlebel@chromium.org>
Date: Thu Mar 16 10:30:28 2017

Fixed permission check in SystemAccountManagerDelegate.updateCredentials

Checking hasGetAccountsPermission before calling android.AccountManager.updateCredentials
is irrelevant, because this call never required GET_ACCOUNTS permission.
Added check for MANAGE_ACCOUNTS permission that was required before
Android M. On M+ it doesn't require any special permission.

BUG= 699050 

Review-Url: https://codereview.chromium.org/2745533002
Cr-Commit-Position: refs/heads/master@{#456124}
(cherry picked from commit 6e26494d0caa38a608ecd3c7852a58387dcaaf96)

Review-Url: https://codereview.chromium.org/2751973004 .
Cr-Commit-Position: refs/branch-heads/3029@{#235}
Cr-Branched-From: 939b32ee5ba05c396eef3fd992822fcca9a2e262-refs/heads/master@{#454471}

[modify] https://crrev.com/9609fe5d7016961497955f2d59b691e84fb92402/components/signin/core/browser/android/java/src/org/chromium/components/signin/SystemAccountManagerDelegate.java

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/03f56c2731c8135012cc9f6b9625abc4e3567bd8

commit 03f56c2731c8135012cc9f6b9625abc4e3567bd8
Author: Jerome Lebel <jlebel@chromium.org>
Date: Thu Mar 16 14:42:39 2017

Fixed SystemAccountManagerDelegate.updateCredentials on KitKat

AccountManager.updateCredentials on Android KitKat fails because of
NullPointerException in authenticator. Passing empty Bundle instead of null
fixes the problem.

BUG= 699050 

Review-Url: https://codereview.chromium.org/2751733002
Cr-Commit-Position: refs/heads/master@{#456723}
(cherry picked from commit 255225462a687af278ab3118122638816dd4a212)

Review-Url: https://codereview.chromium.org/2756503004 .
Cr-Commit-Position: refs/branch-heads/3029@{#239}
Cr-Branched-From: 939b32ee5ba05c396eef3fd992822fcca9a2e262-refs/heads/master@{#454471}

[modify] https://crrev.com/03f56c2731c8135012cc9f6b9625abc4e3567bd8/components/signin/core/browser/android/java/src/org/chromium/components/signin/SystemAccountManagerDelegate.java

Jerome, thank you for helping me to merge this.

Verified fix in 58.0.3029.33 with steps from #0.
Thanks!