[Sync] Clean up ProfileSyncService observer events

chrome/browser/sync/profile_sync_service.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/sync/profile_sync_service.h"
 
 #include <cstddef>
 #include <map>
 #include <vector>
 
@@ skipping 530 matching lines @@
 
 bool ProfileSyncService::IsEncryptedDatatypeEnabled() const {
   if (encryption_pending())
     return true;
   const syncer::ModelTypeSet preferred_types = GetPreferredDataTypes();
   const syncer::ModelTypeSet encrypted_types = GetEncryptedDataTypes();
   DCHECK(encrypted_types.Has(syncer::PASSWORDS));
   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,
                     protocol_event_observers_,
                     OnProtocolEvent(event));
 }
 
 void ProfileSyncService::OnDirectoryTypeCommitCounterUpdated(
     syncer::ModelType type,
     const syncer::CommitCounters& counters) {
@@ skipping 117 matching lines @@
           invalidator,
           sync_prefs_.AsWeakPtr(),
           sync_folder));
 
   // Initialize the backend.  Every time we start up a new SyncBackendHost,
   // we'll want to start from a fresh SyncDB, so delete any old one that might
   // be there.
   InitializeBackend(ShouldDeleteSyncFolder());
 
   UpdateFirstSyncTimePref();
-
-  NotifyObservers();

[maniscalco, 2014/09/29 22:01:37]

Just to confirm, this one is redundant because of the call in
PostBackendInitialization, right?

[Nicolas Zea, 2014/09/29 23:37:17]

Right, InitializeBackend will eventually trigger an observer notification.

 }
 
 void ProfileSyncService::OnGetTokenSuccess(
     const OAuth2TokenService::Request* request,
     const std::string& access_token,
     const base::Time& expiration_time) {
   DCHECK_EQ(access_token_request_, request);
   access_token_request_.reset();
   access_token_ = access_token;
   token_receive_time_ = base::Time::Now();
@@ skipping 249 matching lines @@
     const tracked_objects::Location& from_here,
     const std::string& message,
     bool delete_sync_database) {
   DCHECK(HasUnrecoverableError());
   unrecoverable_error_message_ = message;
   unrecoverable_error_location_ = from_here;
 
   UMA_HISTOGRAM_ENUMERATION(kSyncUnrecoverableErrorHistogram,
                             unrecoverable_error_reason_,
                             ERROR_REASON_LIMIT);
-  NotifyObservers();

[maniscalco, 2014/09/29 22:01:37]

Given the comment for OnStateChange about notifying for sync errors, I would
have expected this call to NotifyObservers to remain.  Is it called somewhere
else for sync errors?

[Nicolas Zea, 2014/09/29 23:37:17]

ShutDownImpl (posted just below) will trigger a NotifyObservers call.

   std::string location;
   from_here.Write(true, true, &location);
   LOG(ERROR)
       << "Unrecoverable error detected at " << location
       << " -- ProfileSyncService unusable: " << message;
 
   // Shut all data types down.
   base::MessageLoop::current()->PostTask(FROM_HERE,
       base::Bind(&ProfileSyncService::ShutdownImpl,
                  weak_factory_.GetWeakPtr(),
@@ skipping 582 matching lines @@
       migrator_->state() != browser_sync::BackendMigrator::IDLE) {
     // Migration in progress.  Let the migrator know we just finished
     // configuring something.  It will be up to the migrator to call
     // StartSyncingWithServer() if migration is now finished.
     migrator_->OnConfigureDone(result);
   } else {
     StartSyncingWithServer();
   }
 }
 
-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();
 }
 
 ProfileSyncService::SyncStatusSummary
       ProfileSyncService::QuerySyncStatusSummary() {
   if (HasUnrecoverableError()) {
     return UNRECOVERABLE_ERROR;
   } else if (!backend_) {
@@ skipping 200 matching lines @@
     return;
   }
 
   UpdateSelectedTypesHistogram(sync_everything, chosen_types);
   sync_prefs_.SetKeepEverythingSynced(sync_everything);
 
   if (directory_data_type_manager_.get())
     directory_data_type_manager_->ResetDataTypeErrors();
   ChangePreferredDataTypes(chosen_types);
   AcknowledgeSyncedTypes();
-  NotifyObservers();
 }
 
 void ProfileSyncService::ChangePreferredDataTypes(
     syncer::ModelTypeSet preferred_types) {
 
   DVLOG(1) << "ChangePreferredDataTypes invoked";
   const syncer::ModelTypeSet registered_types = GetRegisteredDataTypes();
   const syncer::ModelTypeSet registered_preferred_types =
       Intersection(registered_types, preferred_types);
   sync_prefs_.SetPreferredDataTypes(registered_types,
@@ skipping 407 matching lines @@
 }
 
 syncer::ModelTypeSet ProfileSyncService::GetEncryptedDataTypes() const {
   DCHECK(encrypted_types_.Has(syncer::PASSWORDS));
   // We may be called during the setup process before we're
   // initialized.  In this case, we default to the sensitive types.
   return encrypted_types_;
 }
 
 void ProfileSyncService::OnSyncManagedPrefChange(bool is_sync_managed) {
-  NotifyObservers();
   if (is_sync_managed) {
     DisableForUser();
   } else {
     // Sync is no longer disabled by policy. Try starting it up if appropriate.
     startup_controller_.TryStart();
   }
 }
 
 void ProfileSyncService::GoogleSigninSucceeded(const std::string& account_id,
                                                const std::string& username,
@@ skipping 484 matching lines @@
                  true));
 }
 
 bool ProfileSyncService::NeedBackup() const {
   return need_backup_;
 }
 
 base::Time ProfileSyncService::GetDeviceBackupTimeForTesting() const {
   return device_info_sync_service_->GetLocalDeviceBackupTime();
 }