Added secondary /MergeSession verification step.

We will now inspect outcome of /ListAccounts call to check if the primary
session SID/LSID cookies got run over by browser session restore process.

For now, the outcome of this test is just reported as a UMA histogram.

BUG=309131
TEST=none

Review URL: https://codereview.chromium.org/103463003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@239385 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/chromeos/login/oauth2_login_manager.cc

@@ -24,10 +24,14 @@
 
 namespace chromeos {
 
+namespace {
+
 static const char kServiceScopeGetUserInfo[] =
     "https://www.googleapis.com/auth/userinfo.email";
 static const int kMaxRetries = 5;
 
+}  // namespace
+
 OAuth2LoginManager::OAuth2LoginManager(Profile* user_profile)
     : user_profile_(user_profile),
       restore_strategy_(RESTORE_FROM_COOKIE_JAR),
@@ -266,6 +270,51 @@ void OAuth2LoginManager::OnSessionMergeFailure(bool connection_error) {
       OAuth2LoginManager::SESSION_RESTORE_FAILED);
 }
 
+void OAuth2LoginManager::OnListAccountsSuccess(const std::string& data) {
+  PostMergeVerificationOutcome outcome = POST_MERGE_SUCCESS;
+  // Let's analyze which accounts we see logged in here:
+  std::vector<std::string> accounts = gaia::ParseListAccountsData(data);
+  std::string user_email = gaia::CanonicalizeEmail(
+      GetTokenService()->GetPrimaryAccountId());
+  if (!accounts.empty()) {
+    bool found = false;
+    bool first = true;
+    for (std::vector<std::string>::const_iterator iter = accounts.begin();
+         iter != accounts.end(); ++iter) {
+      if (gaia::CanonicalizeEmail(*iter) == user_email) {
+        found = true;
+        break;
+      }
+
+      first = false;
+    }
+
+    if (!found)
+      outcome = POST_MERGE_MISSING_PRIMARY_ACCOUNT;
+    else if (!first)
+      outcome = POST_MERGE_PRIMARY_NOT_FIRST_ACCOUNT;
+
+  } else {
+    outcome = POST_MERGE_NO_ACCOUNTS;
+  }
+
+  RecordPostMergeOutcome(outcome);
+}
+
+void OAuth2LoginManager::OnListAccountsFailure(bool connection_error) {
+  RecordPostMergeOutcome(connection_error ? POST_MERGE_CONNECTION_FAILED :
+                                            POST_MERGE_VERIFICATION_FAILED);
+}
+
+// static
+void OAuth2LoginManager::RecordPostMergeOutcome(
+    PostMergeVerificationOutcome outcome) {
+  UMA_HISTOGRAM_ENUMERATION("OAuth2Login.PostMergeVerification",
+                            outcome,
+                            POST_MERGE_COUNT);
+}
+
+
 void OAuth2LoginManager::SetSessionRestoreState(
     OAuth2LoginManager::SessionRestoreState state) {
   if (state_ == state)

chrome/browser/chromeos/login/oauth2_login_manager.h

@@ -124,6 +124,20 @@ class OAuth2LoginManager : public BrowserContextKeyedService,
     SESSION_RESTORE_COUNT = SESSION_RESTORE_MERGE_SESSION_FAILED,
   };
 
+  // Outcomes of post-merge session verification.
+  // This enum is used for an UMA histogram, and hence new items should only be
+  // appended at the end.
+  enum PostMergeVerificationOutcome {
+    POST_MERGE_UNDEFINED  = 0,
+    POST_MERGE_SUCCESS = 1,
+    POST_MERGE_NO_ACCOUNTS = 2,
+    POST_MERGE_MISSING_PRIMARY_ACCOUNT = 3,
+    POST_MERGE_PRIMARY_NOT_FIRST_ACCOUNT = 4,
+    POST_MERGE_VERIFICATION_FAILED = 5,
+    POST_MERGE_CONNECTION_FAILED = 6,
+    POST_MERGE_COUNT = 7,
+  };
+
   // BrowserContextKeyedService implementation.
   virtual void Shutdown() OVERRIDE;
 
