Respect the new Online Wallet sign-in response.

In addition to handling non-200 response codes, we now
parse the response to see if the sign-in was successful.

See the http://crbug.com/244463#c12 for more details.

BUG=244463
TEST=WalletSigninHelperTest

Review URL: https://chromiumcodereview.appspot.com/16858016

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

3 files changed, 81 insertions, 27 deletions

diff --git a/components/autofill/content/browser/wallet/wallet_signin_helper.cc b/components/autofill/content/browser/wallet/wallet_signin_helper.cc
index 1474a93..9b083b1 100644
--- a/components/autofill/content/browser/wallet/wallet_signin_helper.cc
+++ b/components/autofill/content/browser/wallet/wallet_signin_helper.cc
@@ -8,6 +8,7 @@
 #include "base/json/json_reader.h"
 #include "base/logging.h"
 #include "base/rand_util.h"
+#include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/time.h"
 #include "base/values.h"
@@ -214,15 +215,19 @@ void WalletSigninHelper::OnURLFetchComplete(
       break;
 
     case PASSIVE_EXECUTING_SIGNIN:
-      url_fetcher_.reset();
-      state_ = PASSIVE_FETCHING_USERINFO;
-      StartFetchingUserNameFromSession();
+      if (ParseSignInResponse()) {
+        url_fetcher_.reset();
+        state_ = PASSIVE_FETCHING_USERINFO;
+        StartFetchingUserNameFromSession();
+      }
       break;
 
     case AUTOMATIC_EXECUTING_SIGNIN:
-      state_ = IDLE;
-      url_fetcher_.reset();
-      delegate_->OnAutomaticSigninSuccess(username_);
+      if (ParseSignInResponse()) {
+        url_fetcher_.reset();
+        state_ = IDLE;
+        delegate_->OnAutomaticSigninSuccess(username_);
+      }
       break;
 
     default:
@@ -269,25 +274,47 @@ void WalletSigninHelper::ProcessGetAccountInfoResponseAndFinish() {
   }
 }
 
