summaryrefslogtreecommitdiffstats
path: root/google_apis
diff options
context:
space:
mode:
Diffstat (limited to 'google_apis')
-rw-r--r--google_apis/gcm/engine/checkin_request.cc62
-rw-r--r--google_apis/gcm/engine/checkin_request.h10
-rw-r--r--google_apis/gcm/engine/checkin_request_unittest.cc137
-rw-r--r--google_apis/gcm/gcm_client_impl.cc4
-rw-r--r--google_apis/gcm/tools/mcs_probe.cc1
5 files changed, 187 insertions, 27 deletions
diff --git a/google_apis/gcm/engine/checkin_request.cc b/google_apis/gcm/engine/checkin_request.cc
index 9f3159a..b46415a 100644
--- a/google_apis/gcm/engine/checkin_request.cc
+++ b/google_apis/gcm/engine/checkin_request.cc
@@ -22,6 +22,7 @@ const int kRequestVersionValue = 2;
CheckinRequest::CheckinRequest(
const CheckinRequestCallback& callback,
+ const net::BackoffEntry::Policy& backoff_policy,
const checkin_proto::ChromeBuildProto& chrome_build_proto,
int64 user_serial_number,
uint64 android_id,
@@ -29,10 +30,13 @@ CheckinRequest::CheckinRequest(
net::URLRequestContextGetter* request_context_getter)
: request_context_getter_(request_context_getter),
callback_(callback),
+ backoff_entry_(&backoff_policy),
chrome_build_proto_(chrome_build_proto),
android_id_(android_id),
security_token_(security_token),
- user_serial_number_(user_serial_number) {}
+ user_serial_number_(user_serial_number),
+ weak_ptr_factory_(this) {
+}
CheckinRequest::~CheckinRequest() {}
@@ -53,7 +57,6 @@ void CheckinRequest::Start() {
checkin->set_type(checkin_proto::DEVICE_CHROME_BROWSER);
#endif
-
std::string upload_data;
CHECK(request.SerializeToString(&upload_data));
@@ -64,18 +67,55 @@ void CheckinRequest::Start() {
url_fetcher_->Start();
}
+void CheckinRequest::RetryWithBackoff(bool update_backoff) {
+ if (update_backoff) {
+ backoff_entry_.InformOfRequest(false);
+ url_fetcher_.reset();
+ }
+
+ if (backoff_entry_.ShouldRejectRequest()) {
+ DVLOG(1) << "Delay GCM checkin for: "
+ << backoff_entry_.GetTimeUntilRelease().InMilliseconds()
+ << " milliseconds.";
+ base::MessageLoop::current()->PostDelayedTask(
+ FROM_HERE,
+ base::Bind(&CheckinRequest::RetryWithBackoff,
+ weak_ptr_factory_.GetWeakPtr(),
+ false),
+ backoff_entry_.GetTimeUntilRelease());
+ return;
+ }
+
+ Start();
+}
+
void CheckinRequest::OnURLFetchComplete(const net::URLFetcher* source) {
std::string response_string;
checkin_proto::AndroidCheckinResponse response_proto;
- if (!source->GetStatus().is_success() ||
- source->GetResponseCode() != net::HTTP_OK ||
+ if (!source->GetStatus().is_success()) {
+ LOG(ERROR) << "Failed to get checkin response. Fetcher failed. Retrying.";
+ RetryWithBackoff(true);
+ return;
+ }
+
+ net::HttpStatusCode response_status = static_cast<net::HttpStatusCode>(
+ source->GetResponseCode());
+ if (response_status == net::HTTP_BAD_REQUEST ||
+ response_status == net::HTTP_UNAUTHORIZED) {
+ // BAD_REQUEST indicates that the request was malformed.
+ // UNAUTHORIZED indicates that security token didn't match the android id.
+ LOG(ERROR) << "No point retrying the checkin with status: "
+ << response_status << ". Checkin failed.";
+ callback_.Run(0,0);
+ return;
+ }
+
+ if (response_status != net::HTTP_OK ||
!source->GetResponseAsString(&response_string) ||
!response_proto.ParseFromString(response_string)) {
- LOG(ERROR) << "Failed to get checkin response: "
- << source->GetStatus().is_success() << " "
- << source->GetResponseCode();
- // TODO(fgorski): Handle retry logic for certain responses.
- callback_.Run(0, 0);
+ LOG(ERROR) << "Failed to get checkin response. HTTP Status: "
+ << response_status << ". Retrying.";
+ RetryWithBackoff(true);
return;
}
@@ -83,8 +123,8 @@ void CheckinRequest::OnURLFetchComplete(const net::URLFetcher* source) {
!response_proto.has_security_token() ||
response_proto.android_id() == 0 ||
response_proto.security_token() == 0) {
- LOG(ERROR) << "Badly formatted checkin response.";
- callback_.Run(0, 0);
+ LOG(ERROR) << "Android ID or security token is 0. Retrying.";
+ RetryWithBackoff(true);
return;
}
diff --git a/google_apis/gcm/engine/checkin_request.h b/google_apis/gcm/engine/checkin_request.h
index 8ea6ad9..548f0b8 100644
--- a/google_apis/gcm/engine/checkin_request.h
+++ b/google_apis/gcm/engine/checkin_request.h
@@ -7,8 +7,10 @@
#include "base/basictypes.h"
#include "base/callback.h"
+#include "base/memory/weak_ptr.h"
#include "google_apis/gcm/base/gcm_export.h"
#include "google_apis/gcm/protocol/android_checkin.pb.h"
+#include "net/base/backoff_entry.h"
#include "net/url_request/url_fetcher_delegate.h"
namespace net {
@@ -29,6 +31,7 @@ class GCM_EXPORT CheckinRequest : public net::URLFetcherDelegate {
CheckinRequestCallback;
CheckinRequest(const CheckinRequestCallback& callback,
+ const net::BackoffEntry::Policy& backoff_policy,
const checkin_proto::ChromeBuildProto& chrome_build_proto,
int64 user_serial_number,
uint64 android_id,
@@ -42,15 +45,22 @@ class GCM_EXPORT CheckinRequest : public net::URLFetcherDelegate {
virtual void OnURLFetchComplete(const net::URLFetcher* source) OVERRIDE;
private:
+ // Schedules a retry attempt, informs the backoff of a previous request's
+ // failure when |update_backoff| is true.
+ void RetryWithBackoff(bool update_backoff);
+
net::URLRequestContextGetter* request_context_getter_;
CheckinRequestCallback callback_;
+ net::BackoffEntry backoff_entry_;
scoped_ptr<net::URLFetcher> url_fetcher_;
checkin_proto::ChromeBuildProto chrome_build_proto_;
uint64 android_id_;
uint64 security_token_;
int64 user_serial_number_;
+ base::WeakPtrFactory<CheckinRequest> weak_ptr_factory_;
+
DISALLOW_COPY_AND_ASSIGN(CheckinRequest);
};
diff --git a/google_apis/gcm/engine/checkin_request_unittest.cc b/google_apis/gcm/engine/checkin_request_unittest.cc
index 1a42356..b618bbf 100644
--- a/google_apis/gcm/engine/checkin_request_unittest.cc
+++ b/google_apis/gcm/engine/checkin_request_unittest.cc
@@ -6,12 +6,45 @@
#include "google_apis/gcm/engine/checkin_request.h"
#include "google_apis/gcm/protocol/checkin.pb.h"
+#include "net/base/backoff_entry.h"
#include "net/url_request/test_url_fetcher_factory.h"
#include "net/url_request/url_request_test_util.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace gcm {
+namespace {
+
+const net::BackoffEntry::Policy kDefaultBackoffPolicy = {
+ // Number of initial errors (in sequence) to ignore before applying
+ // exponential back-off rules.
+ // Explicitly set to 1 to skip the delay of the first Retry, as we are not
+ // trying to test the backoff itself, but rather the fact that retry happens.
+ 1,
+
+ // Initial delay for exponential back-off in ms.
+ 15000, // 15 seconds.
+
+ // Factor by which the waiting time will be multiplied.
+ 2,
+
+ // Fuzzing percentage. ex: 10% will spread requests randomly
+ // between 90%-100% of the calculated time.
+ 0.5, // 50%.
+
+ // Maximum amount of time we are willing to delay our request in ms.
+ 1000 * 60 * 5, // 5 minutes.
+
+ // Time to keep an entry from being discarded even when it
+ // has no significant state, -1 to never discard.
+ -1,
+
+ // Don't use initial delay unless the last request was an error.
+ false,
+};
+
+}
+
const uint64 kAndroidId = 42UL;
const uint64 kBlankAndroidId = 999999UL;
const uint64 kBlankSecurityToken = 999999UL;
@@ -45,6 +78,7 @@ class CheckinRequestTest : public testing::Test {
void SetResponse(ResponseScenario response_scenario);
protected:
+ bool callback_called_;
uint64 android_id_;
uint64 security_token_;
int checkin_device_type_;
@@ -56,7 +90,8 @@ class CheckinRequestTest : public testing::Test {
};
CheckinRequestTest::CheckinRequestTest()
- : android_id_(kBlankAndroidId),
+ : callback_called_(false),
+ android_id_(kBlankAndroidId),
security_token_(kBlankSecurityToken),
checkin_device_type_(0),
url_request_context_getter_(new net::TestURLRequestContextGetter(
@@ -66,6 +101,7 @@ CheckinRequestTest::~CheckinRequestTest() {}
void CheckinRequestTest::FetcherCallback(uint64 android_id,
uint64 security_token) {
+ callback_called_ = true;
android_id_ = android_id;
security_token_ = security_token;
}
@@ -83,6 +119,7 @@ void CheckinRequestTest::CreateRequest(uint64 android_id,
request_.reset(new CheckinRequest(
base::Bind(&CheckinRequestTest::FetcherCallback,
base::Unretained(this)),
+ kDefaultBackoffPolicy,
chrome_build_proto_,
kUserSerialNumber,
android_id,
@@ -91,6 +128,7 @@ void CheckinRequestTest::CreateRequest(uint64 android_id,
// Setting android_id_ and security_token_ to blank value, not used elsewhere
// in the tests.
+ callback_called_ = false;
android_id_ = kBlankAndroidId;
security_token_ = kBlankSecurityToken;
}
@@ -98,14 +136,16 @@ void CheckinRequestTest::CreateRequest(uint64 android_id,
void CheckinRequestTest::SetResponseStatusAndString(
net::HttpStatusCode status_code,
const std::string& response_data) {
- net::TestURLFetcher* fetcher = url_fetcher_factory_.GetFetcherByID(0);
+ net::TestURLFetcher* fetcher =
+ url_fetcher_factory_.GetFetcherByID(0);
ASSERT_TRUE(fetcher);
fetcher->set_response_code(status_code);
fetcher->SetResponseString(response_data);
}
void CheckinRequestTest::CompleteFetch() {
- net::TestURLFetcher* fetcher = url_fetcher_factory_.GetFetcherByID(0);
+ net::TestURLFetcher* fetcher =
+ url_fetcher_factory_.GetFetcherByID(0);
ASSERT_TRUE(fetcher);
fetcher->delegate()->OnURLFetchComplete(fetcher);
}
@@ -165,8 +205,14 @@ TEST_F(CheckinRequestTest, ResponseBodyEmpty) {
SetResponseStatusAndString(net::HTTP_OK, std::string());
CompleteFetch();
- EXPECT_EQ(0u, android_id_);
- EXPECT_EQ(0u, security_token_);
+ EXPECT_FALSE(callback_called_);
+
+ SetResponse(VALID_RESPONSE);
+ CompleteFetch();
+
+ EXPECT_TRUE(callback_called_);
+ EXPECT_EQ(kAndroidId, android_id_);
+ EXPECT_EQ(kSecurityToken, security_token_);
}
TEST_F(CheckinRequestTest, ResponseBodyCorrupted) {
@@ -176,21 +222,57 @@ TEST_F(CheckinRequestTest, ResponseBodyCorrupted) {
SetResponseStatusAndString(net::HTTP_OK, "Corrupted response body");
CompleteFetch();
+ EXPECT_FALSE(callback_called_);
+
+ SetResponse(VALID_RESPONSE);
+ CompleteFetch();
+
+ EXPECT_TRUE(callback_called_);
+ EXPECT_EQ(kAndroidId, android_id_);
+ EXPECT_EQ(kSecurityToken, security_token_);
+}
+
+TEST_F(CheckinRequestTest, ResponseHttpStatusUnauthorized) {
+ CreateRequest(0u, 0u);
+ request_->Start();
+
+ SetResponseStatusAndString(net::HTTP_UNAUTHORIZED, std::string());
+ CompleteFetch();
+
+ EXPECT_TRUE(callback_called_);
EXPECT_EQ(0u, android_id_);
EXPECT_EQ(0u, security_token_);
}
-TEST_F(CheckinRequestTest, ResponseHttpStatusNotOK) {
+TEST_F(CheckinRequestTest, ResponseHttpStatusBadRequest) {
CreateRequest(0u, 0u);
request_->Start();
- SetResponseStatusAndString(net::HTTP_UNAUTHORIZED, std::string());
+ SetResponseStatusAndString(net::HTTP_BAD_REQUEST, std::string());
CompleteFetch();
+ EXPECT_TRUE(callback_called_);
EXPECT_EQ(0u, android_id_);
EXPECT_EQ(0u, security_token_);
}
+TEST_F(CheckinRequestTest, ResponseHttpStatusNotOK) {
+ CreateRequest(0u, 0u);
+ request_->Start();
+
+ SetResponseStatusAndString(net::HTTP_INTERNAL_SERVER_ERROR, std::string());
+ CompleteFetch();
+
+ EXPECT_FALSE(callback_called_);
+
+ SetResponse(VALID_RESPONSE);
+ CompleteFetch();
+
+ EXPECT_TRUE(callback_called_);
+ EXPECT_EQ(kAndroidId, android_id_);
+ EXPECT_EQ(kSecurityToken, security_token_);
+}
+
TEST_F(CheckinRequestTest, ResponseMissingAndroidId) {
CreateRequest(0u, 0u);
request_->Start();
@@ -198,8 +280,14 @@ TEST_F(CheckinRequestTest, ResponseMissingAndroidId) {
SetResponse(MISSING_ANDROID_ID);
CompleteFetch();
- EXPECT_EQ(0u, android_id_);
- EXPECT_EQ(0u, security_token_);
+ EXPECT_FALSE(callback_called_);
+
+ SetResponse(VALID_RESPONSE);
+ CompleteFetch();
+
+ EXPECT_TRUE(callback_called_);
+ EXPECT_EQ(kAndroidId, android_id_);
+ EXPECT_EQ(kSecurityToken, security_token_);
}
TEST_F(CheckinRequestTest, ResponseMissingSecurityToken) {
@@ -209,8 +297,14 @@ TEST_F(CheckinRequestTest, ResponseMissingSecurityToken) {
SetResponse(MISSING_SECURITY_TOKEN);
CompleteFetch();
- EXPECT_EQ(0u, android_id_);
- EXPECT_EQ(0u, security_token_);
+ EXPECT_FALSE(callback_called_);
+
+ SetResponse(VALID_RESPONSE);
+ CompleteFetch();
+
+ EXPECT_TRUE(callback_called_);
+ EXPECT_EQ(kAndroidId, android_id_);
+ EXPECT_EQ(kSecurityToken, security_token_);
}
TEST_F(CheckinRequestTest, AndroidIdEqualsZeroInResponse) {
@@ -220,8 +314,14 @@ TEST_F(CheckinRequestTest, AndroidIdEqualsZeroInResponse) {
SetResponse(ANDROID_ID_IS_ZER0);
CompleteFetch();
- EXPECT_EQ(0u, android_id_);
- EXPECT_EQ(0u, security_token_);
+ EXPECT_FALSE(callback_called_);
+
+ SetResponse(VALID_RESPONSE);
+ CompleteFetch();
+
+ EXPECT_TRUE(callback_called_);
+ EXPECT_EQ(kAndroidId, android_id_);
+ EXPECT_EQ(kSecurityToken, security_token_);
}
TEST_F(CheckinRequestTest, SecurityTokenEqualsZeroInResponse) {
@@ -231,8 +331,14 @@ TEST_F(CheckinRequestTest, SecurityTokenEqualsZeroInResponse) {
SetResponse(SECURITY_TOKEN_IS_ZERO);
CompleteFetch();
- EXPECT_EQ(0u, android_id_);
- EXPECT_EQ(0u, security_token_);
+ EXPECT_FALSE(callback_called_);
+
+ SetResponse(VALID_RESPONSE);
+ CompleteFetch();
+
+ EXPECT_TRUE(callback_called_);
+ EXPECT_EQ(kAndroidId, android_id_);
+ EXPECT_EQ(kSecurityToken, security_token_);
}
TEST_F(CheckinRequestTest, SuccessfulFirstTimeCheckin) {
@@ -253,6 +359,7 @@ TEST_F(CheckinRequestTest, SuccessfulSubsequentCheckin) {
SetResponse(VALID_RESPONSE);
CompleteFetch();
+ EXPECT_TRUE(callback_called_);
EXPECT_EQ(kAndroidId, android_id_);
EXPECT_EQ(kSecurityToken, security_token_);
}
diff --git a/google_apis/gcm/gcm_client_impl.cc b/google_apis/gcm/gcm_client_impl.cc
index bf78cf6..abadb62 100644
--- a/google_apis/gcm/gcm_client_impl.cc
+++ b/google_apis/gcm/gcm_client_impl.cc
@@ -256,6 +256,7 @@ void GCMClientImpl::StartCheckin(int64 user_serial_number,
base::Bind(&GCMClientImpl::OnCheckinCompleted,
weak_ptr_factory_.GetWeakPtr(),
user_serial_number),
+ kDefaultBackoffPolicy,
chrome_build_proto_,
user_serial_number,
checkin_info.android_id,
@@ -300,7 +301,8 @@ void GCMClientImpl::OnCheckinCompleted(int64 user_serial_number,
void GCMClientImpl::OnDeviceCheckinCompleted(const CheckinInfo& checkin_info) {
if (!checkin_info.IsValid()) {
- // TODO(fgorski): Trigger a retry logic here. (no need to start over).
+ // TODO(fgorski): I don't think a retry here will help, we should probalby
+ // start over. By checking in with (0, 0).
return;
}
diff --git a/google_apis/gcm/tools/mcs_probe.cc b/google_apis/gcm/tools/mcs_probe.cc
index 7d05a29..b0de673 100644
--- a/google_apis/gcm/tools/mcs_probe.cc
+++ b/google_apis/gcm/tools/mcs_probe.cc
@@ -428,6 +428,7 @@ void MCSProbe::CheckIn() {
chrome_build_proto.set_chrome_version(kChromeVersion);
checkin_request_.reset(new CheckinRequest(
base::Bind(&MCSProbe::OnCheckInCompleted, base::Unretained(this)),
+ kDefaultBackoffPolicy,
chrome_build_proto,
kUserSerialNumber,
0,
generated by cgit v1.1 at 2025-01-23 17:04:57 +0000