@@ -140,6 +154,8 @@ class OAuth2LoginManager : public BrowserContextKeyedService,
   virtual void OnOAuthLoginFailure(bool connection_error) OVERRIDE;
   virtual void OnSessionMergeSuccess() OVERRIDE;
   virtual void OnSessionMergeFailure(bool connection_error) OVERRIDE;
+  virtual void OnListAccountsSuccess(const std::string& data) OVERRIDE;
+  virtual void OnListAccountsFailure(bool connection_error) OVERRIDE;
 
   // OAuth2TokenFetcher::Delegate overrides.
   virtual void OnOAuth2TokensAvailable(
@@ -185,6 +201,9 @@ class OAuth2LoginManager : public BrowserContextKeyedService,
   // Testing helper.
   void SetSessionRestoreStartForTesting(const base::Time& time);
 
+  // Records |outcome| of post merge verification check.
+  static void RecordPostMergeOutcome(PostMergeVerificationOutcome outcome);
+
   // Keeps the track if we have already reported OAuth2 token being loaded
   // by OAuth2TokenService.
   Profile* user_profile_;

chrome/browser/chromeos/login/oauth2_login_verifier.cc

@@ -28,9 +28,17 @@ namespace {
 
 // OAuth token request max retry count.
 const int kMaxRequestAttemptCount = 5;
+
 // OAuth token request retry delay in milliseconds.
 const int kRequestRestartDelay = 3000;
 
+// Post merge session verification delay.
+#ifndef NDEBUG
+const int kPostResoreVerificationDelay = 1000;
+#else
+const int kPostResoreVerificationDelay = 1000*60*3;
+#endif
+
 bool IsConnectionOrServiceError(const GoogleServiceAuthError& error) {
   return error.state() == GoogleServiceAuthError::CONNECTION_FAILED ||
          error.state() == GoogleServiceAuthError::SERVICE_UNAVAILABLE ||
@@ -168,6 +176,27 @@ void OAuth2LoginVerifier::OnMergeSessionSuccess(const std::string& data) {
   delegate_->OnSessionMergeSuccess();
   // Get GAIA credentials needed to kick off OAuth2TokenService and friends.
   StartOAuthLoginForGaiaCredentials();
+
+  // Schedule post-merge verification to analyze how many LSID/SID overruns
+  // were created by the session restore.
+  SchedulePostMergeVerification();
+}
+
+void OAuth2LoginVerifier::SchedulePostMergeVerification() {
+  BrowserThread::PostDelayedTask(
+      BrowserThread::UI,
+      FROM_HERE,
+      base::Bind(
+          &OAuth2LoginVerifier::StartPostRestoreVerification, AsWeakPtr()),
+      base::TimeDelta::FromMilliseconds(kPostResoreVerificationDelay));
+}
+
+void OAuth2LoginVerifier::StartPostRestoreVerification() {
+  gaia_fetcher_.reset(
+      new GaiaAuthFetcher(this,
+                          std::string(GaiaConstants::kChromeOSSource),
+                          user_request_context_.get()));
+  gaia_fetcher_->StartListAccounts();
 }
 
 void OAuth2LoginVerifier::OnMergeSessionFailure(
@@ -213,6 +242,27 @@ void OAuth2LoginVerifier::OnGetTokenFailure(
   delegate_->OnOAuthLoginFailure(IsConnectionOrServiceError(error));
 }
 
+void OAuth2LoginVerifier::OnListAccountsSuccess(
+    const std::string& data) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  VLOG(1) << "ListAccounts successful.";
+  delegate_->OnListAccountsSuccess(data);
+}
+
+void OAuth2LoginVerifier::OnListAccountsFailure(
+    const GoogleServiceAuthError& error) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  LOG(WARNING) << "Failed to get list of session accounts, "
+               << " error: " << error.state();
+  RetryOnError(
+      "ListAccounts",
+      error,
+      base::Bind(&OAuth2LoginVerifier::StartPostRestoreVerification,
+                 AsWeakPtr()),
+      base::Bind(&Delegate::OnListAccountsFailure,
+                 base::Unretained(delegate_)));
+}
+
 void OAuth2LoginVerifier::RetryOnError(const char* operation_id,
                                        const GoogleServiceAuthError& error,
                                        const base::Closure& task_to_retry,

chrome/browser/chromeos/login/oauth2_login_verifier.h

@@ -33,15 +33,26 @@ class OAuth2LoginVerifier : public base::SupportsWeakPtr<OAuth2LoginVerifier>,
   class Delegate {
    public:
     virtual ~Delegate() {}
+
     // Invoked during exchange of OAuth2 refresh token for GAIA service token.
     virtual void OnOAuthLoginSuccess(
         const ClientLoginResult& gaia_credentials) = 0;
+
     // Invoked when provided OAuth2 refresh token is invalid.
     virtual void OnOAuthLoginFailure(bool connection_error) = 0;
+
     // Invoked when cookie session is successfully merged.
     virtual void OnSessionMergeSuccess() = 0;
+
     // Invoked when cookie session can not be merged.
     virtual void OnSessionMergeFailure(bool connection_error) = 0;
+
+    // Invoked when account list is retrieved during post-merge session
+    // verification.
+    virtual void OnListAccountsSuccess(const std::string& data) = 0;
+
+    // Invoked when post-merge session verification fails.
+    virtual void OnListAccountsFailure(bool connection_error) = 0;
   };
 
   OAuth2LoginVerifier(OAuth2LoginVerifier::Delegate* delegate,
@@ -69,6 +80,9 @@ class OAuth2LoginVerifier : public base::SupportsWeakPtr<OAuth2LoginVerifier>,
   virtual void OnMergeSessionSuccess(const std::string& data) OVERRIDE;
   virtual void OnMergeSessionFailure(
       const GoogleServiceAuthError& error) OVERRIDE;
+  virtual void OnListAccountsSuccess(const std::string& data) OVERRIDE;
+  virtual void OnListAccountsFailure(
+      const GoogleServiceAuthError& error) OVERRIDE;
 
   // OAuth2TokenService::Consumer overrides.
   virtual void OnGetTokenSuccess(const OAuth2TokenService::Request* request,
@@ -89,6 +103,13 @@ class OAuth2LoginVerifier : public base::SupportsWeakPtr<OAuth2LoginVerifier>,
   // Attempts to merge session from present |gaia_token_|.
   void StartMergeSession();
 
+  // Schedules post merge verification to ensure that browser session restore
+  // hasn't stumped over SID/LSID.
+  void SchedulePostMergeVerification();
+
+  // Starts post merge request verification.
+  void StartPostRestoreVerification();
+
   // Decides how to proceed on GAIA |error|. If the error looks temporary,
   // retries |task| after certain delay until max retry count is reached.
   void RetryOnError(const char* operation_id,

chrome/browser/signin/account_reconcilor.cc

@@ -2,7 +2,6 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "base/json/json_reader.h"
 #include "base/logging.h"
 #include "base/time/time.h"
 #include "chrome/browser/chrome_notification_types.h"
@@ -18,6 +17,7 @@
 #include "content/public/browser/notification_details.h"
 #include "content/public/browser/notification_source.h"
 #include "google_apis/gaia/gaia_auth_fetcher.h"
+#include "google_apis/gaia/gaia_auth_util.h"
 #include "google_apis/gaia/gaia_constants.h"
 
 AccountReconcilor::AccountReconcilor(Profile* profile)
@@ -196,7 +196,7 @@ void AccountReconcilor::OnListAccountsSuccess(const std::string& data) {
   gaia_fetcher_.reset();
 
   // Get account information from response data.
-  gaia_accounts_ = ParseListAccountsData(data);
+  gaia_accounts_ = gaia::ParseListAccountsData(data);
   if (gaia_accounts_.size() > 0) {
     DVLOG(1) << "AccountReconcilor::OnListAccountsSuccess: "
              << "Gaia " << gaia_accounts_.size() << " accounts, "
@@ -209,39 +209,6 @@ void AccountReconcilor::OnListAccountsSuccess(const std::string& data) {
   FinishReconcileAction();
 }
 
-// static
-std::vector<std::string> AccountReconcilor::ParseListAccountsData(
-    const std::string& data) {
-  std::vector<std::string> account_ids;
-
-  // Parse returned data and make sure we have data.
-  scoped_ptr<base::Value> value(base::JSONReader::Read(data));
-  if (!value)
-    return account_ids;
-
-  base::ListValue* list;
-  if (!value->GetAsList(&list) || list->GetSize() < 2)
-    return account_ids;
-
-  // Get list of account info.
-  base::ListValue* accounts;
-  if (!list->GetList(1, &accounts) || accounts == NULL)
-    return account_ids;
-
-  // Build a vector of accounts from the cookie.  Order is important: the first
-  // account in the list is the primary account.
-  for (size_t i = 0; i < accounts->GetSize(); ++i) {
-    base::ListValue* account;
-    if (accounts->GetList(i, &account) && account != NULL) {
-      std::string email;
-      if (account->GetString(3, &email) && !email.empty())
-        account_ids.push_back(email);
-    }
-  }
-
-  return account_ids;
-}
-
 void AccountReconcilor::OnListAccountsFailure(
     const GoogleServiceAuthError& error) {
   gaia_fetcher_.reset();

chrome/browser/signin/account_reconcilor.h

@@ -37,7 +37,6 @@ class AccountReconcilor : public BrowserContextKeyedService,
 
  private:
   class AccountReconcilorTest;
-  FRIEND_TEST_ALL_PREFIXES(AccountReconcilorTest, ParseListAccountsData);
 
   // Register and unregister with dependent services.
   void RegisterWithCookieMonster();
@@ -51,9 +50,6 @@ class AccountReconcilor : public BrowserContextKeyedService,
 
   void DeleteAccessTokenRequests();
 
-  static std::vector<std::string> ParseListAccountsData(
-      const std::string& data);
-
   // Start and stop the periodic reconciliation.
   void StartPeriodicReconciliation();
   void StopPeriodicReconciliation();

chrome/browser/signin/account_reconcilor_unittest.cc

@@ -102,36 +102,3 @@ TEST_F(AccountReconcilorTest, ProfileAlreadyConnected) {
   ASSERT_TRUE(reconcilor->IsPeriodicReconciliationRunning());
 }
 
-TEST_F(AccountReconcilorTest, ParseListAccountsData) {
-  std::vector<std::string> accounts;
-  accounts = AccountReconcilor::ParseListAccountsData("");
-  ASSERT_EQ(0u, accounts.size());
-
-  accounts = AccountReconcilor::ParseListAccountsData("1");
-  ASSERT_EQ(0u, accounts.size());
-
-  accounts = AccountReconcilor::ParseListAccountsData("[]");
-  ASSERT_EQ(0u, accounts.size());
-
-  accounts = AccountReconcilor::ParseListAccountsData("[\"foo\", \"bar\"]");
-  ASSERT_EQ(0u, accounts.size());
-
-  accounts = AccountReconcilor::ParseListAccountsData("[\"foo\", []]");
-  ASSERT_EQ(0u, accounts.size());
-
-  accounts = AccountReconcilor::ParseListAccountsData(
-      "[\"foo\", [[\"bar\", 0, \"name\", 0, \"photo\", 0, 0, 0]]]");
-  ASSERT_EQ(0u, accounts.size());
-
-  accounts = AccountReconcilor::ParseListAccountsData(
-      "[\"foo\", [[\"bar\", 0, \"name\", \"email\", \"photo\", 0, 0, 0]]]");
-  ASSERT_EQ(1u, accounts.size());
-  ASSERT_EQ("email", accounts[0]);
-
-  accounts = AccountReconcilor::ParseListAccountsData(
-      "[\"foo\", [[\"bar1\", 0, \"name1\", \"email1\", \"photo1\", 0, 0, 0], "
-                 "[\"bar2\", 0, \"name2\", \"email2\", \"photo2\", 0, 0, 0]]]");
-  ASSERT_EQ(2u, accounts.size());
-  ASSERT_EQ("email1", accounts[0]);
-  ASSERT_EQ("email2", accounts[1]);
-}