+bool WalletSigninHelper::ParseSignInResponse() {
+  if (!url_fetcher_) {
+    NOTREACHED();
+    return false;
+  }
+
+  std::string data;
+  if (!url_fetcher_->GetResponseAsString(&data)) {
+    DVLOG(1) << "failed to GetResponseAsString";
+    OnOtherError();
+    return false;
+  }
+
+  if (!LowerCaseEqualsASCII(data, "yes")) {
+    OnServiceError(
+        GoogleServiceAuthError(GoogleServiceAuthError::USER_NOT_SIGNED_UP));
+    return false;
+  }
+
+  return true;
+}
+
 bool WalletSigninHelper::ParseGetAccountInfoResponse(
     const net::URLFetcher* fetcher, std::string* email) {
   DCHECK(email);
 
   std::string data;
   if (!fetcher->GetResponseAsString(&data)) {
-    LOG(ERROR) << "failed to GetResponseAsString";
+    DVLOG(1) << "failed to GetResponseAsString";
     return false;
   }
 
   scoped_ptr<base::Value> value(base::JSONReader::Read(data));
   if (!value.get() || value->GetType() != base::Value::TYPE_DICTIONARY) {
-    LOG(ERROR) << "failed to parse JSON response";
+    DVLOG(1) << "failed to parse JSON response";
     return false;
   }
 
   DictionaryValue* dict = static_cast<DictionaryValue*>(value.get());
   if (!dict->GetStringWithoutPathExpansion("email", email)) {
-    LOG(ERROR) << "no email in JSON response";
+    DVLOG(1) << "no email in JSON response";
     return false;
   }
 
diff --git a/components/autofill/content/browser/wallet/wallet_signin_helper.h b/components/autofill/content/browser/wallet/wallet_signin_helper.h
index 4733dc9..fdcd6f8 100644
--- a/components/autofill/content/browser/wallet/wallet_signin_helper.h
+++ b/components/autofill/content/browser/wallet/wallet_signin_helper.h
@@ -108,6 +108,11 @@ class WalletSigninHelper : public GaiaAuthConsumer,
   // and calls the delegate callbacks on success/failure.
   void ProcessGetAccountInfoResponseAndFinish();
 
+  // Attempts to parse a response from the Online Wallet sign-in.
+  // Returns true if the response is correct and the sign-in has succeeded.
+  // Otherwise, it calls OnServiceError() and returns false.
+  bool ParseSignInResponse();
+
   // Attempts to parse the GetAccountInfo response from the server.
   // Returns true on success; the obtained email address is stored into |email|.
   bool ParseGetAccountInfoResponse(const net::URLFetcher* fetcher,
diff --git a/components/autofill/content/browser/wallet/wallet_signin_helper_unittest.cc b/components/autofill/content/browser/wallet/wallet_signin_helper_unittest.cc
index c768151..942fba1 100644
--- a/components/autofill/content/browser/wallet/wallet_signin_helper_unittest.cc
+++ b/components/autofill/content/browser/wallet/wallet_signin_helper_unittest.cc
@@ -14,6 +14,7 @@
 #include "google_apis/gaia/gaia_constants.h"
 #include "google_apis/gaia/gaia_urls.h"
 #include "google_apis/gaia/google_service_auth_error.h"
+#include "net/http/http_status_code.h"
 #include "net/url_request/test_url_fetcher_factory.h"
 #include "net/url_request/url_request.h"
 #include "net/url_request/url_request_context_getter.h"
@@ -113,7 +114,7 @@ class WalletSigninHelperTest : public testing::Test {
 
   void MockSuccessfulOAuthLoginResponse() {
     SetUpFetcherResponseAndCompleteRequest(
-        GaiaUrls::GetInstance()->client_login_url(), 200,
+        GaiaUrls::GetInstance()->client_login_url(), net::HTTP_OK,
         net::ResponseCookies(),
         "SID=sid\nLSID=lsid\nAuth=auth");
   }
@@ -121,14 +122,14 @@ class WalletSigninHelperTest : public testing::Test {
   void MockFailedOAuthLoginResponse404() {
     SetUpFetcherResponseAndCompleteRequest(
         GaiaUrls::GetInstance()->client_login_url(),
-        404,
+        net::HTTP_NOT_FOUND,
         net::ResponseCookies(),
         std::string());
   }
 
   void MockSuccessfulGaiaUserInfoResponse(const std::string& username) {
     SetUpFetcherResponseAndCompleteRequest(
-        GaiaUrls::GetInstance()->get_user_info_url(), 200,
+        GaiaUrls::GetInstance()->get_user_info_url(), net::HTTP_OK,
         net::ResponseCookies(),
         "email=" + username);
   }
@@ -136,14 +137,14 @@ class WalletSigninHelperTest : public testing::Test {
   void MockFailedGaiaUserInfoResponse404() {
     SetUpFetcherResponseAndCompleteRequest(
         GaiaUrls::GetInstance()->get_user_info_url(),
-        404,
+        net::HTTP_NOT_FOUND,
         net::ResponseCookies(),
         std::string());
   }
 
   void MockSuccessfulGetAccountInfoResponse(const std::string& username) {
     SetUpFetcherResponseAndCompleteRequest(
-        signin_helper_->GetGetAccountInfoUrlForTesting(), 200,
+        signin_helper_->GetGetAccountInfoUrlForTesting(), net::HTTP_OK,
         net::ResponseCookies(),
         base::StringPrintf(
             kGetAccountInfoValidResponseFormat,
@@ -153,21 +154,28 @@ class WalletSigninHelperTest : public testing::Test {
   void MockFailedGetAccountInfoResponse404() {
     SetUpFetcherResponseAndCompleteRequest(
         signin_helper_->GetGetAccountInfoUrlForTesting(),
-        404,
+        net::HTTP_NOT_FOUND,
         net::ResponseCookies(),
         std::string());
   }
 
-  void MockSuccessfulPassiveAuthUrlMergeAndRedirectResponse() {
+  void MockSuccessfulPassiveSignInResponse() {
     SetUpFetcherResponseAndCompleteRequest(wallet::GetPassiveAuthUrl().spec(),
-                                           200,
+                                           net::HTTP_OK,
                                            net::ResponseCookies(),
-                                           std::string());
+                                           "YES");
+  }
+
+  void MockFailedPassiveSignInResponseNo() {
+    SetUpFetcherResponseAndCompleteRequest(wallet::GetPassiveAuthUrl().spec(),
+                                           net::HTTP_OK,
+                                           net::ResponseCookies(),
+                                           "NOOOOOOOOOOOOOOO");
   }
 
-  void MockFailedPassiveAuthUrlMergeAndRedirectResponse404() {
+  void MockFailedPassiveSignInResponse404() {
     SetUpFetcherResponseAndCompleteRequest(wallet::GetPassiveAuthUrl().spec(),
-                                           404,
+                                           net::HTTP_NOT_FOUND,
                                            net::ResponseCookies(),
                                            std::string());
   }
@@ -189,20 +197,26 @@ class WalletSigninHelperTest : public testing::Test {
 TEST_F(WalletSigninHelperTest, PassiveSigninSuccessful) {
   EXPECT_CALL(mock_delegate_, OnPassiveSigninSuccess("user@gmail.com"));
   signin_helper_->StartPassiveSignin();
-  MockSuccessfulPassiveAuthUrlMergeAndRedirectResponse();
+  MockSuccessfulPassiveSignInResponse();
   MockSuccessfulGetAccountInfoResponse("user@gmail.com");
 }
 
-TEST_F(WalletSigninHelperTest, PassiveSigninFailedSignin) {
+TEST_F(WalletSigninHelperTest, PassiveSigninFailedSignin404) {
   EXPECT_CALL(mock_delegate_, OnPassiveSigninFailure(_));
   signin_helper_->StartPassiveSignin();
-  MockFailedPassiveAuthUrlMergeAndRedirectResponse404();
+  MockFailedPassiveSignInResponse404();
+}
+
+TEST_F(WalletSigninHelperTest, PassiveSigninFailedSigninNo) {
+  EXPECT_CALL(mock_delegate_, OnPassiveSigninFailure(_));
+  signin_helper_->StartPassiveSignin();
+  MockFailedPassiveSignInResponseNo();
 }
 
 TEST_F(WalletSigninHelperTest, PassiveSigninFailedUserInfo) {
   EXPECT_CALL(mock_delegate_, OnPassiveSigninFailure(_));
   signin_helper_->StartPassiveSignin();
-  MockSuccessfulPassiveAuthUrlMergeAndRedirectResponse();
+  MockSuccessfulPassiveSignInResponse();
   MockFailedGetAccountInfoResponse404();
 }
 
@@ -223,7 +237,7 @@ TEST_F(WalletSigninHelperTest, AutomaticSigninSuccessful) {
   signin_helper_->StartAutomaticSignin("123SID", "123LSID");
   MockSuccessfulGaiaUserInfoResponse("user@gmail.com");
   MockSuccessfulOAuthLoginResponse();
-  MockSuccessfulPassiveAuthUrlMergeAndRedirectResponse();
+  MockSuccessfulPassiveSignInResponse();
 }
 
 TEST_F(WalletSigninHelperTest, AutomaticSigninFailedGetUserInfo) {
@@ -239,12 +253,20 @@ TEST_F(WalletSigninHelperTest, AutomaticSigninFailedOAuthLogin) {
   MockFailedOAuthLoginResponse404();
 }
 
-TEST_F(WalletSigninHelperTest, AutomaticSigninFailedSignin) {
+TEST_F(WalletSigninHelperTest, AutomaticSigninFailedSignin404) {
+  EXPECT_CALL(mock_delegate_, OnAutomaticSigninFailure(_));
+  signin_helper_->StartAutomaticSignin("123SID", "123LSID");
+  MockSuccessfulGaiaUserInfoResponse("user@gmail.com");
+  MockSuccessfulOAuthLoginResponse();
+  MockFailedPassiveSignInResponse404();
+}
+
+TEST_F(WalletSigninHelperTest, AutomaticSigninFailedSigninNo) {
   EXPECT_CALL(mock_delegate_, OnAutomaticSigninFailure(_));
   signin_helper_->StartAutomaticSignin("123SID", "123LSID");
   MockSuccessfulGaiaUserInfoResponse("user@gmail.com");
   MockSuccessfulOAuthLoginResponse();
-  MockFailedPassiveAuthUrlMergeAndRedirectResponse404();
+  MockFailedPassiveSignInResponseNo();
 }
 
 // TODO(aruslan): http://crbug.com/188317 Need more tests.