Implement new invalidations ack tracking system

chrome/browser/drive/drive_notification_manager.cc

 // Copyright 2013 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.
 
 #include "chrome/browser/drive/drive_notification_manager.h"
 
 #include "base/metrics/histogram.h"
 #include "chrome/browser/drive/drive_notification_observer.h"
 #include "chrome/browser/invalidation/invalidation_service.h"
 #include "chrome/browser/invalidation/invalidation_service_factory.h"
 #include "google/cacheinvalidation/types.pb.h"
+#include "sync/notifier/object_id_invalidation_map.h"
 
 namespace drive {
 
 namespace {
 
 // The polling interval time is used when XMPP is disabled.
 const int kFastPollingIntervalInSecs = 60;
 
 // The polling interval time is used when XMPP is enabled.  Theoretically
 // polling should be unnecessary if XMPP is enabled, but just in case.
@@ skipping 46 matching lines @@
 void DriveNotificationManager::OnIncomingInvalidation(
     const syncer::ObjectIdInvalidationMap& invalidation_map) {
   DVLOG(2) << "XMPP Drive Notification Received";
   syncer::ObjectIdSet ids = invalidation_map.GetObjectIds();
   DCHECK_EQ(1U, ids.size());
   const invalidation::ObjectId object_id(
       ipc::invalidation::ObjectSource::COSMO_CHANGELOG,
       kDriveInvalidationObjectId);
   DCHECK_EQ(1U, ids.count(object_id));
 
-  // TODO(dcheng): Only acknowledge the invalidation once the fetch has
-  // completed. http://crbug.com/156843
-  DCHECK(invalidation_service_);
-  syncer::Invalidation inv = invalidation_map.ForObject(object_id).back();
-  invalidation_service_->AcknowledgeInvalidation(object_id, inv.ack_handle());
-
+  // TODO: Acknowledge only after fetch completes.  See crbug.com/320878.

[satorux1, 2013/11/25 01:57:46]

nit: TODO should have a name.

[rlarocque, 2013/11/25 19:34:24]

Can you suggest an owner?

Implementing this would require intimate knowledge of the
DriveNotificationManager.  I don't think I'm qualified for the task.

I was just carrying over the old TODO without much thought.  Now that I think
about it, maybe this should be rephrased as "investigate if supporting
invalidations local acknowledgements is worthwhile", or removed entirely.  If
the current invalidations behavior suits your needs, then I guess there's no
need to spend much effort on improving its reliability.

+  invalidation_map.AcknowledgeAll();
   NotifyObserversToUpdate(NOTIFICATION_XMPP);
 }
 
 void DriveNotificationManager::AddObserver(
     DriveNotificationObserver* observer) {
   observers_.AddObserver(observer);
 }
 
 void DriveNotificationManager::RemoveObserver(
     DriveNotificationObserver* observer) {
@@ skipping 57 matching lines @@
       return "NOTIFICATION_XMPP";
     case NOTIFICATION_POLLING:
       return "NOTIFICATION_POLLING";
   }
 
   NOTREACHED();
   return "";
 }
 
 }  // namespace drive