summaryrefslogtreecommitdiffstats
path: root/sync
diff options
context:
space:
mode:
authorzea@chromium.org <zea@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2014-04-09 00:17:36 +0000
committerzea@chromium.org <zea@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2014-04-09 00:17:36 +0000
commit694ffab8fbe7a05c10d694a109fc430cfc3ba472 (patch)
tree53d66bef2148a64e0a968638f07a6047f927c0ac /sync
parent6a9fa70e1b13a3bc5a125062cb03cf1f6ccb9239 (diff)
downloadchromium_src-694ffab8fbe7a05c10d694a109fc430cfc3ba472.zip
chromium_src-694ffab8fbe7a05c10d694a109fc430cfc3ba472.tar.gz
chromium_src-694ffab8fbe7a05c10d694a109fc430cfc3ba472.tar.bz2
[Sync] Add support for context changes that trigger refreshes
The ContextRefreshStatus controls whether a context change should force a refresh or not. A refresh just discards the progress marker token and marks all synced entities for the type has having version 1. This means all entities the server knows about are overwritten, while any locally created entities will remain in the same state (and be pushed to the server on the next commit). BUG=357368 Review URL: https://codereview.chromium.org/226473005 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@262567 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'sync')
-rw-r--r--sync/api/sync_change_processor.cc1
-rw-r--r--sync/api/sync_change_processor.h16
-rw-r--r--sync/engine/syncer_unittest.cc60
-rw-r--r--sync/internal_api/public/write_transaction.h10
-rw-r--r--sync/internal_api/write_transaction.cc36
-rw-r--r--sync/syncable/directory.cc34
-rw-r--r--sync/syncable/directory.h5
7 files changed, 147 insertions, 15 deletions
diff --git a/sync/api/sync_change_processor.cc b/sync/api/sync_change_processor.cc
index f6c268c..fd43835 100644
--- a/sync/api/sync_change_processor.cc
+++ b/sync/api/sync_change_processor.cc
@@ -12,6 +12,7 @@ SyncChangeProcessor::~SyncChangeProcessor() {}
syncer::SyncError SyncChangeProcessor::UpdateDataTypeContext(
ModelType type,
+ syncer::SyncChangeProcessor::ContextRefreshStatus refresh_status,
const std::string& context) {
// Do nothing.
return syncer::SyncError();
diff --git a/sync/api/sync_change_processor.h b/sync/api/sync_change_processor.h
index efc35f7..b70ef3d 100644
--- a/sync/api/sync_change_processor.h
+++ b/sync/api/sync_change_processor.h
@@ -25,6 +25,8 @@ typedef std::vector<SyncChange> SyncChangeList;
// An interface for services that handle receiving SyncChanges.
class SYNC_EXPORT SyncChangeProcessor {
public:
+ // Whether a context change should force a datatype refresh or not.
+ enum ContextRefreshStatus { NO_REFRESH, REFRESH_NEEDED };
typedef base::Callback<void(const SyncData&)> GetSyncDataCallback;
SyncChangeProcessor();
@@ -64,11 +66,15 @@ class SYNC_EXPORT SyncChangeProcessor {
const std::string& sync_tag,
const GetSyncDataCallback& callback) const {}
- // Updates the context for |type|. Default implementation does nothing.
- // A type's context is a per-client blob that can affect all SyncData
- // sent to/from the server, much like a cookie.
- virtual syncer::SyncError UpdateDataTypeContext(ModelType type,
- const std::string& context);
+ // Updates the context for |type|, triggering an optional context refresh.
+ // Default implementation does nothing. A type's context is a per-client blob
+ // that can affect all SyncData sent to/from the server, much like a cookie.
+ // TODO(zea): consider pulling the refresh logic into a separate method
+ // unrelated to datatype implementations.
+ virtual syncer::SyncError UpdateDataTypeContext(
+ ModelType type,
+ ContextRefreshStatus refresh_status,
+ const std::string& context);
};
} // namespace syncer
diff --git a/sync/engine/syncer_unittest.cc b/sync/engine/syncer_unittest.cc
index 72972a4..803fd43 100644
--- a/sync/engine/syncer_unittest.cc
+++ b/sync/engine/syncer_unittest.cc
@@ -1053,6 +1053,66 @@ TEST_F(SyncerTest, TestPurgeWithJournal) {
}
}
+TEST_F(SyncerTest, ResetVersions) {
+ // Download the top level pref node and some pref items.
+ mock_server_->AddUpdateDirectory(
+ parent_id_, root_id_, "prefs", 1, 10, std::string(), std::string());
+ mock_server_->SetLastUpdateServerTag(ModelTypeToRootTag(PREFERENCES));
+ mock_server_->AddUpdatePref("id1", parent_id_.GetServerId(), "tag1", 20, 20);
+ mock_server_->AddUpdatePref("id2", parent_id_.GetServerId(), "tag2", 30, 30);
+ mock_server_->AddUpdatePref("id3", parent_id_.GetServerId(), "tag3", 40, 40);
+ SyncShareNudge();
+
+ {
+ // Modify one of the preferences locally, mark another one as unapplied,
+ // and create another unsynced preference.
+ WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
+ MutableEntry entry(&wtrans, GET_BY_CLIENT_TAG, "tag1");
+ entry.PutIsUnsynced(true);
+
+ MutableEntry entry2(&wtrans, GET_BY_CLIENT_TAG, "tag2");
+ entry2.PutIsUnappliedUpdate(true);
+
+ MutableEntry entry4(&wtrans, CREATE, PREFERENCES, parent_id_, "name");
+ entry4.PutUniqueClientTag("tag4");
+ entry4.PutIsUnsynced(true);
+ }
+
+ {
+ // Reset the versions.
+ WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
+ ASSERT_TRUE(directory()->ResetVersionsForType(&wtrans, PREFERENCES));
+ }
+
+ {
+ // Verify the synced items are all with version 1 now, with
+ // unsynced/unapplied state preserved.
+ syncable::ReadTransaction trans(FROM_HERE, directory());
+ Entry entry(&trans, GET_BY_CLIENT_TAG, "tag1");
+ EXPECT_EQ(1, entry.GetBaseVersion());
+ EXPECT_EQ(1, entry.GetServerVersion());
+ EXPECT_TRUE(entry.GetIsUnsynced());
+ EXPECT_FALSE(entry.GetIsUnappliedUpdate());
+ Entry entry2(&trans, GET_BY_CLIENT_TAG, "tag2");
+ EXPECT_EQ(1, entry2.GetBaseVersion());
+ EXPECT_EQ(1, entry2.GetServerVersion());
+ EXPECT_FALSE(entry2.GetIsUnsynced());
+ EXPECT_TRUE(entry2.GetIsUnappliedUpdate());
+ Entry entry3(&trans, GET_BY_CLIENT_TAG, "tag3");
+ EXPECT_EQ(1, entry3.GetBaseVersion());
+ EXPECT_EQ(1, entry3.GetServerVersion());
+ EXPECT_FALSE(entry3.GetIsUnsynced());
+ EXPECT_FALSE(entry3.GetIsUnappliedUpdate());
+
+ // Entry 4 (the locally created one) should remain the same.
+ Entry entry4(&trans, GET_BY_CLIENT_TAG, "tag4");
+ EXPECT_EQ(-1, entry4.GetBaseVersion());
+ EXPECT_EQ(0, entry4.GetServerVersion());
+ EXPECT_TRUE(entry4.GetIsUnsynced());
+ EXPECT_FALSE(entry4.GetIsUnappliedUpdate());
+ }
+}
+
TEST_F(SyncerTest, TestCommitListOrderingTwoItemsTall) {
CommitOrderingTest items[] = {
{1, ids_.FromNumber(-1001), ids_.FromNumber(-1000)},
diff --git a/sync/internal_api/public/write_transaction.h b/sync/internal_api/public/write_transaction.h
index 8e797be..b0865ec 100644
--- a/sync/internal_api/public/write_transaction.h
+++ b/sync/internal_api/public/write_transaction.h
@@ -7,6 +7,7 @@
#include "base/basictypes.h"
#include "base/compiler_specific.h"
+#include "sync/api/sync_change_processor.h"
#include "sync/base/sync_export.h"
#include "sync/internal_api/public/base_transaction.h"
@@ -43,8 +44,13 @@ class SYNC_EXPORT WriteTransaction : public BaseTransaction {
virtual syncable::BaseTransaction* GetWrappedTrans() const OVERRIDE;
syncable::WriteTransaction* GetWrappedWriteTrans() { return transaction_; }
- // Set's a |type|'s local context. Does not affect any individual entities.
- void SetDataTypeContext(ModelType type, const std::string& context);
+ // Set's a |type|'s local context. |refresh_status| controls whether
+ // a datatype refresh is performed (clearing the progress marker token and
+ // setting the version of all synced entities to 1).
+ void SetDataTypeContext(
+ ModelType type,
+ syncer::SyncChangeProcessor::ContextRefreshStatus refresh_status,
+ const std::string& context);
protected:
WriteTransaction() {}
diff --git a/sync/internal_api/write_transaction.cc b/sync/internal_api/write_transaction.cc
index b0ed6a2..b2dfe87 100644
--- a/sync/internal_api/write_transaction.cc
+++ b/sync/internal_api/write_transaction.cc
@@ -37,8 +37,12 @@ syncable::BaseTransaction* WriteTransaction::GetWrappedTrans() const {
return transaction_;
}
-void WriteTransaction::SetDataTypeContext(ModelType type,
- const std::string& context) {
+void WriteTransaction::SetDataTypeContext(
+ ModelType type,
+ syncer::SyncChangeProcessor::ContextRefreshStatus refresh_status,
+ const std::string& context) {
+ DCHECK(ProtocolTypes().Has(type));
+ int field_number = GetSpecificsFieldNumberFromModelType(type);
sync_pb::DataTypeContext local_context;
GetDirectory()->GetDataTypeContext(transaction_,
type,
@@ -46,18 +50,34 @@ void WriteTransaction::SetDataTypeContext(ModelType type,
if (local_context.context() == context)
return;
- if (!local_context.has_data_type_id()) {
- local_context.set_data_type_id(
- syncer::GetSpecificsFieldNumberFromModelType(type));
- }
- DCHECK_EQ(syncer::GetSpecificsFieldNumberFromModelType(type),
- local_context.data_type_id());
+ if (!local_context.has_data_type_id())
+ local_context.set_data_type_id(field_number);
+
+ DCHECK_EQ(field_number, local_context.data_type_id());
DCHECK_GE(local_context.version(), 0);
local_context.set_version(local_context.version() + 1);
local_context.set_context(context);
GetDirectory()->SetDataTypeContext(transaction_,
type,
local_context);
+ if (refresh_status == syncer::SyncChangeProcessor::REFRESH_NEEDED) {
+ DVLOG(1) << "Forcing refresh of type " << ModelTypeToString(type);
+ // Clear the progress token from the progress markers. Preserve all other
+ // state, in case a GC directive was present.
+ sync_pb::DataTypeProgressMarker progress_marker;
+ GetDirectory()->GetDownloadProgress(type, &progress_marker);
+ progress_marker.clear_token();
+ GetDirectory()->SetDownloadProgress(type, progress_marker);
+
+ // Go through and reset the versions for all the synced entities of this
+ // data type.
+ GetDirectory()->ResetVersionsForType(transaction_, type);
+ }
+
+ // Note that it's possible for a GetUpdatesResponse that arrives immediately
+ // after the context update to override the cleared progress markers.
+ // TODO(zea): add a flag in the directory to prevent this from happening.
+ // See crbug.com/360280
}
} // namespace syncer
diff --git a/sync/syncable/directory.cc b/sync/syncable/directory.cc
index 193a538..11bc956 100644
--- a/sync/syncable/directory.cc
+++ b/sync/syncable/directory.cc
@@ -725,6 +725,40 @@ bool Directory::PurgeEntriesWithTypeIn(ModelTypeSet disabled_types,
return true;
}
+bool Directory::ResetVersionsForType(BaseWriteTransaction* trans,
+ ModelType type) {
+ if (!ProtocolTypes().Has(type))
+ return false;
+ DCHECK_NE(type, BOOKMARKS) << "Only non-hierarchical types are supported";
+
+ EntryKernel* type_root = GetEntryByServerTag(ModelTypeToRootTag(type));
+ if (!type_root)
+ return false;
+
+ ScopedKernelLock lock(this);
+ const Id& type_root_id = type_root->ref(ID);
+ Directory::Metahandles children;
+ AppendChildHandles(lock, type_root_id, &children);
+
+ for (Metahandles::iterator it = children.begin(); it != children.end();
+ ++it) {
+ EntryKernel* entry = GetEntryByHandle(*it, &lock);
+ if (!entry)
+ continue;
+ if (entry->ref(BASE_VERSION) > 1)
+ entry->put(BASE_VERSION, 1);
+ if (entry->ref(SERVER_VERSION) > 1)
+ entry->put(SERVER_VERSION, 1);
+
+ // Note that we do not unset IS_UNSYNCED or IS_UNAPPLIED_UPDATE in order
+ // to ensure no in-transit data is lost.
+
+ entry->mark_dirty(&kernel_->dirty_metahandles);
+ }
+
+ return true;
+}
+
void Directory::HandleSaveChangesFailure(const SaveChangesSnapshot& snapshot) {
WriteTransaction trans(FROM_HERE, HANDLE_SAVE_FAILURE, this);
ScopedKernelLock lock(this);
diff --git a/sync/syncable/directory.h b/sync/syncable/directory.h
index 07e66e6..6b91a5a 100644
--- a/sync/syncable/directory.h
+++ b/sync/syncable/directory.h
@@ -378,6 +378,11 @@ class SYNC_EXPORT Directory {
ModelTypeSet types_to_journal,
ModelTypeSet types_to_unapply);
+ // Resets the base_versions and server_versions of all synced entities
+ // associated with |type| to 1.
+ // WARNING! This can be slow, as it iterates over all entries for a type.
+ bool ResetVersionsForType(BaseWriteTransaction* trans, ModelType type);
+
protected: // for friends, mainly used by Entry constructors
virtual EntryKernel* GetEntryByHandle(int64 handle);
virtual EntryKernel* GetEntryByHandle(int64 metahandle,