[Sync] Clean up ProfileSyncService observer events

Several spurious NotifyObserver calls have been removed, in addition to
removing the configuration retry code (which was not doing anything anymore).

BUG=418140

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

Cr-Commit-Position: refs/heads/master@{#297345}

13 files changed, 5 insertions, 57 deletions

diff --git a/chrome/browser/sync/glue/sync_backend_host_core.cc b/chrome/browser/sync/glue/sync_backend_host_core.cc
index 1827f51..cade550 100644
--- a/chrome/browser/sync/glue/sync_backend_host_core.cc
+++ b/chrome/browser/sync/glue/sync_backend_host_core.cc
@@ -135,11 +135,6 @@ void SyncBackendHostCore::DoRefreshTypes(syncer::ModelTypeSet types) {
   sync_manager_->RefreshTypes(types);
 }
 
-void SyncBackendHostCore::OnControlTypesDownloadRetry() {
-  host_.Call(FROM_HERE,
-             &SyncBackendHostImpl::HandleControlTypesDownloadRetry);
-}
-
 void SyncBackendHostCore::OnInitializationComplete(
     const syncer::WeakHandle<syncer::JsBackend>& js_backend,
     const syncer::WeakHandle<syncer::DataTypeDebugInfoListener>&
@@ -222,8 +217,7 @@ void SyncBackendHostCore::OnInitializationComplete(
       routing_info,
       base::Bind(&SyncBackendHostCore::DoInitialProcessControlTypes,
                  weak_ptr_factory_.GetWeakPtr()),
-      base::Bind(&SyncBackendHostCore::OnControlTypesDownloadRetry,
-                 weak_ptr_factory_.GetWeakPtr()));
+      base::Closure());
 }
 
 void SyncBackendHostCore::OnConnectionStatusChange(
diff --git a/chrome/browser/sync/glue/sync_backend_host_impl.cc b/chrome/browser/sync/glue/sync_backend_host_impl.cc
index cbded53..edf1097 100644
--- a/chrome/browser/sync/glue/sync_backend_host_impl.cc
+++ b/chrome/browser/sync/glue/sync_backend_host_impl.cc
@@ -626,14 +626,6 @@ void SyncBackendHostImpl::AddExperimentalTypes() {
     frontend_->OnExperimentsChanged(experiments);
 }
 
-void SyncBackendHostImpl::HandleControlTypesDownloadRetry() {
-  DCHECK_EQ(base::MessageLoop::current(), frontend_loop_);
-  if (!frontend_)
-    return;
-
-  frontend_->OnSyncConfigureRetry();
-}
-
 void SyncBackendHostImpl::HandleInitializationSuccessOnFrontendLoop(
     const syncer::WeakHandle<syncer::JsBackend> js_backend,
     const syncer::WeakHandle<syncer::DataTypeDebugInfoListener>
diff --git a/chrome/browser/sync/glue/sync_backend_host_impl.h b/chrome/browser/sync/glue/sync_backend_host_impl.h
index d4cd61f..8dee37f 100644
--- a/chrome/browser/sync/glue/sync_backend_host_impl.h
+++ b/chrome/browser/sync/glue/sync_backend_host_impl.h
@@ -179,10 +179,6 @@ class SyncBackendHostImpl
       syncer::SyncContextProxy* sync_context_proxy,
       const std::string& cache_guid);
 
-  // Downloading of control types failed and will be retried. Invokes the
-  // frontend's sync configure retry method.
-  void HandleControlTypesDownloadRetry();
-
   // Forwards a ProtocolEvent to the frontend.  Will not be called unless a
   // call to SetForwardProtocolEvents() explicitly requested that we start
   // forwarding these events.
diff --git a/chrome/browser/sync/profile_sync_service.cc b/chrome/browser/sync/profile_sync_service.cc
index 3135388..cb699d8 100644
--- a/chrome/browser/sync/profile_sync_service.cc
+++ b/chrome/browser/sync/profile_sync_service.cc
@@ -548,15 +548,6 @@ bool ProfileSyncService::IsEncryptedDatatypeEnabled() const {
   return !Intersection(preferred_types, encrypted_types).Empty();
 }
 
-void ProfileSyncService::OnSyncConfigureRetry() {
-  // Note: in order to handle auth failures that arise before the backend is
-  // initialized (e.g. from invalidation notifier, or downloading new control
-  // types), we have to gracefully handle configuration retries at all times.
-  // At this point an auth error badge should be shown, which once resolved
-  // will trigger a new sync cycle.
-  NotifyObservers();
-}
-
 void ProfileSyncService::OnProtocolEvent(
     const syncer::ProtocolEvent& event) {
   FOR_EACH_OBSERVER(browser_sync::ProtocolEventObserver,
@@ -694,8 +685,6 @@ void ProfileSyncService::StartUpSlowBackendComponents(
   InitializeBackend(ShouldDeleteSyncFolder());
 
   UpdateFirstSyncTimePref();
-
-  NotifyObservers();
 }
 
 void ProfileSyncService::OnGetTokenSuccess(
@@ -965,7 +954,6 @@ void ProfileSyncService::OnUnrecoverableErrorImpl(
   UMA_HISTOGRAM_ENUMERATION(kSyncUnrecoverableErrorHistogram,
                             unrecoverable_error_reason_,
                             ERROR_REASON_LIMIT);
-  NotifyObservers();
   std::string location;
   from_here.Write(true, true, &location);
   LOG(ERROR)
@@ -1568,14 +1556,6 @@ void ProfileSyncService::OnConfigureDone(
   }
 }
 
-void ProfileSyncService::OnConfigureRetry() {
-  // We should have cleared our cached passphrase before we get here (in
-  // OnBackendInitialized()).
-  DCHECK(cached_passphrase_.empty());
-
-  OnSyncConfigureRetry();
-}
-
 void ProfileSyncService::OnConfigureStart() {
   sync_configure_start_time_ = base::Time::Now();
   NotifyObservers();
@@ -1796,7 +1776,6 @@ void ProfileSyncService::OnUserChoseDatatypes(
     directory_data_type_manager_->ResetDataTypeErrors();
   ChangePreferredDataTypes(chosen_types);
   AcknowledgeSyncedTypes();
-  NotifyObservers();
 }
 
 void ProfileSyncService::ChangePreferredDataTypes(
@@ -2224,7 +2203,6 @@ syncer::ModelTypeSet ProfileSyncService::GetEncryptedDataTypes() const {
 }
 
 void ProfileSyncService::OnSyncManagedPrefChange(bool is_sync_managed) {
-  NotifyObservers();
   if (is_sync_managed) {
     DisableForUser();
   } else {
diff --git a/chrome/browser/sync/profile_sync_service.h b/chrome/browser/sync/profile_sync_service.h
index ac43cf8..a8066bf 100644
--- a/chrome/browser/sync/profile_sync_service.h
+++ b/chrome/browser/sync/profile_sync_service.h
@@ -411,7 +411,6 @@ class ProfileSyncService : public ProfileSyncServiceBase,
   virtual void OnDirectoryTypeStatusCounterUpdated(
       syncer::ModelType type,
       const syncer::StatusCounters& counters) OVERRIDE;
-  virtual void OnSyncConfigureRetry() OVERRIDE;
   virtual void OnConnectionStatusChange(
       syncer::ConnectionStatus status) OVERRIDE;
   virtual void OnPassphraseRequired(
@@ -432,7 +431,6 @@ class ProfileSyncService : public ProfileSyncServiceBase,
   // DataTypeManagerObserver implementation.
   virtual void OnConfigureDone(
       const sync_driver::DataTypeManager::ConfigureResult& result) OVERRIDE;
-  virtual void OnConfigureRetry() OVERRIDE;
   virtual void OnConfigureStart() OVERRIDE;
 
   // DataTypeEncryptionHandler implementation.
diff --git a/chrome/browser/sync/profile_sync_service_mock.h b/chrome/browser/sync/profile_sync_service_mock.h
index 53253a2..85f6a3d 100644
--- a/chrome/browser/sync/profile_sync_service_mock.h
+++ b/chrome/browser/sync/profile_sync_service_mock.h
@@ -99,7 +99,6 @@ class ProfileSyncServiceMock : public ProfileSyncService {
   MOCK_METHOD0(OnConfigureBlocked, void());
   MOCK_METHOD1(OnConfigureDone,
                void(const sync_driver::DataTypeManager::ConfigureResult&));
-  MOCK_METHOD0(OnConfigureRetry, void());
   MOCK_METHOD0(OnConfigureStart, void());
 
   MOCK_METHOD0(IsSyncEnabledAndLoggedIn, bool());
diff --git a/chrome/browser/sync/profile_sync_service_observer.h b/chrome/browser/sync/profile_sync_service_observer.h
index 9916c5d..5073ccb8 100644
--- a/chrome/browser/sync/profile_sync_service_observer.h
+++ b/chrome/browser/sync/profile_sync_service_observer.h
@@ -12,9 +12,12 @@ class ProfileSyncServiceObserver {
   // When one of the following events occurs, OnStateChanged() is called.
   // Observers should query the service to determine what happened.
   // - We initialized successfully.
-  // - There was an authentication error and the user needs to reauthenticate.
   // - The sync servers are unavailable at this time.
   // - Credentials are now in flight for authentication.
+  // - The data type configuration has started or ended.
+  // - Sync shut down.
+  // - Sync errors (passphrase, auth, unrecoverable, actionable, etc.).
+  // - Encryption changes.
   virtual void OnStateChanged() = 0;
 
   // If a client wishes to handle sync cycle completed events in a special way,
diff --git a/components/sync_driver/data_type_manager_impl.cc b/components/sync_driver/data_type_manager_impl.cc
index 52b535f..32da8b5 100644
--- a/components/sync_driver/data_type_manager_impl.cc
+++ b/components/sync_driver/data_type_manager_impl.cc
@@ -341,7 +341,6 @@ void DataTypeManagerImpl::ProcessReconfigure() {
 
 void DataTypeManagerImpl::OnDownloadRetry() {
   DCHECK(state_ == DOWNLOAD_PENDING || state_ == CONFIGURING);
-  observer_->OnConfigureRetry();
 }
 
 void DataTypeManagerImpl::DownloadReady(
diff --git a/components/sync_driver/data_type_manager_impl_unittest.cc b/components/sync_driver/data_type_manager_impl_unittest.cc
index c2e32f7..3aed1f28 100644
--- a/components/sync_driver/data_type_manager_impl_unittest.cc
+++ b/components/sync_driver/data_type_manager_impl_unittest.cc
@@ -168,10 +168,6 @@ class FakeDataTypeManagerObserver : public DataTypeManagerObserver {
     done_expectation_ = DataTypeManager::ConfigureResult();
   }
 
-  virtual void OnConfigureRetry() OVERRIDE{
-    // Not verified.
-  }
-
   virtual void OnConfigureStart() OVERRIDE {
     EXPECT_TRUE(start_expected_);
     start_expected_ = false;
diff --git a/components/sync_driver/data_type_manager_observer.h b/components/sync_driver/data_type_manager_observer.h
index 4e7289d..c5081d5 100644
--- a/components/sync_driver/data_type_manager_observer.h
+++ b/components/sync_driver/data_type_manager_observer.h
@@ -15,7 +15,6 @@ class DataTypeManagerObserver {
  public:
   virtual void OnConfigureDone(
       const DataTypeManager::ConfigureResult& result) = 0;
-  virtual void OnConfigureRetry() = 0;
   virtual void OnConfigureStart() = 0;
 
  protected:
diff --git a/components/sync_driver/sync_frontend.h b/components/sync_driver/sync_frontend.h
index fd40b30..774a553 100644
--- a/components/sync_driver/sync_frontend.h
+++ b/components/sync_driver/sync_frontend.h
@@ -54,10 +54,6 @@ class SyncFrontend {
   // The backend queried the server recently and received some updates.
   virtual void OnSyncCycleCompleted() = 0;
 
-  // Configure ran into some kind of error. But it is scheduled to be
-  // retried.
-  virtual void OnSyncConfigureRetry() = 0;
-
   // Informs the frontned of some network event.  These notifications are
   // disabled by default and must be enabled through an explicit request to the
   // SyncBackendHost.
diff --git a/sync/engine/sync_scheduler_impl.cc b/sync/engine/sync_scheduler_impl.cc
index 22dc5e0..d279fd0 100644
--- a/sync/engine/sync_scheduler_impl.cc
+++ b/sync/engine/sync_scheduler_impl.cc
@@ -96,7 +96,6 @@ ConfigurationParams::ConfigurationParams(
       ready_task(ready_task),
       retry_task(retry_task) {
   DCHECK(!ready_task.is_null());
-  DCHECK(!retry_task.is_null());
 }
 ConfigurationParams::~ConfigurationParams() {}
 
diff --git a/sync/internal_api/sync_manager_impl.cc b/sync/internal_api/sync_manager_impl.cc
index 23ac6b2..8ff4843 100644
--- a/sync/internal_api/sync_manager_impl.cc
+++ b/sync/internal_api/sync_manager_impl.cc
@@ -189,7 +189,6 @@ void SyncManagerImpl::ConfigureSyncer(
     const base::Closure& retry_task) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(!ready_task.is_null());
-  DCHECK(!retry_task.is_null());
   DCHECK(initialized_);
 
   DVLOG(1) << "Configuring -"