summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--sync/engine/model_thread_sync_entity.cc15
-rw-r--r--sync/engine/model_thread_sync_entity.h6
-rw-r--r--sync/engine/non_blocking_type_processor.cc20
-rw-r--r--sync/engine/non_blocking_type_processor.h9
-rw-r--r--sync/engine/non_blocking_type_processor_unittest.cc110
-rw-r--r--sync/test/engine/injectable_sync_core_proxy.cc4
6 files changed, 161 insertions, 3 deletions
diff --git a/sync/engine/model_thread_sync_entity.cc b/sync/engine/model_thread_sync_entity.cc
index 6e466f5..053a6c8 100644
--- a/sync/engine/model_thread_sync_entity.cc
+++ b/sync/engine/model_thread_sync_entity.cc
@@ -154,4 +154,19 @@ void ModelThreadSyncEntity::ReceiveCommitResponse(const std::string& id,
base_version_ = response_version;
}
+void ModelThreadSyncEntity::ClearTransientSyncState() {
+ // If we have any unacknowledged commit requests outstatnding, they've been
+ // dropped and we should forget about them.
+ commit_requested_sequence_number_ = acked_sequence_number_;
+}
+
+void ModelThreadSyncEntity::ClearSyncState() {
+ base_version_ = kUncommittedVersion;
+ is_dirty_ = true;
+ sequence_number_ = 1;
+ commit_requested_sequence_number_ = 0;
+ acked_sequence_number_ = 0;
+ id_.clear();
+}
+
} // namespace syncer
diff --git a/sync/engine/model_thread_sync_entity.h b/sync/engine/model_thread_sync_entity.h
index 31cc2b8..03a6d27 100644
--- a/sync/engine/model_thread_sync_entity.h
+++ b/sync/engine/model_thread_sync_entity.h
@@ -106,6 +106,12 @@ class SYNC_EXPORT_PRIVATE ModelThreadSyncEntity {
int64 sequence_number,
int64 response_version);
+ // Clears any in-memory sync state associated with outstanding commits.
+ void ClearTransientSyncState();
+
+ // Clears all sync state. Invoked when a user signs out.
+ void ClearSyncState();
+
private:
ModelThreadSyncEntity(int64 sequence_number,
int64 commit_requested_sequence_number,
diff --git a/sync/engine/non_blocking_type_processor.cc b/sync/engine/non_blocking_type_processor.cc
index c1982b3..9b80543 100644
--- a/sync/engine/non_blocking_type_processor.cc
+++ b/sync/engine/non_blocking_type_processor.cc
@@ -62,6 +62,8 @@ void NonBlockingTypeProcessor::Disable() {
DCHECK(CalledOnValidThread());
is_preferred_ = false;
Disconnect();
+
+ ClearSyncState();
}
void NonBlockingTypeProcessor::Disconnect() {
@@ -76,6 +78,8 @@ void NonBlockingTypeProcessor::Disconnect() {
weak_ptr_factory_for_sync_.InvalidateWeakPtrs();
core_interface_.reset();
+
+ ClearTransientSyncState();
}
base::WeakPtr<NonBlockingTypeProcessor>
@@ -227,4 +231,20 @@ void NonBlockingTypeProcessor::OnUpdateReceived(
// TODO: Inform the model of the new or updated data.
}
+void NonBlockingTypeProcessor::ClearTransientSyncState() {
+ for (EntityMap::iterator it = entities_.begin(); it != entities_.end();
+ ++it) {
+ it->second->ClearTransientSyncState();
+ }
+}
+
+void NonBlockingTypeProcessor::ClearSyncState() {
+ for (EntityMap::iterator it = entities_.begin(); it != entities_.end();
+ ++it) {
+ it->second->ClearSyncState();
+ }
+
+ data_type_state_ = DataTypeState();
+}
+
} // namespace syncer
diff --git a/sync/engine/non_blocking_type_processor.h b/sync/engine/non_blocking_type_processor.h
index 2274c5d..da680f1 100644
--- a/sync/engine/non_blocking_type_processor.h
+++ b/sync/engine/non_blocking_type_processor.h
@@ -86,6 +86,15 @@ class SYNC_EXPORT_PRIVATE NonBlockingTypeProcessor : base::NonThreadSafe {
// Sends all commit requests that are due to be sent to the sync thread.
void FlushPendingCommitRequests();
+ // Clears any state related to outstanding communications with the
+ // NonBlockingTypeProcessorCore. Used when we want to disconnect from
+ // the current core.
+ void ClearTransientSyncState();
+
+ // Clears any state related to our communications with the current sync
+ // account. Useful when a user signs out of the current account.
+ void ClearSyncState();
+
ModelType type_;
DataTypeState data_type_state_;
diff --git a/sync/engine/non_blocking_type_processor_unittest.cc b/sync/engine/non_blocking_type_processor_unittest.cc
index 20ecb24..2c42afc 100644
--- a/sync/engine/non_blocking_type_processor_unittest.cc
+++ b/sync/engine/non_blocking_type_processor_unittest.cc
@@ -51,6 +51,17 @@ class NonBlockingTypeProcessorTest : public ::testing::Test {
// Initialize to a "ready-to-commit" state.
void InitializeToReadyState();
+ // Disconnect the NonBlockingTypeProcessorCore from our
+ // NonBlockingTypeProcessor.
+ void Disconnect();
+
+ // Disable sync for this NonBlockingTypeProcessor. Should cause sync state
+ // to be discarded.
+ void Disable();
+
+ // Re-enable sync after Disconnect() or Disable().
+ void ReEnable();
+
// Local data modification. Emulates signals from the model thread.
void WriteItem(const std::string& tag, const std::string& value);
void DeleteItem(const std::string& tag);
@@ -116,6 +127,31 @@ void NonBlockingTypeProcessorTest::InitializeToReadyState() {
OnInitialSyncDone();
}
+void NonBlockingTypeProcessorTest::Disconnect() {
+ processor_->Disconnect();
+ injectable_sync_core_proxy_.reset();
+ mock_processor_core_ = NULL;
+}
+
+void NonBlockingTypeProcessorTest::Disable() {
+ processor_->Disable();
+ injectable_sync_core_proxy_.reset();
+ mock_processor_core_ = NULL;
+}
+
+void NonBlockingTypeProcessorTest::ReEnable() {
+ DCHECK(!processor_->IsConnected());
+
+ // Prepare a new NonBlockingTypeProcesorCore instance, just as we would
+ // if this happened in the real world.
+ mock_processor_core_ = new MockNonBlockingTypeProcessorCore();
+ injectable_sync_core_proxy_.reset(
+ new InjectableSyncCoreProxy(mock_processor_core_));
+
+ // Re-enable sync with the new NonBlockingTypeProcessorCore.
+ processor_->Enable(injectable_sync_core_proxy_->Clone());
+}
+
void NonBlockingTypeProcessorTest::WriteItem(const std::string& tag,
const std::string& value) {
const std::string tag_hash = GenerateTagHash(tag);
@@ -351,6 +387,78 @@ TEST_F(NonBlockingTypeProcessorTest, NoCommitsUntilInitialSyncDone) {
EXPECT_TRUE(HasCommitRequestForTag("tag1"));
}
-// TODO(rlarocque): Add more testing of non_unique_name fields.
+// Test proper handling of disconnect and reconnect.
+//
+// Creates items in various states of commit and verifies they re-attempt to
+// commit on reconnect.
+TEST_F(NonBlockingTypeProcessorTest, Disconnect) {
+ InitializeToReadyState();
+
+ // The first item is fully committed.
+ WriteItem("tag1", "value1");
+ ASSERT_TRUE(HasCommitRequestForTag("tag1"));
+ SuccessfulCommitResponse(GetLatestCommitRequestForTag("tag1"));
+
+ // The second item has a commit request in progress.
+ WriteItem("tag2", "value2");
+ EXPECT_TRUE(HasCommitRequestForTag("tag2"));
+
+ Disconnect();
+
+ // The third item is added after disconnection.
+ WriteItem("tag3", "value3");
+
+ ReEnable();
+
+ EXPECT_EQ(1U, GetNumCommitRequestLists());
+ EXPECT_EQ(2U, GetNthCommitRequestList(0).size());
+
+ // The first item was already in sync.
+ EXPECT_FALSE(HasCommitRequestForTag("tag1"));
+
+ // The second item's commit was interrupted and should be retried.
+ EXPECT_TRUE(HasCommitRequestForTag("tag2"));
+
+ // The third item's commit was not started until the reconnect.
+ EXPECT_TRUE(HasCommitRequestForTag("tag3"));
+}
+
+// Test proper handling of disable and re-enable.
+//
+// Creates items in various states of commit and verifies they re-attempt to
+// commit on re-enable.
+TEST_F(NonBlockingTypeProcessorTest, Disable) {
+ InitializeToReadyState();
+
+ // The first item is fully committed.
+ WriteItem("tag1", "value1");
+ ASSERT_TRUE(HasCommitRequestForTag("tag1"));
+ SuccessfulCommitResponse(GetLatestCommitRequestForTag("tag1"));
+
+ // The second item has a commit request in progress.
+ WriteItem("tag2", "value2");
+ EXPECT_TRUE(HasCommitRequestForTag("tag2"));
+
+ Disable();
+
+ // The third item is added after disable.
+ WriteItem("tag3", "value3");
+
+ // Now we re-enable.
+ ReEnable();
+
+ // There should be nothing to commit right away, since we need to
+ // re-initialize the client state first.
+ EXPECT_EQ(0U, GetNumCommitRequestLists());
+
+ // Once we're ready to commit, all three local items should consider
+ // themselves uncommitted and pending for commit.
+ OnInitialSyncDone();
+ EXPECT_EQ(1U, GetNumCommitRequestLists());
+ EXPECT_EQ(3U, GetNthCommitRequestList(0).size());
+ EXPECT_TRUE(HasCommitRequestForTag("tag1"));
+ EXPECT_TRUE(HasCommitRequestForTag("tag2"));
+ EXPECT_TRUE(HasCommitRequestForTag("tag3"));
+}
} // namespace syncer
diff --git a/sync/test/engine/injectable_sync_core_proxy.cc b/sync/test/engine/injectable_sync_core_proxy.cc
index ac96ca4..4041736 100644
--- a/sync/test/engine/injectable_sync_core_proxy.cc
+++ b/sync/test/engine/injectable_sync_core_proxy.cc
@@ -33,8 +33,8 @@ void InjectableSyncCoreProxy::ConnectTypeToCore(
}
void InjectableSyncCoreProxy::Disconnect(syncer::ModelType type) {
- // This mock object is not meant for connect and disconnect tests.
- NOTREACHED() << "Not implemented";
+ // This should delete the core, but we don't own it.
+ processor_core_ = NULL;
}
scoped_ptr<SyncCoreProxy> InjectableSyncCoreProxy::Clone() const {