Commit 3947750d authored by Rushan Suleymanov's avatar Rushan Suleymanov Committed by Commit Bot

[Sync] Handle sync disabling in sharing message bridge.

Remove all callbacks from the bridge after sync stopped. Check
if sync is running while sending new message.

Bug: 1045953
Change-Id: I6483ef246c6b521092dcfa837f66e9d524aae549
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2022711
Commit-Queue: Rushan Suleymanov <rushans@google.com>
Reviewed-by: default avatarAlex Chau <alexchau@chromium.org>
Reviewed-by: default avatarMarc Treib <treib@chromium.org>
Reviewed-by: default avatarvitaliii <vitaliii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#736358}
parent 5048588f
...@@ -43,6 +43,13 @@ SharingMessageBridgeImpl::~SharingMessageBridgeImpl() = default; ...@@ -43,6 +43,13 @@ SharingMessageBridgeImpl::~SharingMessageBridgeImpl() = default;
void SharingMessageBridgeImpl::SendSharingMessage( void SharingMessageBridgeImpl::SendSharingMessage(
std::unique_ptr<sync_pb::SharingMessageSpecifics> specifics, std::unique_ptr<sync_pb::SharingMessageSpecifics> specifics,
CommitFinishedCallback on_commit_callback) { CommitFinishedCallback on_commit_callback) {
if (!change_processor()->IsTrackingMetadata()) {
sync_pb::SharingMessageCommitError sync_disabled_error_message;
sync_disabled_error_message.set_error_code(
sync_pb::SharingMessageCommitError::SYNC_TURNED_OFF);
std::move(on_commit_callback).Run(sync_disabled_error_message);
return;
}
std::unique_ptr<syncer::MetadataChangeList> metadata_change_list = std::unique_ptr<syncer::MetadataChangeList> metadata_change_list =
CreateMetadataChangeList(); CreateMetadataChangeList();
// Fill in the internal message id with unique generated identifier. // Fill in the internal message id with unique generated identifier.
...@@ -75,8 +82,7 @@ base::Optional<syncer::ModelError> SharingMessageBridgeImpl::MergeSyncData( ...@@ -75,8 +82,7 @@ base::Optional<syncer::ModelError> SharingMessageBridgeImpl::MergeSyncData(
syncer::EntityChangeList entity_data) { syncer::EntityChangeList entity_data) {
DCHECK(entity_data.empty()); DCHECK(entity_data.empty());
DCHECK(change_processor()->IsTrackingMetadata()); DCHECK(change_processor()->IsTrackingMetadata());
return ApplySyncChanges(std::move(metadata_change_list), return {};
std::move(entity_data));
} }
base::Optional<syncer::ModelError> SharingMessageBridgeImpl::ApplySyncChanges( base::Optional<syncer::ModelError> SharingMessageBridgeImpl::ApplySyncChanges(
...@@ -129,6 +135,19 @@ void SharingMessageBridgeImpl::OnCommitAttemptErrors( ...@@ -129,6 +135,19 @@ void SharingMessageBridgeImpl::OnCommitAttemptErrors(
} }
} }
void SharingMessageBridgeImpl::ApplyStopSyncChanges(
std::unique_ptr<syncer::MetadataChangeList> metadata_change_list) {
sync_pb::SharingMessageCommitError sync_disabled_error_message;
sync_disabled_error_message.set_error_code(
sync_pb::SharingMessageCommitError::SYNC_TURNED_OFF);
for (auto& cth_and_callback : commit_callbacks_) {
// We do not need to untrack data here because the change processor will
// remove all entities anyway.
std::move(cth_and_callback.second).Run(sync_disabled_error_message);
}
commit_callbacks_.clear();
}
void SharingMessageBridgeImpl::ProcessCommitResponse( void SharingMessageBridgeImpl::ProcessCommitResponse(
const syncer::ClientTagHash& client_tag_hash, const syncer::ClientTagHash& client_tag_hash,
const sync_pb::SharingMessageCommitError& commit_error_message) { const sync_pb::SharingMessageCommitError& commit_error_message) {
......
...@@ -45,6 +45,8 @@ class SharingMessageBridgeImpl : public SharingMessageBridge, ...@@ -45,6 +45,8 @@ class SharingMessageBridgeImpl : public SharingMessageBridge,
std::string GetStorageKey(const syncer::EntityData& entity_data) override; std::string GetStorageKey(const syncer::EntityData& entity_data) override;
void OnCommitAttemptErrors( void OnCommitAttemptErrors(
const syncer::FailedCommitResponseDataList& error_response_list) override; const syncer::FailedCommitResponseDataList& error_response_list) override;
void ApplyStopSyncChanges(std::unique_ptr<syncer::MetadataChangeList>
metadata_change_list) override;
size_t GetCallbacksCountForTesting() const { size_t GetCallbacksCountForTesting() const {
return commit_callbacks_.size(); return commit_callbacks_.size();
......
...@@ -30,6 +30,10 @@ ACTION_TEMPLATE(SaveArgPointeeMove, ...@@ -30,6 +30,10 @@ ACTION_TEMPLATE(SaveArgPointeeMove,
*pointer = std::move(*testing::get<k>(args)); *pointer = std::move(*testing::get<k>(args));
} }
MATCHER_P(HasErrorCode, expected_error_code, "") {
return arg.error_code() == expected_error_code;
}
class SharingMessageBridgeTest : public testing::Test { class SharingMessageBridgeTest : public testing::Test {
protected: protected:
SharingMessageBridgeTest() { SharingMessageBridgeTest() {
...@@ -95,22 +99,15 @@ TEST_F(SharingMessageBridgeTest, ShouldInvokeCallbackOnSuccess) { ...@@ -95,22 +99,15 @@ TEST_F(SharingMessageBridgeTest, ShouldInvokeCallbackOnSuccess) {
EXPECT_CALL(*processor(), Put(_, _, _)).WillOnce(SaveArg<0>(&storage_key)); EXPECT_CALL(*processor(), Put(_, _, _)).WillOnce(SaveArg<0>(&storage_key));
base::MockCallback<SharingMessageBridge::CommitFinishedCallback> callback; base::MockCallback<SharingMessageBridge::CommitFinishedCallback> callback;
sync_pb::SharingMessageCommitError commit_error;
EXPECT_CALL(callback, Run).WillOnce(SaveArg<0>(&commit_error));
bridge()->SendSharingMessage(CreateSpecifics("payload"), callback.Get()); bridge()->SendSharingMessage(CreateSpecifics("payload"), callback.Get());
EXPECT_CALL(callback,
// The callback should be called only after committing data. Run(HasErrorCode(sync_pb::SharingMessageCommitError::NONE)));
EXPECT_FALSE(commit_error.has_error_code());
// Mark data as committed. // Mark data as committed.
syncer::EntityChangeList change_list; syncer::EntityChangeList change_list;
change_list.push_back(syncer::EntityChange::CreateDelete(storage_key)); change_list.push_back(syncer::EntityChange::CreateDelete(storage_key));
bridge()->ApplySyncChanges(nullptr, std::move(change_list)); bridge()->ApplySyncChanges(nullptr, std::move(change_list));
EXPECT_TRUE(commit_error.has_error_code());
EXPECT_EQ(commit_error.error_code(),
sync_pb::SharingMessageCommitError::NONE);
EXPECT_EQ(bridge()->GetCallbacksCountForTesting(), 0u); EXPECT_EQ(bridge()->GetCallbacksCountForTesting(), 0u);
} }
...@@ -149,4 +146,32 @@ TEST_F(SharingMessageBridgeTest, ShouldInvokeCallbackOnFailure) { ...@@ -149,4 +146,32 @@ TEST_F(SharingMessageBridgeTest, ShouldInvokeCallbackOnFailure) {
EXPECT_EQ(bridge()->GetCallbacksCountForTesting(), 0u); EXPECT_EQ(bridge()->GetCallbacksCountForTesting(), 0u);
} }
TEST_F(SharingMessageBridgeTest, ShouldInvokeCallbackIfSyncIsDisabled) {
ON_CALL(*processor(), IsTrackingMetadata()).WillByDefault(Return(false));
EXPECT_CALL(*processor(), Put).Times(0);
base::MockCallback<SharingMessageBridge::CommitFinishedCallback> callback;
sync_pb::SharingMessageCommitError commit_error;
EXPECT_CALL(callback, Run).WillOnce(SaveArg<0>(&commit_error));
bridge()->SendSharingMessage(CreateSpecifics("test_payload"), callback.Get());
EXPECT_EQ(commit_error.error_code(),
sync_pb::SharingMessageCommitError::SYNC_TURNED_OFF);
EXPECT_EQ(bridge()->GetCallbacksCountForTesting(), 0u);
}
TEST_F(SharingMessageBridgeTest, ShouldInvokeCallbackOnSyncStoppedEvent) {
base::MockCallback<SharingMessageBridge::CommitFinishedCallback> callback;
bridge()->SendSharingMessage(CreateSpecifics("test_payload"), callback.Get());
ASSERT_EQ(bridge()->GetCallbacksCountForTesting(), 1u);
EXPECT_CALL(
callback,
Run(HasErrorCode(sync_pb::SharingMessageCommitError::SYNC_TURNED_OFF)));
bridge()->ApplyStopSyncChanges(nullptr);
EXPECT_EQ(bridge()->GetCallbacksCountForTesting(), 0u);
}
} // namespace } // namespace
...@@ -63,6 +63,8 @@ message SharingMessageCommitError { ...@@ -63,6 +63,8 @@ message SharingMessageCommitError {
RESOURCE_EXHAUSTED = 5; RESOURCE_EXHAUSTED = 5;
UNAUTHENTICATED = 6; UNAUTHENTICATED = 6;
PERMISSION_DENIED = 7; PERMISSION_DENIED = 7;
// Client-specific error code.
SYNC_TURNED_OFF = 8;
} }
optional ErrorCode error_code = 1; optional ErrorCode error_code = 1;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment