Change DriveNotificationManager to pass invalidation change IDs as well.

Bug: 870004
Tbr: nhiroki@chromium.org
Change-Id: I49b60914dfe719e50941b18d7b96116d98ada86b
Reviewed-on: https://chromium-review.googlesource.com/c/1282513
Commit-Queue: Sam McNally <sammc@chromium.org>
Reviewed-by: Stuart Langley <slangley@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600605}

chrome/browser/chromeos/drive/drive_integration_service.cc

@@ -786,9 +786,13 @@ void DriveIntegrationService::RemoveObserver(
 }
 
 void DriveIntegrationService::OnNotificationReceived(
-    const std::set<std::string>& ids) {
+    const std::map<std::string, int64_t>& invalidations) {
   logger_->Log(logging::LOG_INFO,
                "Received Drive update notification. Will check for update.");
+  std::set<std::string> ids;
+  for (auto& invalidation : invalidations) {
+    ids.insert(invalidation.first);
+  }
   file_system_->CheckForUpdates(ids);
 }
 

chrome/browser/chromeos/drive/drive_integration_service.h

@@ -141,7 +141,8 @@ class DriveIntegrationService : public KeyedService,
   void RemoveObserver(DriveIntegrationServiceObserver* observer);
 
   // DriveNotificationObserver implementation.
-  void OnNotificationReceived(const std::set<std::string>& ids) override;
+  void OnNotificationReceived(
+      const std::map<std::string, int64_t>& invalidations) override;
   void OnNotificationTimerFired() override;
   void OnPushNotificationEnabled(bool enabled) override;
 

chrome/browser/sync_file_system/drive_backend/sync_engine.cc

@@ -645,7 +645,8 @@ void SyncEngine::ApplyLocalChange(const FileChange& local_change,
                      local_path, local_metadata, url, relayed_callback));
 }
 
-void SyncEngine::OnNotificationReceived(const std::set<std::string>& ids) {
+void SyncEngine::OnNotificationReceived(
+    const std::map<std::string, int64_t>& invalidations) {
   OnNotificationTimerFired();
 }
 

chrome/browser/sync_file_system/drive_backend/sync_engine.h

@@ -135,7 +135,8 @@ class SyncEngine
                         const SyncStatusCallback& callback) override;
 
   // drive::DriveNotificationObserver overrides.
-  void OnNotificationReceived(const std::set<std::string>& ids) override;
+  void OnNotificationReceived(
+      const std::map<std::string, int64_t>& invalidations) override;
   void OnNotificationTimerFired() override;
   void OnPushNotificationEnabled(bool enabled) override;
 

components/drive/drive_notification_manager.cc

