Implement new invalidations ack tracking system

sync/internal_api/public/base/invalidation.h

 // Copyright 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef SYNC_INTERNAL_API_PUBLIC_BASE_INVALIDATION_H_
 #define SYNC_INTERNAL_API_PUBLIC_BASE_INVALIDATION_H_
 
 #include <string>
 
 #include "base/basictypes.h"
@@ skipping 33 matching lines @@
   bool is_unknown_version() const;
 
   // Safe to call only if is_unknown_version() returns false.
   int64 version() const;
 
   // Safe to call only if is_unknown_version() returns false.
   const std::string& payload() const;
 
   const AckHandle& ack_handle() const;
 
-  // TODO(rlarocque): Remove this method and use AckHandlers instead.
-  void set_ack_handle(const AckHandle& ack_handle);
+  // Sets the AckHandler to be used to track this Invalidation.
+  //
+  // This should be set by the class that generates the invalidation.  Clients
+  // of the Invalidations API should not need to call this.

[tim (not reviewing), 2013/11/18 21:59:05]

This comment makes it sound like this could go in the constructor / factory
function and be fully hidden from clients of this class?

[rlarocque, 2013/11/19 00:35:49]

Almost, but not quite.  The UnackedInvalidationSet::ExportInvalidations function
uses it for mostly good reasons.  The Invalidations we load from the preferences
file during startup can't be attached to a SyncInvalidationListener right away,
since it might not exist yet.  We end up storing a set of invalidations without
an attached ack_handler in the UnackedInvalidationSet for that reason.

We could do more to avoid calling this function as often as we do (there are
five different invocations of it), but we would need to keep at least one
invocation for the case described above, and another for tests.

+  //
+  // Note that some sources of invalidations do not support ack tracking, and do
+  // not set the ack_handler.  This will be hidden from users of this class.
+  void set_ack_handler(syncer::WeakHandle<AckHandler> ack_handler);
 
-  // Functions from the alternative ack tracking framework.
-  // Currently unused.
-  void set_ack_handler(syncer::WeakHandle<AckHandler> ack_handler);
+  // Returns whether or not this instance supports ack tracking.  This will
+  // depend on whether or not the source of invaliadations supports
+  // invalidations.
+  //
+  // Clients can safely ignore this flag.  They can assume that all
+  // invalidations support ack tracking.  If they're wrong, then invalidations
+  // will be less reliable, but their behavior will be no less correct.
   bool SupportsAcknowledgement() const;
+
+  // Acknowledges the receipt of this invalidation.
+  //
+  // Clients should call this on a received invalidation when they have fully
+  // processed the invalidation and persisted the results to disk.  Once this
+  // function is called, the invalidations system is under no obligation to
+  // re-deliver this invalidation in the event of a crash or restart.
   void Acknowledge() const;
 
-  // Drops an invalidation.
+  // Informs the ack tracker that this invalidation will not be serviced.
+  //
+  // If a client's buffer reaches its limit and it is forced to start dropping
+  // invalidations, it should call this function before dropping its
+  // invalidations in order to allow the ack tracker to drop the invalidation,
+  // too.
   //
   // The drop record will be tracked by the specified
   // DroppedInvalidationTracker.  The caller should hang on to this tracker.  It
-  // will need to use it when it recovers from this drop event.  See the
+  // will need to use it when it recovers from this drop event, or if it needs
+  // to record another drop event for the same ObjectID.  Refer to the
   // documentation of DroppedInvalidationTracker for more details.
   void Drop(DroppedInvalidationTracker* tracker) const;
 
   scoped_ptr<base::DictionaryValue> ToValue() const;
   std::string ToString() const;
 
  private:
   Invalidation(const invalidation::ObjectId& id,
                bool is_unknown_version,
                int64 version,
@@ skipping 15 matching lines @@
   std::string payload_;
 
   // A locally generated unique ID used to manage local acknowledgements.
   AckHandle ack_handle_;
   syncer::WeakHandle<AckHandler> ack_handler_;
 };
 
 }  // namespace syncer
 
 #endif  // SYNC_INTERNAL_API_PUBLIC_BASE_INVALIDATION_H_