[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..71502257eb15ef792d6b67ef0c31bea10d8daeba 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>
+    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,

[tim (not reviewing), 2012/12/13 23:41:30]

Where is this called?

[haitaol1, 2012/12/14 19:22:38]

It's called from delete_journal.cc in the other patch.

On 2012/12/13 23:41:30, timsteele wrote:

+                           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_;

[tim (not reviewing), 2012/12/14 20:12:38]

I have one somewhat significant but honest suggestion -- I think we should pull
the journal specific state (these indexes, passive_delete_journal_types_) into a
DeleteJournal class and have Directory own that. You've done a nice job of
keeping access to the member variables limited (mostly to a pair of methods,
Update and Purge) and not only would it be a shame to regress on that by some
day adding rebel code that ignores the contract you put in place, but there's a
lot of complex code in Directory already and I think it'll be really nice to
think about DeleteJournal as something separable from the core Directory work,
code, and tests. 

[haitaol1, 2012/12/14 23:15:36]

Working on it.

On 2012/12/14 20:12:38, timsteele wrote:

   };
 
   // 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);
+
   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_;
 };