Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(729)

Unified Diff: sync/internal_api/attachments/attachment_service_impl_unittest.cc

Issue 569463002: Make AttachmentServiceImpl clear backoff when reconnected to network. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Merge with master. Created 6 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: sync/internal_api/attachments/attachment_service_impl_unittest.cc
diff --git a/sync/internal_api/attachments/attachment_service_impl_unittest.cc b/sync/internal_api/attachments/attachment_service_impl_unittest.cc
index dbbf89fdb8235000623e3287058100be2d23c58f..85bbb6a9a8d2671919e06a1db6d80779ff64bc2c 100644
--- a/sync/internal_api/attachments/attachment_service_impl_unittest.cc
+++ b/sync/internal_api/attachments/attachment_service_impl_unittest.cc
@@ -8,6 +8,7 @@
#include "base/memory/weak_ptr.h"
#include "base/message_loop/message_loop.h"
#include "base/run_loop.h"
+#include "base/timer/mock_timer.h"
#include "sync/internal_api/public/attachments/fake_attachment_downloader.h"
#include "sync/internal_api/public/attachments/fake_attachment_uploader.h"
#include "testing/gmock/include/gmock/gmock-matchers.h"
@@ -155,6 +156,7 @@ class AttachmentServiceImplTest : public testing::Test,
AttachmentServiceImplTest() {}
virtual void SetUp() OVERRIDE {
+ network_change_notifier_.reset(net::NetworkChangeNotifier::CreateMock());
InitializeAttachmentService(make_scoped_ptr(new MockAttachmentUploader()),
make_scoped_ptr(new MockAttachmentDownloader()),
this);
@@ -192,12 +194,19 @@ class AttachmentServiceImplTest : public testing::Test,
uploader.PassAs<AttachmentUploader>(),
downloader.PassAs<AttachmentDownloader>(),
delegate,
- base::TimeDelta(),
- base::TimeDelta()));
+ base::TimeDelta::FromMinutes(1),
+ base::TimeDelta::FromMinutes(8)));
+
+ scoped_ptr<base::MockTimer> timer_to_pass(
+ new base::MockTimer(false, false));
+ mock_timer_ = timer_to_pass.get();
+ attachment_service_->SetTimerForTest(timer_to_pass.PassAs<base::Timer>());
}
AttachmentService* attachment_service() { return attachment_service_.get(); }
+ base::MockTimer* mock_timer() { return mock_timer_; }
+
AttachmentService::GetOrDownloadCallback download_callback() {
return base::Bind(&AttachmentServiceImplTest::DownloadDone,
base::Unretained(this));
@@ -214,6 +223,14 @@ class AttachmentServiceImplTest : public testing::Test,
run_loop.RunUntilIdle();
}
+ void RunLoopAndFireTimer() {
+ RunLoop();
+ if (mock_timer()->IsRunning()) {
+ mock_timer()->Fire();
+ }
+ RunLoop();
+ }
+
const std::vector<AttachmentService::GetOrDownloadResult>&
download_results() const {
return download_results_;
@@ -223,6 +240,10 @@ class AttachmentServiceImplTest : public testing::Test,
return *last_download_attachments_.get();
}
+ net::NetworkChangeNotifier* network_change_notifier() {
+ return network_change_notifier_.get();
+ }
+
MockAttachmentStore* store() { return attachment_store_.get(); }
MockAttachmentDownloader* downloader() {
@@ -239,10 +260,12 @@ class AttachmentServiceImplTest : public testing::Test,
private:
base::MessageLoop message_loop_;
+ scoped_ptr<net::NetworkChangeNotifier> network_change_notifier_;
base::WeakPtr<MockAttachmentStore> attachment_store_;
base::WeakPtr<MockAttachmentDownloader> attachment_downloader_;
base::WeakPtr<MockAttachmentUploader> attachment_uploader_;
- scoped_ptr<AttachmentService> attachment_service_;
+ scoped_ptr<AttachmentServiceImpl> attachment_service_;
+ base::MockTimer* mock_timer_; // not owned
std::vector<AttachmentService::GetOrDownloadResult> download_results_;
scoped_ptr<AttachmentMap> last_download_attachments_;
@@ -353,13 +376,13 @@ TEST_F(AttachmentServiceImplTest, UploadAttachments_Success) {
attachment_service()->UploadAttachments(attachment_ids);
for (unsigned i = 0; i < num_attachments; ++i) {
- RunLoop();
+ RunLoopAndFireTimer();
// See that the service has issued a read for at least one of the
// attachments.
- ASSERT_GE(1U, store()->read_ids.size());
+ ASSERT_GE(store()->read_ids.size(), 1U);
store()->RespondToRead(attachment_ids);
RunLoop();
- ASSERT_GE(1U, uploader()->upload_requests.size());
+ ASSERT_GE(uploader()->upload_requests.size(), 1U);
uploader()->RespondToUpload(uploader()->upload_requests.begin()->first,
AttachmentUploader::UPLOAD_SUCCESS);
}
@@ -384,13 +407,13 @@ TEST_F(AttachmentServiceImplTest, UploadAttachments_Success_NoDelegate) {
AttachmentIdSet attachment_ids;
attachment_ids.insert(AttachmentId::Create());
attachment_service()->UploadAttachments(attachment_ids);
- RunLoop();
- EXPECT_EQ(1U, store()->read_ids.size());
- EXPECT_EQ(0U, uploader()->upload_requests.size());
+ RunLoopAndFireTimer();
+ ASSERT_EQ(1U, store()->read_ids.size());
+ ASSERT_EQ(0U, uploader()->upload_requests.size());
store()->RespondToRead(attachment_ids);
RunLoop();
- EXPECT_EQ(0U, store()->read_ids.size());
- EXPECT_EQ(1U, uploader()->upload_requests.size());
+ ASSERT_EQ(0U, store()->read_ids.size());
+ ASSERT_EQ(1U, uploader()->upload_requests.size());
uploader()->RespondToUpload(*attachment_ids.begin(),
AttachmentUploader::UPLOAD_SUCCESS);
RunLoop();
@@ -402,8 +425,8 @@ TEST_F(AttachmentServiceImplTest, UploadAttachments_SomeMissingFromStore) {
attachment_ids.insert(AttachmentId::Create());
attachment_ids.insert(AttachmentId::Create());
attachment_service()->UploadAttachments(attachment_ids);
- RunLoop();
- ASSERT_GE(1U, store()->read_ids.size());
+ RunLoopAndFireTimer();
+ ASSERT_GE(store()->read_ids.size(), 1U);
ASSERT_EQ(0U, uploader()->upload_requests.size());
store()->RespondToRead(attachment_ids);
@@ -412,9 +435,9 @@ TEST_F(AttachmentServiceImplTest, UploadAttachments_SomeMissingFromStore) {
uploader()->RespondToUpload(uploader()->upload_requests.begin()->first,
AttachmentUploader::UPLOAD_SUCCESS);
- RunLoop();
+ RunLoopAndFireTimer();
ASSERT_EQ(1U, on_attachment_uploaded_list().size());
- ASSERT_GE(1U, store()->read_ids.size());
+ ASSERT_GE(store()->read_ids.size(), 1U);
// Not found!
store()->RespondToRead(AttachmentIdSet());
RunLoop();
@@ -431,8 +454,8 @@ TEST_F(AttachmentServiceImplTest, UploadAttachments_AllMissingFromStore) {
attachment_service()->UploadAttachments(attachment_ids);
for (unsigned i = 0; i < num_attachments; ++i) {
- RunLoop();
- ASSERT_GE(1U, store()->read_ids.size());
+ RunLoopAndFireTimer();
+ ASSERT_GE(store()->read_ids.size(), 1U);
// None found!
store()->RespondToRead(AttachmentIdSet());
}
@@ -466,9 +489,9 @@ TEST_F(AttachmentServiceImplTest, UploadAttachments_OneUploadFails) {
}
attachment_service()->UploadAttachments(attachment_ids);
- for (unsigned i = 0; i < num_attachments; ++i) {
- RunLoop();
- ASSERT_GE(1U, store()->read_ids.size());
+ for (unsigned i = 0; i < 3; ++i) {
+ RunLoopAndFireTimer();
+ ASSERT_GE(store()->read_ids.size(), 1U);
store()->RespondToRead(attachment_ids);
RunLoop();
ASSERT_EQ(1U, uploader()->upload_requests.size());
@@ -482,9 +505,50 @@ TEST_F(AttachmentServiceImplTest, UploadAttachments_OneUploadFails) {
}
uploader()->RespondToUpload(uploader()->upload_requests.begin()->first,
result);
+ RunLoop();
}
- RunLoop();
ASSERT_EQ(2U, on_attachment_uploaded_list().size());
}
+// Attempt an upload, respond with transient error to trigger backoff, issue
+// network disconnect/connect events and see that backoff is cleared.
+TEST_F(AttachmentServiceImplTest,
+ UploadAttachments_ResetBackoffAfterNetworkChange) {
+ AttachmentIdSet attachment_ids;
+ attachment_ids.insert(AttachmentId::Create());
+ attachment_service()->UploadAttachments(attachment_ids);
+
+ RunLoopAndFireTimer();
+ ASSERT_EQ(1U, store()->read_ids.size());
+ store()->RespondToRead(attachment_ids);
+ RunLoop();
+ ASSERT_EQ(1U, uploader()->upload_requests.size());
+
+ uploader()->RespondToUpload(uploader()->upload_requests.begin()->first,
+ AttachmentUploader::UPLOAD_TRANSIENT_ERROR);
+ RunLoop();
+
+ // See that we are in backoff.
+ ASSERT_TRUE(mock_timer()->IsRunning());
+ ASSERT_GT(mock_timer()->GetCurrentDelay(), base::TimeDelta());
+
+ // Issue a network disconnect event.
+ network_change_notifier()->NotifyObserversOfNetworkChangeForTests(
+ net::NetworkChangeNotifier::CONNECTION_NONE);
+ RunLoop();
+
+ // Still in backoff.
+ ASSERT_TRUE(mock_timer()->IsRunning());
+ ASSERT_GT(mock_timer()->GetCurrentDelay(), base::TimeDelta());
+
+ // Issue a network connect event.
+ net::NetworkChangeNotifier::NotifyObserversOfNetworkChangeForTests(
+ net::NetworkChangeNotifier::CONNECTION_WIFI);
+ RunLoop();
+
+ // No longer in backoff.
+ ASSERT_TRUE(mock_timer()->IsRunning());
+ ASSERT_EQ(base::TimeDelta(), mock_timer()->GetCurrentDelay());
+}
+
} // namespace syncer
« no previous file with comments | « sync/internal_api/attachments/attachment_service_impl.cc ('k') | sync/internal_api/attachments/task_queue_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698