Revoking the primary refresh token directly in the ProfileOAuth2TokenService should not be happening in production. I don't think this happens currently, so I am not too worried. Maybe Mihai knows more about it though.

We were talking about that recently, and about adding safeguards in the code. One idea that was discussed was to intercept all calls to revoke the primary token (at the token service level, or close to it), and instead of removing the token, replace it by a dummy invalid token. We did not do that yet, and we don't even have a clean design for it (because the token service does not know about the primary account currently).

One other option may be to add such a safeguard at the identity service layer, but I don't think we discussed that yet.

I would not recommend merging chromium-review.googlesource.com/c/chromium/src/+/1264203 to M-70 now, because we're late in the cycle, and I can't think of a concrete bug that could be caused by the old behavior.

Merging the bug would probably be fine too though, because it looks pretty safe, so in any case we probably cannot go wrong.

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

commit 2ac90d1edba884df6f85cb726a70bfb6286af52f
Author: Colin Blundell <blundell@chromium.org>
Date: Fri Oct 05 10:18:45 2018

Fix IdentityManager::HasPrimaryAccount()

"Signing out" always happens via SigninManager and causes the account to
be removed from SigninManager as well as AccountTrackerService. However,
the primary account can be removed from AccountTrackerService in
circumstances *other than* the user signing out of the primary account;
e.g., if the refresh token for the account is revoked from
ProfileOAuth2TokenService. In these circumstances, there would still be
a primary account (i.e., SigninManagerBase::GetAuthenticatedAccountId()
returns a valid account ID), but there would be no AccountInfo for the
account (i.e., SigninManagerBase::GetAuthenticatedAccountInfo() returns
an empty AccountInfo).

IdentityManager::HasPrimaryAccount() currently internally uses
SigninManagerBase::GetAuthenticatedAccountInfo(). This will result in it
giving an incorrect result in these edge cases. This CL changes it to
use SigninManagerBase::IsAuthenticated() and adds a test that fails
before the change.

Bug:  892553 
Change-Id: I227ad6767089b259c0d54e33f5f88f6789f2de9a
Reviewed-on: https://chromium-review.googlesource.com/c/1264203
Reviewed-by: David Roger <droger@chromium.org>
Commit-Queue: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597064}
[modify] https://crrev.com/2ac90d1edba884df6f85cb726a70bfb6286af52f/services/identity/public/cpp/identity_manager.cc
[modify] https://crrev.com/2ac90d1edba884df6f85cb726a70bfb6286af52f/services/identity/public/cpp/identity_manager_unittest.cc

David, thanks for the analysis in c#2! That matches my feelings. 

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

commit 8f92feb2de04e3fca8801732848f0085372fa043
Author: Colin Blundell <blundell@chromium.org>
Date: Tue Oct 09 10:56:52 2018

IdentityManager: Eliminate usage of GetPrimaryAccountInfo().account_id

Before https://chromium-review.googlesource.com/c/1264162 landed,
the recommended equivalent to
SigninManagerBase::GetAuthenticatedAccountId() was
IdentityManager::GetPrimaryAccountInfo().account_id. However, as
detailed in  crbug.com/892553 , this replacement was actually potentially
problematic: IdentityManager::GetPrimaryAccountInfo() can return an
empty struct in cases where
SigninManagerBase::GetAuthenticatedAccountId() would return a valid
ID (e.g., the refresh token of the primary account was revoked without
the user signing out).

This CL ports all consumers to use
IdentityManager::GetPrimaryAccountId(), which is a strict equivalent
to SigninManagerBase::GetAuthenticatedAccountId(). I looked through
git history for all the production consumers being converted in this
CL to verify that they were all either (1) ported from using
SigninManagerBase::GetAuthenticatedAccountId(), or (2) developed from
scratch using IdentityManager. In the latter case, this change is
correct as this API is now the canonical way to get the primary account
ID.

TBR=jochen@chromium.org

Bug:  892553 
Change-Id: I8c1718fadb6c443f18d1bcbe1dae475465f5d914
Reviewed-on: https://chromium-review.googlesource.com/c/1269729
Reviewed-by: Colin Blundell <blundell@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Commit-Queue: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597875}
[modify] https://crrev.com/8f92feb2de04e3fca8801732848f0085372fa043/chrome/browser/chromeos/cryptauth/chrome_cryptauth_service.cc
[modify] https://crrev.com/8f92feb2de04e3fca8801732848f0085372fa043/chrome/browser/chromeos/login/session/user_session_manager.cc
[modify] https://crrev.com/8f92feb2de04e3fca8801732848f0085372fa043/chrome/browser/extensions/api/identity/identity_apitest.cc
[modify] https://crrev.com/8f92feb2de04e3fca8801732848f0085372fa043/chrome/browser/profiles/profile_downloader.cc
[modify] https://crrev.com/8f92feb2de04e3fca8801732848f0085372fa043/chrome/browser/ui/desktop_ios_promotion/sms_service.cc
[modify] https://crrev.com/8f92feb2de04e3fca8801732848f0085372fa043/chromeos/services/device_sync/device_sync_service_unittest.cc
[modify] https://crrev.com/8f92feb2de04e3fca8801732848f0085372fa043/components/browser_sync/profile_sync_service_unittest.cc
[modify] https://crrev.com/8f92feb2de04e3fca8801732848f0085372fa043/components/browser_sync/sync_auth_manager.cc
[modify] https://crrev.com/8f92feb2de04e3fca8801732848f0085372fa043/components/feed/core/feed_networking_host.cc
[modify] https://crrev.com/8f92feb2de04e3fca8801732848f0085372fa043/components/gcm_driver/account_tracker.cc
[modify] https://crrev.com/8f92feb2de04e3fca8801732848f0085372fa043/components/history/core/browser/web_history_service.cc
[modify] https://crrev.com/8f92feb2de04e3fca8801732848f0085372fa043/components/ntp_snippets/remote/remote_suggestions_fetcher_impl.cc
[modify] https://crrev.com/8f92feb2de04e3fca8801732848f0085372fa043/services/identity/public/cpp/identity_test_utils.cc
[modify] https://crrev.com/8f92feb2de04e3fca8801732848f0085372fa043/services/identity/public/cpp/primary_account_access_token_fetcher.cc