summaryrefslogtreecommitdiffstats
path: root/sync/syncable
diff options
context:
space:
mode:
authorzea <zea@chromium.org>2015-04-01 18:11:04 -0700
committerCommit bot <commit-bot@chromium.org>2015-04-02 01:12:08 +0000
commit5a36760990918e47c63d9c19471aab229f566b3d (patch)
tree5dfe480798a4980edaae5dcd0c779bb7edcfbdc9 /sync/syncable
parent9faf912c9b5e7135630e2ccb7fbfe4e451d66a77 (diff)
downloadchromium_src-5a36760990918e47c63d9c19471aab229f566b3d.zip
chromium_src-5a36760990918e47c63d9c19471aab229f566b3d.tar.gz
chromium_src-5a36760990918e47c63d9c19471aab229f566b3d.tar.bz2
[Sync] Fix undeletion during commit logic
We were relying on the SYNCING bit to track whether a commit has begun or not. This will not work in all cases, as any modification actually unsets the SYNCING bit, thereby making it unreliable as a source for whether a sync cycle has started for an entity. To fix that, introduce a new DIRTY_SYNC bit that is only modified when a entry becomes dirty during a sync cycle. SYNCING can then better match it's name, and will stay true until the sync cycle ends. BUG=466098 TEST=repeatedly bookmark a page then remove the bookmark Review URL: https://codereview.chromium.org/1048933003 Cr-Commit-Position: refs/heads/master@{#323388}
Diffstat (limited to 'sync/syncable')
-rw-r--r--sync/syncable/entry.cc10
-rw-r--r--sync/syncable/entry.h6
-rw-r--r--sync/syncable/entry_kernel.h8
-rw-r--r--sync/syncable/model_neutral_mutable_entry.cc5
-rw-r--r--sync/syncable/model_neutral_mutable_entry.h1
-rw-r--r--sync/syncable/mutable_entry.cc7
-rw-r--r--sync/syncable/syncable_enum_conversions.cc3
7 files changed, 30 insertions, 10 deletions
diff --git a/sync/syncable/entry.cc b/sync/syncable/entry.cc
index 5bee5ee..9c6f862 100644
--- a/sync/syncable/entry.cc
+++ b/sync/syncable/entry.cc
@@ -62,6 +62,16 @@ base::DictionaryValue* Entry::ToValue(Cryptographer* cryptographer) const {
return entry_info;
}
+bool Entry::GetSyncing() const {
+ DCHECK(kernel_);
+ return kernel_->ref(SYNCING);
+}
+
+bool Entry::GetDirtySync() const {
+ DCHECK(kernel_);
+ return kernel_->ref(DIRTY_SYNC);
+}
+
ModelType Entry::GetServerModelType() const {
ModelType specifics_type = kernel_->GetServerModelType();
if (specifics_type != UNSPECIFIED)
diff --git a/sync/syncable/entry.h b/sync/syncable/entry.h
index 184377a..0c30ccd 100644
--- a/sync/syncable/entry.h
+++ b/sync/syncable/entry.h
@@ -215,10 +215,8 @@ class SYNC_EXPORT Entry {
return kernel_->ref(SERVER_ATTACHMENT_METADATA);
}
- bool GetSyncing() const {
- DCHECK(kernel_);
- return kernel_->ref(SYNCING);
- }
+ bool GetSyncing() const;
+ bool GetDirtySync() const ;
ModelType GetServerModelType() const;
ModelType GetModelType() const;
diff --git a/sync/syncable/entry_kernel.h b/sync/syncable/entry_kernel.h
index 0c2f471..3e0d2c7 100644
--- a/sync/syncable/entry_kernel.h
+++ b/sync/syncable/entry_kernel.h
@@ -178,9 +178,13 @@ enum {
};
enum BitTemp {
- // Not to be confused with IS_UNSYNCED, this bit is used to detect local
- // changes to items that happen during the server Commit operation.
+ // Whether a server commit operation was started and has not yet completed
+ // for this entity.
SYNCING = BIT_TEMPS_BEGIN,
+ // Whether a local change was made to an entity that had SYNCING set to true,
+ // and was therefore in the middle of a commit operation.
+ // Note: must only be set if SYNCING is true.
+ DIRTY_SYNC,
BIT_TEMPS_END,
};
diff --git a/sync/syncable/model_neutral_mutable_entry.cc b/sync/syncable/model_neutral_mutable_entry.cc
index 81c25c3..5064f4507 100644
--- a/sync/syncable/model_neutral_mutable_entry.cc
+++ b/sync/syncable/model_neutral_mutable_entry.cc
@@ -405,6 +405,11 @@ void ModelNeutralMutableEntry::PutSyncing(bool value) {
kernel_->put(SYNCING, value);
}
+void ModelNeutralMutableEntry::PutDirtySync(bool value) {
+ DCHECK(!value || GetSyncing());
+ kernel_->put(DIRTY_SYNC, value);
+}
+
void ModelNeutralMutableEntry::PutParentIdPropertyOnly(const Id& parent_id) {
base_write_transaction_->TrackChangesTo(kernel_);
dir()->ReindexParentId(base_write_transaction(), kernel_, parent_id);
diff --git a/sync/syncable/model_neutral_mutable_entry.h b/sync/syncable/model_neutral_mutable_entry.h
index 2a9a698..5ff71ad 100644
--- a/sync/syncable/model_neutral_mutable_entry.h
+++ b/sync/syncable/model_neutral_mutable_entry.h
@@ -78,6 +78,7 @@ class SYNC_EXPORT_PRIVATE ModelNeutralMutableEntry : public Entry {
void PutServerUniquePosition(const UniquePosition& value);
void PutServerAttachmentMetadata(const sync_pb::AttachmentMetadata& value);
void PutSyncing(bool value);
+ void PutDirtySync(bool value);
// Do a simple property-only update of the PARENT_ID field. Use with caution.
//
diff --git a/sync/syncable/mutable_entry.cc b/sync/syncable/mutable_entry.cc
index d627cf4..f6f3f16 100644
--- a/sync/syncable/mutable_entry.cc
+++ b/sync/syncable/mutable_entry.cc
@@ -187,8 +187,8 @@ void MutableEntry::PutIsDel(bool value) {
// - Let us delete this entry permanently when we next restart sync - see
// DirectoryBackingStore::SafeToPurgeOnLoading.
// This will avoid crbug.com/125381.
- // Note: do not unset IsUnsynced if the syncer is in the middle of
- // attempting to commit this entity.
+ // Note: do not unset IsUnsynced if the syncer has started but not yet
+ // finished committing this entity.
if (!GetId().ServerKnows() && !GetSyncing()) {
PutIsUnsynced(false);
}
@@ -293,7 +293,8 @@ bool MarkForSyncing(MutableEntry* e) {
DCHECK(!e->IsRoot()) << "We shouldn't mark a permanent object for syncing.";
if (!(e->PutIsUnsynced(true)))
return false;
- e->PutSyncing(false);
+ if (e->GetSyncing())
+ e->PutDirtySync(true);
return true;
}
diff --git a/sync/syncable/syncable_enum_conversions.cc b/sync/syncable/syncable_enum_conversions.cc
index b98500f..fe8cd5d4 100644
--- a/sync/syncable/syncable_enum_conversions.cc
+++ b/sync/syncable/syncable_enum_conversions.cc
@@ -176,10 +176,11 @@ const char* GetAttachmentMetadataFieldString(
}
const char* GetBitTempString(BitTemp bit_temp) {
- ASSERT_ENUM_BOUNDS(SYNCING, SYNCING,
+ ASSERT_ENUM_BOUNDS(SYNCING, DIRTY_SYNC,
BIT_TEMPS_BEGIN, BIT_TEMPS_END - 1);
switch (bit_temp) {
ENUM_CASE(SYNCING);
+ ENUM_CASE(DIRTY_SYNC);
case BIT_TEMPS_END: break;
}
NOTREACHED();