[Sync] Add support for loading, updating and querying delete journals in

sync/syncable/directory.h

Index: sync/syncable/directory.h
diff --git a/sync/syncable/directory.h b/sync/syncable/directory.h
index 42475305c2952ee7608194050e7e0c5e301e8cb7..376b5f5710ddee071fe8e0f1215fc983e78a03d1 100644
--- a/sync/syncable/directory.h
+++ b/sync/syncable/directory.h
@@ -17,6 +17,7 @@
 #include "sync/syncable/entry_kernel.h"
 #include "sync/syncable/metahandle_set.h"
 #include "sync/syncable/scoped_kernel_lock.h"
+#include "sync/syncable/syncable-inl.h"
 
 namespace syncer {
 
@@ -114,13 +115,14 @@ enum UnlinkReason {
 
 class EntryKernelLessByMetaHandle {
  public:
-  inline bool operator()(const EntryKernel& a,
-                         const EntryKernel& b) const {
-    return a.ref(META_HANDLE) < b.ref(META_HANDLE);
+  inline bool operator()(const EntryKernel* a,
+                         const EntryKernel* b) const {
+    return a->ref(META_HANDLE) < b->ref(META_HANDLE);
   }
 };
 
-typedef std::set<EntryKernel, EntryKernelLessByMetaHandle> EntryKernelSet;
+typedef std::set<const EntryKernel*, EntryKernelLessByMetaHandle>

[tim (not reviewing), 2012/12/13 02:48:10]

Do we really need pointers? If so, I'd prefer to use base/memory/linked_ptr for
this. There was a thread on chromium-dev about discouraging linked_ptr with
vectors, and I just re-read it, and I still come away with the belief that
linked_ptr is preferable to a raw pointer (and using STLDeleteElements), and a
main reason is it would eliminate the need for the careful delete *it code you
have to write today. (And there's no ScopedSet style class in existence yet as
far as I know.)

[haitaol1, 2012/12/13 21:34:21]

Changed to pointer because it's more efficient to move delete journal from entry
kernel to snapshot to save. If using linked_ptr to store delete journal in entry
kernel, other indexes are converted to linked_ptr, too because they share same
template. The overhead of linked_ptr is probably not worth the cost then.

On 2012/12/13 02:48:10, timsteele wrote:

+    EntryKernelSet;
 
 enum InvariantCheckLevel {
   OFF = 0,            // No checking.
@@ -138,6 +140,7 @@ class Directory {
   friend class ScopedKernelUnlock;
   friend class WriteTransaction;
   friend class SyncableDirectoryTest;
+  FRIEND_TEST_ALL_PREFIXES(SyncableDirectoryTest, ManageDeleteJournals);
   FRIEND_TEST_ALL_PREFIXES(SyncableDirectoryTest,
                            TakeSnapshotGetsAllDirtyHandlesTest);
   FRIEND_TEST_ALL_PREFIXES(SyncableDirectoryTest,
@@ -212,6 +215,8 @@ class Directory {
     PersistedKernelInfo kernel_info;
     EntryKernelSet dirty_metas;
     MetahandleSet metahandles_to_purge;
+    EntryKernelSet delete_journals;
+    MetahandleSet delete_journals_to_purge;
   };
 
   // Does not take ownership of |encryptor|.
@@ -314,6 +319,28 @@ class Directory {
                             const tracked_objects::Location& location,
                             const std::string & message);
 
+  // Add/remove |entry| to/from |delete_journals_| according to its
+  // SERVER_IS_DEL field and |was_deleted|.
+  void UpdateDeleteJournals(BaseTransaction* trans, bool was_deleted,
+                            const EntryKernel* entry);
+
+  // Return entries of specified type in |delete_journals_|. This should be
+  // called ONCE in model association. |deleted_entries| can be used to
+  // detect deleted sync data that's not persisted in native model to
+  // prevent back-from-dead problem. |deleted_entries| are only valid during
+  // lifetime of |trans|. |type| is added to |passive_delete_journal_types_| to
+  // enable periodically saving/clearing of delete journals of |type| because
+  // new journals added later are not needed until next model association.
+  void GetDeleteJournals(BaseTransaction* trans, ModelType type,
+                         EntryKernelSet* deleted_entries);
+
+  // Purge entries of specified type in |delete_journals_| if their handles are
+  // in |to_purge|. This should be called after model association and
+  // |to_purge| should contain handles of the entries whose deletions are
+  // confirmed in native model.
+  void PurgeDeleteJournals(BaseTransaction* trans,
+                           const MetahandleSet& to_purge);
+
  protected:  // for friends, mainly used by Entry constructors
   virtual EntryKernel* GetEntryByHandle(int64 handle);
   virtual EntryKernel* GetEntryByHandle(int64 metahandle,
@@ -591,6 +618,16 @@ class Directory {
 
     // The transaction observer.
     const WeakHandle<TransactionObserver> transaction_observer;
+
+    // Contains deleted entries that may not be persisted in native models. And
+    // in case of unrecoverable error, all purged entries are moved here for
+    // bookkeeping to prevent back-from-dead entries that are deleted elsewhere
+    // when sync's down.
+    IdsIndex* delete_journals_;
+
+    // Contains meta handles of deleted entries that have been persisted or
+    // undeleted, thus can be removed from database.
+    MetahandleSet* const delete_journals_to_purge_;
   };
 
   // Helper method used to do searches on |parent_id_child_index|.
@@ -623,6 +660,9 @@ class Directory {
   EntryKernel* GetPossibleFirstChild(
       const ScopedKernelLock& lock, const Id& parent_id);
 
+  // Return true if delete journals of |type| are maintained.
+  static bool IsDeleteJournalEnabled(ModelType type);

[tim (not reviewing), 2012/12/13 02:48:10]

Do you anticipate needing to control this result from a test some day, or no?

[haitaol1, 2012/12/13 21:34:21]

No. I don't think that's necessary.

On 2012/12/13 02:48:10, timsteele wrote:

+
   Kernel* kernel_;
 
   scoped_ptr<DirectoryBackingStore> store_;
@@ -635,6 +675,10 @@ class Directory {
   NigoriHandler* const nigori_handler_;
   Cryptographer* const cryptographer_;
 
+  // Delete journals of these types can be cleared from memory after being
+  // saved to database.
+  ModelTypeSet passive_delete_journal_types_;
+
   InvariantCheckLevel invariant_check_level_;
 };