Fix refresh token is not available after Chrome restart on crash.

chrome/browser/chromeos/arc/arc_auth_context.cc

Index: chrome/browser/chromeos/arc/arc_auth_context.cc
diff --git a/chrome/browser/chromeos/arc/arc_auth_context.cc b/chrome/browser/chromeos/arc/arc_auth_context.cc
index dc0e829e1a4b82b2e991210c4d8abf63a6292819..f390168d206ef58b82bbd58f27031402f8d6bed9 100644
--- a/chrome/browser/chromeos/arc/arc_auth_context.cc
+++ b/chrome/browser/chromeos/arc/arc_auth_context.cc
@@ -27,21 +27,18 @@ constexpr base::TimeDelta kRefreshTokenTimeout =
 
 }  // namespace
 
-ArcAuthContext::ArcAuthContext(Profile* profile) {
+ArcAuthContext::ArcAuthContext(Profile* profile) : profile_(profile) {
   // Reuse storage used in ARC OptIn platform app.
   const std::string site_url = base::StringPrintf(
       "%s://%s/persist?%s", content::kGuestScheme, ArcSupportHost::kHostAppId,
       ArcSupportHost::kStorageId);
   storage_partition_ = content::BrowserContext::GetStoragePartitionForSite(
-      profile, GURL(site_url));
+      profile_, GURL(site_url));
   CHECK(storage_partition_);
 
   // Get token service and account ID to fetch auth tokens.
-  token_service_ = ProfileOAuth2TokenServiceFactory::GetForProfile(profile);
-  const SigninManagerBase* const signin_manager =
-      SigninManagerFactory::GetForProfile(profile);
-  CHECK(token_service_ && signin_manager);
-  account_id_ = signin_manager->GetAuthenticatedAccountId();

[khmel, 2017/03/15 18:12:22]

This is race condition on Chrome restart on crash. account_id_ is empty at time
of context creation. Access account_id on time of use.

[xiyuan, 2017/03/15 19:34:30]

Not sure I understand why there is a race. SigninManagerBase should store the
account info in prefs and load it in SigninManagerBase::Initialize. On a
crash-n-restart case, the account id should still be available.

The store code after login is in UserSessionManager::InitProfilePreferences [1].
And SigninManagerBase::Initialize should read it out in Line 92 and 154 [2].

If this is broken, we would have other problems too. Can you double check?

[1]:
https://cs.chromium.org/chromium/src/chrome/browser/chromeos/login/session/us...

[2]:
https://cs.chromium.org/chromium/src/components/signin/core/browser/signin_ma...

[khmel, 2017/03/15 19:39:05]

InitProfilePreferences is not called in case of crash_and_restart (at least at
scenario I tested) because OnProfileCreated is not called.

[xiyuan, 2017/03/15 19:53:16]

InitProfilePreferences stores the info in SigninManagerBase. On crash-n-restart
case, it does not need to be called. SigninManagerBase should get the info in
prefs. Can you track SigninManagerBase::Initialize and see why this does not
happen?

[khmel, 2017/03/16 02:07:24]

So, I investigated this case and can confirm my suspicion.
/home/chronos/XXXX/Preferences has commit interval 10 seconds. If crash happens
between commit and setting account id to prefs then it could not be restored
after crash. And this is the reason of RefreshTokenProblem.
WDYT?

[xiyuan, 2017/03/16 04:37:42]

Thanks for looking into this.

I would prefer to add a client_->GetPrefs()->CommitPendingWrite() at the end of
SigninManagerBase::SetAuthenticatedAccountId() to flush. Would this work? 

[khmel, 2017/03/16 15:41:42]

Yes, this works. 

+  token_service_ = ProfileOAuth2TokenServiceFactory::GetForProfile(profile_);
+  CHECK(token_service_);
 }
 
 ArcAuthContext::~ArcAuthContext() {
@@ -54,6 +51,12 @@ void ArcAuthContext::Prepare(const PrepareCallback& callback) {
     return;
   }
 
+  const SigninManagerBase* const signin_manager =
+      SigninManagerFactory::GetForProfile(profile_);
+  CHECK(signin_manager);
+  account_id_ = signin_manager->GetAuthenticatedAccountId();
+  DCHECK(!account_id_.empty());
+
   callback_ = callback;
   token_service_->RemoveObserver(this);
   refresh_token_timeout_.Stop();