Unsubscribe from Drive invalidations when Drive shuts down

This is so that we don't stay subscribed to Drive invalidations in
Chrome when we switch to using DriveFS's invalidation stack.

Bug: 1130427
Change-Id: I290f5ddacac20175a3e4845d8dc7c62f86315c61
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2428286Reviewed-by: Kazuhiro Inaba <kinaba@chromium.org>
Reviewed-by: Sergei Datsenko <dats@chromium.org>
Commit-Queue: Austin Tankiang <austinct@chromium.org>
Cr-Commit-Position: refs/heads/master@{#812034}

chromeos/components/drivefs/drivefs_host.cc

@@ -59,6 +59,7 @@ class DriveFsHost::MountState : public DriveFsSession,
   ~MountState() override {
     DCHECK_CALLED_ON_VALID_SEQUENCE(host_->sequence_checker_);
     if (team_drives_fetched_) {
+      host_->delegate_->GetDriveNotificationManager().ClearTeamDriveIds();
       host_->delegate_->GetDriveNotificationManager().RemoveObserver(this);
     }
     if (is_mounted()) {

components/drive/drive_notification_manager.cc

@@ -50,7 +50,6 @@ DriveNotificationManager::DriveNotificationManager(
       batch_timer_(clock) {
   DCHECK(invalidation_service_);
   RegisterDriveNotifications();
-  RestartPollingTimer();
 }
 
 DriveNotificationManager::~DriveNotificationManager() {
@@ -128,6 +127,11 @@ bool DriveNotificationManager::IsPublicTopic(const syncer::Topic& topic) const {
 void DriveNotificationManager::AddObserver(
     DriveNotificationObserver* observer) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  if (!observers_.might_have_observers()) {
+    UpdateRegisteredDriveNotifications();
+    RestartPollingTimer();
+  }
+
   observers_.AddObserver(observer);
 }
 
@@ -135,6 +139,14 @@ void DriveNotificationManager::RemoveObserver(
     DriveNotificationObserver* observer) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   observers_.RemoveObserver(observer);
+
+  if (!observers_.might_have_observers()) {
+    CHECK(invalidation_service_->UpdateInterestedTopics(this,
+                                                        syncer::TopicSet()));
+    polling_timer_.Stop();
+    batch_timer_.Stop();
+    invalidated_change_ids_.clear();
+  }
 }
 
 void DriveNotificationManager::UpdateTeamDriveIds(
@@ -157,11 +169,21 @@ void DriveNotificationManager::UpdateTeamDriveIds(
     }
   }
 
-  if (set_changed) {
+  if (set_changed && observers_.might_have_observers()) {
     UpdateRegisteredDriveNotifications();
   }
 }
 
+void DriveNotificationManager::ClearTeamDriveIds() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  if (!team_drive_ids_.empty()) {
+    team_drive_ids_.clear();
+    if (observers_.might_have_observers()) {
+      UpdateRegisteredDriveNotifications();
+    }
+  }
+}
+
 void DriveNotificationManager::RestartPollingTimer() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   const int interval_secs = (push_notification_enabled_ ?
@@ -232,7 +254,7 @@ void DriveNotificationManager::RegisterDriveNotifications() {
 
   invalidation_service_->RegisterInvalidationHandler(this);
 
-  UpdateRegisteredDriveNotifications();
+  push_notification_registered_ = true;
 
   UMA_HISTOGRAM_BOOLEAN("Drive.PushNotificationRegistered",
                         push_notification_registered_);
@@ -251,7 +273,6 @@ void DriveNotificationManager::UpdateRegisteredDriveNotifications() {
   }
 
   CHECK(invalidation_service_->UpdateInterestedTopics(this, topics));
-  push_notification_registered_ = true;
   OnInvalidatorStateChange(invalidation_service_->GetInvalidatorState());
 }
 

components/drive/drive_notification_manager.h

@@ -60,6 +60,9 @@ class DriveNotificationManager : public KeyedService,
   void UpdateTeamDriveIds(const std::set<std::string>& added_team_drive_ids,
                           const std::set<std::string>& removed_team_drive_ids);
 
+  // Unsubscribe from invalidations from all team drives.
+  void ClearTeamDriveIds();
+
   // True when XMPP notification is currently enabled.
   bool push_notification_enabled() const {
     return push_notification_enabled_;

components/drive/drive_notification_manager_unittest.cc

@@ -222,4 +222,33 @@ TEST_F(DriveNotificationManagerTest, TestBatchInvalidation) {
   EXPECT_EQ(expected_ids, drive_notification_observer_->GetNotificationIds());
 }
 
+TEST_F(DriveNotificationManagerTest, UnregisterOnNoObservers) {
+  // By default, we should have registered for default corpus notifications on
+  // initialization.
+  auto subscribed_topics = fake_invalidation_service_->invalidator_registrar()
+                               .GetAllSubscribedTopics();
+
+  // TODO(crbug.com/1029698): replace syncer::Topics with syncer::TopicSet once
+  // |is_public| become the part of dedicated syncer::Topic struct.
+  syncer::Topics expected_topics;
+  expected_topics.emplace(kDefaultCorpusTopic,
+                          syncer::TopicMetadata{/*is_public=*/false});
+  EXPECT_EQ(expected_topics, subscribed_topics);
+
+  // Stop observing drive notification manager.
+  drive_notification_manager_->RemoveObserver(
+      drive_notification_observer_.get());
+
+  subscribed_topics = fake_invalidation_service_->invalidator_registrar()
+                          .GetAllSubscribedTopics();
+  EXPECT_EQ(syncer::Topics(), subscribed_topics);
+
+  // Start observing drive notification manager again. It should subscribe to
+  // the previously subscried topics.
+  drive_notification_manager_->AddObserver(drive_notification_observer_.get());
+  subscribed_topics = fake_invalidation_service_->invalidator_registrar()
+                          .GetAllSubscribedTopics();
+  EXPECT_EQ(expected_topics, subscribed_topics);
+}
+
 }  // namespace drive