Add shutdown notification and service refs to SyncServiceObserver.

components/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 "components/browser_sync/profile_sync_service.h"
 
 #include <stddef.h>
 
 #include <cstddef>
 #include <map>
@@ skipping 695 matching lines @@
   if (HasSyncingEngine()) {
     RequestAccessToken();
   } else {
     startup_controller_->TryStart();
   }
 }
 
 void ProfileSyncService::Shutdown() {
   DCHECK(thread_checker_.CalledOnValidThread());
   UnregisterAuthNotifications();
+  NotifyShutdown();

[skym, 2017/01/26 17:00:37]

Having this right before ShutdownImpl(...), which does call NotifyObservers()
worries me a little bit. While I'm suspicious this doesn't break any of the
existing observer's functionality, it does kind of go against the documentation
in sync_service_observer.h.

I think I'd prefer it if you swapped these two, unless there's a reason for this
ordering.

[Steven Holte, 2017/01/26 20:31:47]

Done.

 
   ShutdownImpl(syncer::BROWSER_SHUTDOWN);
   if (sync_error_controller_) {
     // Destroy the SyncErrorController when the service shuts down for good.
     RemoveObserver(sync_error_controller_.get());

[skym, 2017/01/26 17:00:37]

Ugh. I guess this could be done with the OnSyncShutdown() event now. Not sure
why it wasn't originally done in the SyncErrorController's dtor. Not sure which
is better. I don't really understand why we have to manually reset it if we're
going to shutdown anyways. Regardless, out of scope of your CL.

[Steven Holte, 2017/01/26 20:31:47]

Acknowledged.

     sync_error_controller_.reset();
   }
 
   if (sync_thread_)
     sync_thread_->Stop();
 }
 
 void ProfileSyncService::ShutdownImpl(syncer::ShutdownReason reason) {
   if (!engine_) {
     if (reason == syncer::ShutdownReason::DISABLE_SYNC && sync_thread_) {
@@ skipping 116 matching lines @@
 void ProfileSyncService::NotifySyncCycleCompleted() {
   for (auto& observer : observers_)
     observer.OnSyncCycleCompleted();
 }
 
 void ProfileSyncService::NotifyForeignSessionUpdated() {
   for (auto& observer : observers_)
     observer.OnForeignSessionUpdated();
 }
 
+void ProfileSyncService::NotifyShutdown() {
+  for (auto& observer : observers_)
+    observer.OnSyncShutdown(this);
+}
+
 void ProfileSyncService::ClearStaleErrors() {
   ClearUnrecoverableError();
   last_actionable_error_ = SyncProtocolError();
   // Clear the data type errors as well.
   if (data_type_manager_.get())
     data_type_manager_->ResetDataTypeErrors();
 }
 
 void ProfileSyncService::ClearUnrecoverableError() {
   unrecoverable_error_reason_ = ERROR_REASON_UNSET;
@@ skipping 499 matching lines @@
         UMA_HISTOGRAM_LONG_TIMES("Sync.ServiceInitialConfigureTime", delta);
       } else {
         UMA_HISTOGRAM_LONG_TIMES("Sync.ServiceSubsequentConfigureTime", delta);
       }
     }
     sync_configure_start_time_ = base::Time();
   }
 
   // Notify listeners that configuration is done.
   for (auto& observer : observers_)
-    observer.OnSyncConfigurationCompleted();
+    observer.OnSyncConfigurationCompleted(this);
 
   DVLOG(1) << "PSS OnConfigureDone called with status: " << configure_status_;
   // The possible status values:
   //    ABORT - Configuration was aborted. This is not an error, if
   //            initiated by user.
   //    OK - Some or all types succeeded.
   //    Everything else is an UnrecoverableError. So treat it as such.
 
   // First handle the abort case.
   if (configure_status_ == DataTypeManager::ABORTED &&
@@ skipping 1248 matching lines @@
 
   DCHECK(startup_controller_->IsSetupInProgress());
   startup_controller_->SetSetupInProgress(false);
 
   if (IsEngineInitialized())
     ReconfigureDatatypeManager();
   NotifyObservers();
 }
 
 }  // namespace browser_sync