sync: Fix and test delete of server-unknown items

The fix we settled on for issue 125381 was to unset the unsynced bit for
items which were created and deleted locally, before the server was ever
iuformed of their existence.  The server doesn't like being notified of
deletions to items it never knew existed, as we learned with
issue 132905.

It turns out that the syncapi was deleting items in a way that bypassed
the originally attempted fix for issue 125381 in r137476.  This shows
that the testing of the original fix was inadequate.  This commit fixes
the bug and improves testing, with the hope of preventing these bugs
from reoccuring in the future.  The fix is so subtle that I doubt
anything other than lots and lots of tests will be able to prevent
regressions.

BUG=134934,125381
TEST=


Review URL: https://chromiumcodereview.appspot.com/10703060

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@145380 0039d316-1c4b-4281-b951-d872f2087c98

1 files changed, 4 insertions, 1 deletions

diff --git a/sync/internal_api/write_node.cc b/sync/internal_api/write_node.cc
index 5a303c4..9e6e403 100644
--- a/sync/internal_api/write_node.cc
+++ b/sync/internal_api/write_node.cc
@@ -484,8 +484,11 @@ syncable::MutableEntry* WriteNode::GetMutableEntryForTest() {
 }
 
 void WriteNode::Remove() {
-  entry_->Put(syncable::IS_DEL, true);
+  // These lines must be in this order.  The call to Put(IS_DEL) might choose to
+  // unset the IS_UNSYNCED bit if the item was not known to the server at the
+  // time of deletion.  It's important that the bit not be reset in that case.
   MarkForSyncing();
+  entry_->Put(syncable::IS_DEL, true);
 }
 
 bool WriteNode::PutPredecessor(const BaseNode* predecessor) {