Ports XMPP Invalidation code from drive_file_sync_service in /sync_file_system to /google_apis/driv…

chrome/browser/sync_file_system/drive_file_sync_service.cc

Index: chrome/browser/sync_file_system/drive_file_sync_service.cc
diff --git a/chrome/browser/sync_file_system/drive_file_sync_service.cc b/chrome/browser/sync_file_system/drive_file_sync_service.cc
index 8b377cd3530cb14d81679a67e98e26ca31d1ceda..be1326eb4ae2a42d02be2c7c4cc5ee808a1d0ce6 100644
--- a/chrome/browser/sync_file_system/drive_file_sync_service.cc
+++ b/chrome/browser/sync_file_system/drive_file_sync_service.cc
@@ -18,9 +18,9 @@
 #include "base/values.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_system.h"
+#include "chrome/browser/google_apis/drive_notification_manager.h"
+#include "chrome/browser/google_apis/drive_notification_manager_factory.h"
 #include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/sync/profile_sync_service.h"
-#include "chrome/browser/sync/profile_sync_service_factory.h"
 #include "chrome/browser/sync_file_system/conflict_resolution_policy.h"
 #include "chrome/browser/sync_file_system/drive_file_sync_client.h"
 #include "chrome/browser/sync_file_system/drive_file_sync_util.h"
@@ -31,7 +31,6 @@
 #include "chrome/common/extensions/extension.h"
 #include "content/public/browser/browser_thread.h"
 #include "extensions/common/constants.h"
-#include "google/cacheinvalidation/types.pb.h"
 #include "webkit/fileapi/file_system_url.h"
 #include "webkit/fileapi/syncable/sync_file_metadata.h"
 #include "webkit/fileapi/syncable/sync_file_type.h"
@@ -47,9 +46,6 @@ const base::FilePath::CharType kTempDirName[] = FILE_PATH_LITERAL("tmp");
 const base::FilePath::CharType kSyncFileSystemDir[] =
     FILE_PATH_LITERAL("Sync FileSystem");
 
-// The sync invalidation object ID for Google Drive.
-const char kDriveInvalidationObjectId[] = "CHANGELOG";
-
 // Incremental sync polling interval.
 // TODO(calvinlo): Improve polling algorithm dependent on whether push
 // notifications are on or off.
@@ -155,31 +151,6 @@ class DriveFileSyncService::TaskToken {
   DISALLOW_COPY_AND_ASSIGN(TaskToken);
 };
 
