Make DriveSystemService an observer of DriveNotificationManager so that it no longer has to handle registering for Drive XMPP notifications itself. This will also allow both SyncFileSystem and the ChromeOS Filemanager (and possibly more observers) to use XMPP notifications solving the problem from before where the same XMPP Invalidation ID was being registered twice.

Also removed polling code from SyncFS it's OKed to remove by both SyncFS and ChromeOS teams.

BUG=173339

Review URL: https://codereview.chromium.org/13891016

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@195482 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 22 insertions, 76 deletions

diff --git a/chrome/browser/chromeos/drive/drive_system_service.cc b/chrome/browser/chromeos/drive/drive_system_service.cc
index ae0d051..7dba08b 100644
--- a/chrome/browser/chromeos/drive/drive_system_service.cc
+++ b/chrome/browser/chromeos/drive/drive_system_service.cc
@@ -25,12 +25,12 @@
 #include "chrome/browser/google_apis/auth_service.h"
 #include "chrome/browser/google_apis/drive_api_service.h"
 #include "chrome/browser/google_apis/drive_api_util.h"
+#include "chrome/browser/google_apis/drive_notification_manager.h"
+#include "chrome/browser/google_apis/drive_notification_manager_factory.h"
 #include "chrome/browser/google_apis/gdata_wapi_service.h"
 #include "chrome/browser/google_apis/gdata_wapi_url_generator.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_dependency_manager.h"
-#include "chrome/browser/sync/profile_sync_service.h"
-#include "chrome/browser/sync/profile_sync_service_factory.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/chrome_version_info.h"
 #include "chrome/common/pref_names.h"
@@ -49,9 +49,6 @@ namespace {
 
 static const size_t kEventLogHistorySize = 100;
 
-// The sync invalidation object ID for Google Drive.
-const char kDriveInvalidationObjectId[] = "CHANGELOG";
-
 // Returns true if Drive is enabled for the given Profile.
 bool IsDriveEnabledForProfile(Profile* profile) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
@@ -106,7 +103,6 @@ DriveSystemService::DriveSystemService(
     DriveFileSystemInterface* test_file_system)
     : profile_(profile),
       drive_disabled_(false),
-      push_notification_registered_(false),
       ALLOW_THIS_IN_INITIALIZER_LIST(weak_ptr_factory_(this)) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   base::SequencedWorkerPool* blocking_pool = BrowserThread::GetBlockingPool();
@@ -176,19 +172,10 @@ void DriveSystemService::Initialize() {
 void DriveSystemService::Shutdown() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
-  ProfileSyncService* profile_sync_service =
-      profile_ ? ProfileSyncServiceFactory::GetForProfile(profile_) : NULL;
-  if (profile_sync_service && push_notification_registered_) {
-    // TODO(kochi): Once DriveSystemService gets started / stopped at runtime,
-    // this ID needs to be unregistered *before* the handler is unregistered
-    // as ID persists across browser restarts.
-    if (!IsDriveEnabledForProfile(profile_)) {
-      profile_sync_service->UpdateRegisteredInvalidationIds(
-          this, syncer::ObjectIdSet());
-    }
-    profile_sync_service->UnregisterInvalidationHandler(this);
-    push_notification_registered_ = false;
-  }
+  google_apis::DriveNotificationManager* drive_notification_manager =
+      google_apis::DriveNotificationManagerFactory::GetForProfile(profile_);
+  DCHECK(drive_notification_manager);
+  drive_notification_manager->RemoveObserver(this);
 
   RemoveDriveMountPoint();
 }
@@ -203,6 +190,10 @@ void DriveSystemService::RemoveObserver(DriveSystemServiceObserver* observer) {
   observers_.RemoveObserver(observer);
 }
 
+void DriveSystemService::OnNotificationReceived() {
+  file_system_->CheckForUpdates();
+}
+
 bool DriveSystemService::IsDriveEnabled() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
@@ -216,22 +207,6 @@ bool DriveSystemService::IsDriveEnabled() {
   return true;
 }
 
-void DriveSystemService::OnInvalidatorStateChange(
-    syncer::InvalidatorState state) {
-  DVLOG(1) << "InvalidatorState changed to " << state;
-}
-
-void DriveSystemService::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));
-
-  file_system_->CheckForUpdates();
-}
-
 void DriveSystemService::ClearCacheAndRemountFileSystem(
     const base::Callback<void(bool)>& callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
@@ -289,19 +264,6 @@ void DriveSystemService::ReloadAndRemountFileSystem() {
   AddDriveMountPoint();
 }
 
-bool DriveSystemService::PushNotificationEnabled() {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  DCHECK(profile_);
-
-  ProfileSyncService* profile_sync_service =
-      ProfileSyncServiceFactory::GetForProfile(profile_);
-  if (!profile_sync_service)
-    return false;
-
-  return (profile_sync_service->GetInvalidatorState() ==
-          syncer::INVALIDATIONS_ENABLED);
-}
-
 void DriveSystemService::AddDriveMountPoint() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(!file_system_proxy_.get());
@@ -379,18 +341,10 @@ void DriveSystemService::InitializeAfterResourceMetadataInitialized(
       cache_->GetCacheDirectoryPath(DriveCache::CACHE_TYPE_TMP_DOWNLOADS));
 
   // Register for Google Drive invalidation notifications.
-  ProfileSyncService* profile_sync_service =
-      profile_ ? ProfileSyncServiceFactory::GetForProfile(profile_) : NULL;
-  if (profile_sync_service) {
-    DCHECK(!push_notification_registered_);
-    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;
-  }
+  google_apis::DriveNotificationManager* drive_notification_manager =
+      google_apis::DriveNotificationManagerFactory::GetForProfile(profile_);
+  DCHECK(drive_notification_manager);
+  drive_notification_manager->AddObserver(this);
 
   AddDriveMountPoint();
 }