@@ -98,15 +98,20 @@ void DriveNotificationManager::OnIncomingInvalidation(
   syncer::ObjectIdSet ids = invalidation_map.GetObjectIds();
 
   for (const auto& id : ids) {
-    if (id.name() == kDriveInvalidationObjectId) {
-      // Empty string indicates default change list.
-      invalidated_change_ids_.insert("");
-    } else if (base::StartsWith(id.name(), kTeamDriveChangePrefix,
-                                base::CompareCase::SENSITIVE)) {
-      invalidated_change_ids_.insert(
-          id.name().substr(kTeamDriveChangePrefixLength));
-    } else {
-      NOTREACHED() << "Unexpected ID " << id.name();
+    // Empty string indicates default change list.
+    std::string unpacked_id;
+    if (id.name() != kDriveInvalidationObjectId) {
+      DCHECK(base::StartsWith(id.name(), kTeamDriveChangePrefix,
+                              base::CompareCase::SENSITIVE))
+          << "Unexpected ID " << id.name();
+      unpacked_id = id.name().substr(kTeamDriveChangePrefixLength);
+    }
+    auto invalidations = invalidation_map.ForObject(id);
+    int64_t& invalidation_version = invalidated_change_ids_[unpacked_id] = -1;
+    for (auto& invalidation : invalidations) {
+      if (invalidation.version() > invalidation_version) {
+        invalidation_version = invalidation.version();
+      }
     }
   }
 
@@ -173,18 +178,18 @@ void DriveNotificationManager::RestartPollingTimer() {
       FROM_HERE, base::TimeDelta::FromSeconds(interval_secs),
       base::BindOnce(&DriveNotificationManager::NotifyObserversToUpdate,
                      weak_ptr_factory_.GetWeakPtr(), NOTIFICATION_POLLING,
-                     std::set<std::string>()));
+                     std::map<std::string, int64_t>()));
 }
 
 void DriveNotificationManager::NotifyObserversToUpdate(
     NotificationSource source,
-    const std::set<std::string> ids) {
+    std::map<std::string, int64_t> invalidations) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DVLOG(1) << "Notifying observers: " << NotificationSourceToString(source);
 
   if (source == NOTIFICATION_XMPP) {
     for (auto& observer : observers_)
-      observer.OnNotificationReceived(ids);
+      observer.OnNotificationReceived(invalidations);
   } else {
     for (auto& observer : observers_)
       observer.OnNotificationTimerFired();
@@ -240,7 +245,7 @@ void DriveNotificationManager::UpdateRegisteredDriveNotifications() {
 void DriveNotificationManager::OnBatchTimerExpired() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
-  std::set<std::string> change_ids_to_update;
+  std::map<std::string, int64_t> change_ids_to_update;
   invalidated_change_ids_.swap(change_ids_to_update);
   if (!change_ids_to_update.empty()) {
     NotifyObserversToUpdate(NOTIFICATION_XMPP, change_ids_to_update);

components/drive/drive_notification_manager.h

@@ -5,6 +5,9 @@
 #ifndef COMPONENTS_DRIVE_DRIVE_NOTIFICATION_MANAGER_H_
 #define COMPONENTS_DRIVE_DRIVE_NOTIFICATION_MANAGER_H_
 
+#include <stdint.h>
+
+#include <map>
 #include <memory>
 #include <set>
 #include <string>
@@ -77,7 +80,7 @@ class DriveNotificationManager : public KeyedService,
   // Notifies the observers that it's time to check for updates.
   // |source| indicates where the notification comes from.
   void NotifyObserversToUpdate(NotificationSource source,
-                               std::set<std::string> ids);
+                               std::map<std::string, int64_t> invalidations);
 
   // Registers for Google Drive invalidation notifications through XMPP.
   void RegisterDriveNotifications();
@@ -115,7 +118,7 @@ class DriveNotificationManager : public KeyedService,
 
   // The batch of invalidation id's that we've seen from the invaliation
   // service, will be reset when when send the invalidations to the observers.
-  std::set<std::string> invalidated_change_ids_;
+  std::map<std::string, int64_t> invalidated_change_ids_;
 
   SEQUENCE_CHECKER(sequence_checker_);
 

components/drive/drive_notification_manager_unittest.cc

@@ -36,20 +36,21 @@ class FakeDriveNotificationObserver : public DriveNotificationObserver {
   ~FakeDriveNotificationObserver() override {}
 
   // DriveNotificationObserver overrides
-  void OnNotificationReceived(const std::set<std::string>& ids) override {
+  void OnNotificationReceived(
+      const std::map<std::string, int64_t>& ids) override {
     notification_ids_ = ids;
   }
   void OnNotificationTimerFired() override {}
   void OnPushNotificationEnabled(bool enabled) override {}
 
-  const std::set<std::string> GetNotificationIds() const {
+  const std::map<std::string, int64_t> GetNotificationIds() const {
     return notification_ids_;
   }
 
   void ClearNotificationIds() { notification_ids_.clear(); }
 
  private:
-  std::set<std::string> notification_ids_;
+  std::map<std::string, int64_t> notification_ids_;
 };
 
 invalidation::ObjectId CreateTeamDriveInvalidationObjectId(
@@ -158,13 +159,13 @@ TEST_F(DriveNotificationManagerTest, TestBatchInvalidation) {
   // Emitting an invalidation should not call our observer until the timer
   // expires.
   fake_invalidation_service_->EmitInvalidationForTest(
-      syncer::Invalidation::InitUnknownVersion(kDefaultCorpusObjectId));
+      syncer::Invalidation::Init(kDefaultCorpusObjectId, 1, ""));
   EXPECT_TRUE(drive_notification_observer_->GetNotificationIds().empty());
 
   task_runner_->FastForwardBy(base::TimeDelta::FromSeconds(5));
 
   // Default corpus is has the id "" when sent to observers.
-  std::set<std::string> expected_ids = {""};
+  std::map<std::string, int64_t> expected_ids = {{"", 1}};
   EXPECT_EQ(expected_ids, drive_notification_observer_->GetNotificationIds());
   drive_notification_observer_->ClearNotificationIds();
 
@@ -177,35 +178,35 @@ TEST_F(DriveNotificationManagerTest, TestBatchInvalidation) {
   // Emit invalidation for default corpus, should not emit a team drive
   // invalidation.
   fake_invalidation_service_->EmitInvalidationForTest(
-      syncer::Invalidation::InitUnknownVersion(kDefaultCorpusObjectId));
+      syncer::Invalidation::Init(kDefaultCorpusObjectId, 1, ""));
   EXPECT_TRUE(drive_notification_observer_->GetNotificationIds().empty());
 
   task_runner_->FastForwardBy(base::TimeDelta::FromSeconds(5));
 
   // Default corpus is has the id "" when sent to observers.
-  expected_ids = {""};
+  expected_ids = {{"", 1}};
   EXPECT_EQ(expected_ids, drive_notification_observer_->GetNotificationIds());
   drive_notification_observer_->ClearNotificationIds();
 
   // Emit team drive invalidation
   fake_invalidation_service_->EmitInvalidationForTest(
-      syncer::Invalidation::InitUnknownVersion(team_drive_1_object_id));
+      syncer::Invalidation::Init(team_drive_1_object_id, 2, ""));
   EXPECT_TRUE(drive_notification_observer_->GetNotificationIds().empty());
 
   task_runner_->FastForwardBy(base::TimeDelta::FromSeconds(5));
-  expected_ids = {team_drive_id_1};
+  expected_ids = {{team_drive_id_1, 2}};
   EXPECT_EQ(expected_ids, drive_notification_observer_->GetNotificationIds());
   drive_notification_observer_->ClearNotificationIds();
 
   // Emit both default corpus and team drive.
   fake_invalidation_service_->EmitInvalidationForTest(
-      syncer::Invalidation::InitUnknownVersion(kDefaultCorpusObjectId));
+      syncer::Invalidation::Init(kDefaultCorpusObjectId, 1, ""));
   fake_invalidation_service_->EmitInvalidationForTest(
-      syncer::Invalidation::InitUnknownVersion(team_drive_1_object_id));
+      syncer::Invalidation::Init(team_drive_1_object_id, 2, ""));
   EXPECT_TRUE(drive_notification_observer_->GetNotificationIds().empty());
 
   task_runner_->FastForwardBy(base::TimeDelta::FromSeconds(5));
-  expected_ids = {"", team_drive_id_1};
+  expected_ids = {{"", 1}, {team_drive_id_1, 2}};
   EXPECT_EQ(expected_ids, drive_notification_observer_->GetNotificationIds());
 }
 

components/drive/drive_notification_observer.h

@@ -5,7 +5,7 @@
 #ifndef COMPONENTS_DRIVE_DRIVE_NOTIFICATION_OBSERVER_H_
 #define COMPONENTS_DRIVE_DRIVE_NOTIFICATION_OBSERVER_H_
 
-#include <set>
+#include <map>
 #include <string>
 
 namespace drive {
@@ -14,10 +14,12 @@ namespace drive {
 // updates.
 class DriveNotificationObserver {
  public:
-  // Called when a notification from Google Drive is received. |ids| is the set
-  // of objects that raised the notification, either a team drive id or empty
-  // string to represent the users default corpus.
-  virtual void OnNotificationReceived(const std::set<std::string>& ids) = 0;
+  // Called when a notification from Google Drive is received. |invalidations|
+  // is the map from objects that raised the notification to the changelist,
+  // either a team drive id or empty string to represent the users default
+  // corpus, to the change ID.
+  virtual void OnNotificationReceived(
+      const std::map<std::string, int64_t>& invalidations) = 0;
 
   // Called when there has not been a recent notification from Google Drive and
   // the timer used for polling Google Drive fired.