-void DriveFileSyncService::OnInvalidatorStateChange(
-    syncer::InvalidatorState state) {
-  SetPushNotificationEnabled(state);
-}
-
-void DriveFileSyncService::OnIncomingInvalidation(
-    const syncer::ObjectIdInvalidationMap& invalidation_map) {
-  DCHECK_EQ(1U, invalidation_map.size());
-  const invalidation::ObjectId object_id(
-      ipc::invalidation::ObjectSource::COSMO_CHANGELOG,
-      kDriveInvalidationObjectId);
-  DCHECK_EQ(1U, invalidation_map.count(object_id));
-  // TODO(dcheng): Only acknowledge the invalidation once the fetch has
-  // completed. http://crbug.com/156843
-  ProfileSyncService* profile_sync_service =
-      ProfileSyncServiceFactory::GetForProfile(profile_);
-  CHECK(profile_sync_service);
-  profile_sync_service->AcknowledgeInvalidation(
-      invalidation_map.begin()->first,
-      invalidation_map.begin()->second.ack_handle);
-
-  may_have_unfetched_changes_ = true;
-  MaybeStartFetchChanges();
-}
-
 struct DriveFileSyncService::ProcessRemoteChangeParam {
   scoped_ptr<TaskToken> token;
   RemoteChange remote_change;
@@ -304,8 +275,6 @@ DriveFileSyncService::DriveFileSyncService(Profile* profile)
       state_(REMOTE_SERVICE_OK),
       sync_enabled_(true),
       largest_fetched_changestamp_(0),
-      push_notification_registered_(false),
-      push_notification_enabled_(false),
       polling_delay_seconds_(kMinimumPollingDelaySeconds),
       may_have_unfetched_changes_(false),
       remote_change_processor_(NULL),
@@ -338,22 +307,10 @@ DriveFileSyncService::~DriveFileSyncService() {
     sync_client_->RemoveObserver(this);
   token_.reset();
 
-  // Unregister for Drive notifications.
-  ProfileSyncService* profile_sync_service =
-      ProfileSyncServiceFactory::GetForProfile(profile_);
-  if (!profile_sync_service || !push_notification_registered_) {
-    return;
-  }
-
-  // TODO(calvinlo): Revisit this later in Consolidate Drive XMPP Notification
-  // and Polling Backup into one Class patch. http://crbug/173339.
-  // Original comment from Kochi about the order this is done in:
-  // Once DriveSystemService gets started / stopped at runtime, this ID needs to
-  // be unregistered *before* the handler is unregistered
-  // as ID persists across browser restarts.
-  profile_sync_service->UpdateRegisteredInvalidationIds(
-      this, syncer::ObjectIdSet());
-  profile_sync_service->UnregisterInvalidationHandler(this);
+  google_apis::DriveNotificationManager* drive_notification_manager =
+      google_apis::DriveNotificationManagerFactory::GetForProfile(profile_);
+  DCHECK(drive_notification_manager);
+  drive_notification_manager->RemoveObserver(this);
 }
 
 // static
@@ -741,8 +698,6 @@ DriveFileSyncService::DriveFileSyncService(
       state_(REMOTE_SERVICE_OK),
       sync_enabled_(true),
       largest_fetched_changestamp_(0),
-      push_notification_registered_(false),
-      push_notification_enabled_(false),
       polling_delay_seconds_(-1),
       may_have_unfetched_changes_(false),
       remote_change_processor_(NULL),
@@ -923,8 +878,12 @@ void DriveFileSyncService::DidInitializeMetadataStore(
     sync_client_->EnsureSyncRootIsNotInMyDrive(sync_root_resource_id());
 
   NotifyTaskDone(status, token.Pass());
-  RegisterDriveNotifications();
   may_have_unfetched_changes_ = true;
+
+  google_apis::DriveNotificationManager* drive_notification_manager =
+      google_apis::DriveNotificationManagerFactory::GetForProfile(profile_);
+  DCHECK(drive_notification_manager);
+  drive_notification_manager->AddObserver(this);
 }
 
 void DriveFileSyncService::UpdateRegisteredOrigins() {
@@ -1113,14 +1072,6 @@ void DriveFileSyncService::DidGetDirectoryContentForBatchSync(
     metadata_store_->MoveBatchSyncOriginToIncremental(origin);
   }
 
-  // If this was the last batch sync origin and push_notification is enabled
-  // (indicates that we may have longer polling cycle), trigger the first
-  // incremental sync on next task cycle.
-  if (pending_batch_sync_origins_.empty() &&
-      push_notification_enabled_) {
-    may_have_unfetched_changes_ = true;

[tzik, 2013/04/12 02:00:15]

Any alternate for this?

[calvinlo, 2013/04/12 04:42:00]

I think in general I want to get rid of the "may_have_unfetched_changes"
variable so only DriveNotificationManager manages when new changes should be
checked. Everytime CheckForUpdates() is called, may_have_unfetched_changes would
implicitly be true. If we can make that assertion true, then we can remove all
references to may_have_unfetched_changes in this file and thus this block would
no longer be needed.

[tzik, 2013/04/12 05:12:46]

I think we still need to kick CheckForUpdates around here, since the
notification has been ignored before this point.
So, how about just call CheckForUpdate here instead?

[calvinlo, 2013/04/12 06:44:38]

Done. Yeah, I agree, calling CheckForUpdates here is probably the safest thing
to do even thought it might not be the most optimized.

-  }
-
   NotifyTaskDone(SYNC_STATUS_OK, token.Pass());
 }
 
@@ -2191,6 +2142,8 @@ void DriveFileSyncService::MaybeStartFetchChanges() {
 }
 
 void DriveFileSyncService::CheckForUpdates() {
+  // TODO(calvinlo): Try to eliminate may_have_unfetched_changes_ variable.
+  may_have_unfetched_changes_ = true;
   MaybeStartFetchChanges();
 }
 
@@ -2352,11 +2305,6 @@ void DriveFileSyncService::UpdatePollingDelay(int64 new_delay_sec) {
     return;
   }
 
-  if (push_notification_enabled_) {
-    polling_delay_seconds_ = kPollingDelaySecondsWithNotification;
-    return;
-  }
-
   int64 old_delay = polling_delay_seconds_;
 
   // Push notifications off.
@@ -2366,52 +2314,6 @@ void DriveFileSyncService::UpdatePollingDelay(int64 new_delay_sec) {
     polling_timer_.Stop();
 }
 
-bool DriveFileSyncService::IsDriveNotificationSupported() {
-  // TODO(calvinlo): A invalidation ID can only be registered to one handler.
-  // Therefore ChromeOS and SyncFS cannot both use XMPP notifications until
-  // (http://crbug.com/173339) is completed.
-  // For now, disable XMPP notifications for SyncFC on ChromeOS to guarantee
-  // that ChromeOS's file manager can register itself to the invalidationID.
-
-#if defined(OS_CHROMEOS)
-  return false;
-#else
-  return true;
-#endif
-}
-
-// Register for Google Drive invalidation notifications through XMPP.
-void DriveFileSyncService::RegisterDriveNotifications() {
-  // Push notification registration might have already occurred if called from
-  // a different extension.
-  if (!IsDriveNotificationSupported() || push_notification_registered_)
-    return;
-
-  ProfileSyncService* profile_sync_service =
-      ProfileSyncServiceFactory::GetForProfile(profile_);
-  if (!profile_sync_service)
-    return;
-
-  profile_sync_service->RegisterInvalidationHandler(this);
-  syncer::ObjectIdSet ids;
-  ids.insert(invalidation::ObjectId(
-      ipc::invalidation::ObjectSource::COSMO_CHANGELOG,
-      kDriveInvalidationObjectId));
-  profile_sync_service->UpdateRegisteredInvalidationIds(this, ids);
-  push_notification_registered_ = true;
-  SetPushNotificationEnabled(profile_sync_service->GetInvalidatorState());
-}
-
-void DriveFileSyncService::SetPushNotificationEnabled(
-    syncer::InvalidatorState state) {
-  push_notification_enabled_ = (state == syncer::INVALIDATIONS_ENABLED);
-  if (!push_notification_enabled_)
-    return;
-
-  // Push notifications are enabled so reset polling timer.
-  UpdatePollingDelay(kPollingDelaySecondsWithNotification);
-}
-
 void DriveFileSyncService::NotifyObserversFileStatusChanged(
     const FileSystemURL& url,
     SyncFileStatus sync_status,