@@ -459,7 +413,7 @@ void DriveSystemServiceFactory::SetFactoryForTest(
 DriveSystemServiceFactory::DriveSystemServiceFactory()
     : ProfileKeyedServiceFactory("DriveSystemService",
                                  ProfileDependencyManager::GetInstance()) {
-  DependsOn(ProfileSyncServiceFactory::GetInstance());
+  DependsOn(google_apis::DriveNotificationManagerFactory::GetInstance());
   DependsOn(DownloadServiceFactory::GetInstance());
 }
 
diff --git a/chrome/browser/chromeos/drive/drive_system_service.h b/chrome/browser/chromeos/drive/drive_system_service.h
index 0ad2f05..f408fc9 100644
--- a/chrome/browser/chromeos/drive/drive_system_service.h
+++ b/chrome/browser/chromeos/drive/drive_system_service.h
@@ -16,9 +16,9 @@
 #include "chrome/browser/chromeos/drive/drive_file_error.h"
 #include "chrome/browser/chromeos/drive/drive_file_system_util.h"
 #include "chrome/browser/chromeos/drive/drive_scheduler.h"
+#include "chrome/browser/google_apis/drive_notification_observer.h"
 #include "chrome/browser/profiles/profile_keyed_service.h"
 #include "chrome/browser/profiles/profile_keyed_service_factory.h"
-#include "sync/notifier/invalidation_handler.h"
 
 namespace base {
 class FilePath;
@@ -65,8 +65,9 @@ class DriveSystemServiceObserver {
 // The class is essentially a container that manages lifetime of the objects
 // that are used to run the Drive system. The DriveSystemService object is
 // created per-profile.
-class DriveSystemService : public ProfileKeyedService,
-                           public syncer::InvalidationHandler {
+class DriveSystemService
+    : public ProfileKeyedService,
+      public google_apis::DriveNotificationObserver {
  public:
   // test_drive_service, test_cache_root and test_file_system are used by tests
   // to inject customized instances.
@@ -88,6 +89,9 @@ class DriveSystemService : public ProfileKeyedService,
   void AddObserver(DriveSystemServiceObserver* observer);
   void RemoveObserver(DriveSystemServiceObserver* observer);
 
+  // google_apis::DriveNotificationObserver implementation.
+  virtual void OnNotificationReceived() OVERRIDE;
+
   google_apis::DriveServiceInterface* drive_service() {
     return drive_service_.get();
   }
@@ -110,16 +114,7 @@ class DriveSystemService : public ProfileKeyedService,
   // Reloads and remounts the file system.
   void ReloadAndRemountFileSystem();
 
-  // Returns true if the push notification is enabled.
-  bool PushNotificationEnabled();
-
  private:
-  // syncer::InvalidationHandler implementation.
-  virtual void OnInvalidatorStateChange(
-      syncer::InvalidatorState state) OVERRIDE;
-  virtual void OnIncomingInvalidation(
-      const syncer::ObjectIdInvalidationMap& invalidation_map) OVERRIDE;
-
   // Returns true if Drive is enabled.
   // Must be called on UI thread.
   bool IsDriveEnabled();
@@ -159,9 +154,6 @@ class DriveSystemService : public ProfileKeyedService,
   // True if Drive is disabled due to initialization errors.
   bool drive_disabled_;
 
-  // True once this is registered to listen to the Drive updates.
-  bool push_notification_registered_;
-
   scoped_refptr<base::SequencedTaskRunner> blocking_task_runner_;
   scoped_ptr<EventLogger> event_logger_;
   scoped_ptr<DriveCache, util::DestroyHelper> cache_;