Add 3 more registartion status codes for UMA collection. Also added tests.

BUG=284553

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=252218

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

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

3 files changed, 81 insertions, 27 deletions

diff --git a/google_apis/gcm/engine/registration_request.cc b/google_apis/gcm/engine/registration_request.cc
index f0de831..af405ab 100644
--- a/google_apis/gcm/engine/registration_request.cc
+++ b/google_apis/gcm/engine/registration_request.cc
@@ -70,7 +70,10 @@ RegistrationRequest::Status GetStatusFromError(const std::string& error) {
 bool ShouldRetryWithStatus(RegistrationRequest::Status status) {
   return status == RegistrationRequest::UNKNOWN_ERROR ||
          status == RegistrationRequest::AUTHENTICATION_FAILED ||
-         status == RegistrationRequest::DEVICE_REGISTRATION_ERROR;
+         status == RegistrationRequest::DEVICE_REGISTRATION_ERROR ||
+         status == RegistrationRequest::HTTP_NOT_OK ||
+         status == RegistrationRequest::URL_FETCHING_FAILED ||
+         status == RegistrationRequest::RESPONSE_PARSING_FAILED;
 }
 
 void RecordRegistrationStatusToUMA(RegistrationRequest::Status status) {
@@ -175,43 +178,45 @@ void RegistrationRequest::RetryWithBackoff(bool update_backoff) {
   Start();
 }
 
-void RegistrationRequest::OnURLFetchComplete(const net::URLFetcher* source) {
-  std::string response;
-  if (!source->GetStatus().is_success() ||
-      source->GetResponseCode() != net::HTTP_OK ||
-      !source->GetResponseAsString(&response)) {
-    LOG(ERROR) << "Failed to get registration response: "
-               << source->GetStatus().is_success() << " "
+RegistrationRequest::Status RegistrationRequest::ParseResponse(
+    const net::URLFetcher* source, std::string* token) {
+  if (!source->GetStatus().is_success()) {
+    LOG(ERROR) << "URL fetching failed.";
+    return URL_FETCHING_FAILED;
+  }
+  if (source->GetResponseCode() != net::HTTP_OK) {
+    LOG(ERROR) << "URL fetching HTTP response code is not OK. It is "
                << source->GetResponseCode();
-    RetryWithBackoff(true);
-    return;
+    return HTTP_NOT_OK;
+  }
+  std::string response;
+  if (!source->GetResponseAsString(&response)) {
+    LOG(ERROR) << "Failed to parse registration response as a string.";
+    return RESPONSE_PARSING_FAILED;
   }
 
   DVLOG(1) << "Parsing registration response: " << response;
   size_t token_pos = response.find(kTokenPrefix);
   if (token_pos != std::string::npos) {
-    std::string token =
-        response.substr(token_pos + arraysize(kTokenPrefix) - 1);
-    RecordRegistrationStatusToUMA(SUCCESS);
-    callback_.Run(SUCCESS, token);
-    return;
+    *token = response.substr(token_pos + arraysize(kTokenPrefix) - 1);
+    return SUCCESS;
   }
 
   size_t error_pos = response.find(kErrorPrefix);
-  Status status = UNKNOWN_ERROR;
-  if (error_pos != std::string::npos) {
-    std::string error =
-        response.substr(error_pos + arraysize(kErrorPrefix) - 1);
-    status = GetStatusFromError(error);
-  }
-  RecordRegistrationStatusToUMA(status);
+  if (error_pos == std::string::npos)
+    return UNKNOWN_ERROR;
+  std::string error = response.substr(error_pos + arraysize(kErrorPrefix) - 1);
+  return GetStatusFromError(error);
+}
 
-  if (ShouldRetryWithStatus(status)) {
+void RegistrationRequest::OnURLFetchComplete(const net::URLFetcher* source) {
+  std::string token;
+  Status status = ParseResponse(source, &token);
+  RecordRegistrationStatusToUMA(status );
+  if (ShouldRetryWithStatus(status))
     RetryWithBackoff(true);
-    return;
-  }
-
-  callback_.Run(status, std::string());
+  else
+    callback_.Run(status, token);
 }
 
 }  // namespace gcm
diff --git a/google_apis/gcm/engine/registration_request.h b/google_apis/gcm/engine/registration_request.h
index 2936350..40334d0 100644
--- a/google_apis/gcm/engine/registration_request.h
+++ b/google_apis/gcm/engine/registration_request.h
@@ -40,6 +40,9 @@ class GCM_EXPORT RegistrationRequest : public net::URLFetcherDelegate {
     AUTHENTICATION_FAILED,      // Authentication failed.
     DEVICE_REGISTRATION_ERROR,  // Chrome is not properly registered.
     UNKNOWN_ERROR,              // Unknown error.
+    URL_FETCHING_FAILED,        // URL fetching failed.
+    HTTP_NOT_OK,                // HTTP status was not OK.
+    RESPONSE_PARSING_FAILED,    // Registration response parsing failed.
     // NOTE: always keep this entry at the end. Add new status types only
     // immediately above this line. Make sure to update the corresponding
     // histogram enum accordingly.
@@ -91,6 +94,10 @@ class GCM_EXPORT RegistrationRequest : public net::URLFetcherDelegate {
   // failure, when |update_backoff| is true.
   void RetryWithBackoff(bool update_backoff);
 
+  // Parse the response returned by the URL fetcher into token, and returns the
+  // status.
+  Status ParseResponse(const net::URLFetcher* source, std::string* token);
+
   RegistrationCallback callback_;
   RequestInfo request_info_;
 
diff --git a/google_apis/gcm/engine/registration_request_unittest.cc b/google_apis/gcm/engine/registration_request_unittest.cc
index 178c135..62b8717 100644
--- a/google_apis/gcm/engine/registration_request_unittest.cc
+++ b/google_apis/gcm/engine/registration_request_unittest.cc
@@ -10,6 +10,7 @@
 #include "base/strings/string_tokenizer.h"
 #include "google_apis/gcm/engine/registration_request.h"
 #include "net/url_request/test_url_fetcher_factory.h"
+#include "net/url_request/url_request_status.h"
 #include "net/url_request/url_request_test_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -311,4 +312,45 @@ TEST_F(RegistrationRequestTest, ResponseInvalidSender) {
   EXPECT_EQ(std::string(), registration_id_);
 }
 
+TEST_F(RegistrationRequestTest, RequestNotSucessful) {
+  CreateRequest("sender1,sender2");
+  request_->Start();
+
+  net::URLRequestStatus request_status(net::URLRequestStatus::FAILED, 1);
+  SetResponseStatusAndString(net::HTTP_OK, "token=2501");
+  net::TestURLFetcher* fetcher = url_fetcher_factory_.GetFetcherByID(0);
+  ASSERT_TRUE(fetcher);
+  fetcher->set_status(request_status);
+
+  CompleteFetch();
+
+  EXPECT_FALSE(callback_called_);
+
+  // Ensuring a retry happened and succeeded.
+  SetResponseStatusAndString(net::HTTP_OK, "token=2501");
+  CompleteFetch();
+
+  EXPECT_TRUE(callback_called_);
+  EXPECT_EQ(RegistrationRequest::SUCCESS, status_);
+  EXPECT_EQ("2501", registration_id_);
+}
+
+TEST_F(RegistrationRequestTest, ResponseHttpNotOk) {
+  CreateRequest("sender1,sender2");
+  request_->Start();
+
+  SetResponseStatusAndString(net::HTTP_GATEWAY_TIMEOUT, "token=2501");
+  CompleteFetch();
+
+  EXPECT_FALSE(callback_called_);
+
+  // Ensuring a retry happened and succeeded.
+  SetResponseStatusAndString(net::HTTP_OK, "token=2501");
+  CompleteFetch();
+
+  EXPECT_TRUE(callback_called_);
+  EXPECT_EQ(RegistrationRequest::SUCCESS, status_);
+  EXPECT_EQ("2501", registration_id_);
+}
+
 }  // namespace gcm