ProfileSyncService: Notify of OnSyncConfigurationCompleted only on success

On a configuration failure, PSS is temporarily in an inconsistent state
until the error is processed (by calling OnUnrecoverableErrorImpl). If we
call an observer during that time, and the observer calls
PSS::GetTransportState, then that could trigger a DCHECK.

It's arguable whether calling OnSyncConfigurationCompleted on an error
even makes sense. It looks like none of the clients can do anything useful
in that case anyway. So let's just not call them. If they're interested in
the error, then they should listen for OnStateChanged instead.

Bug: 880080
Change-Id: If4179cd530f3e625bd53de466c059e051b36f42d
Reviewed-on: https://chromium-review.googlesource.com/1210083Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589869}

components/browser_sync/profile_sync_service.cc

@@ -1249,10 +1249,6 @@ void ProfileSyncService::OnConfigureDone(
     sync_configure_start_time_ = base::Time();
   }
 
-  // Notify listeners that configuration is done.
-  for (auto& observer : observers_)
-    observer.OnSyncConfigurationCompleted(this);
-
   DVLOG(1) << "PSS OnConfigureDone called with status: " << result.status;
   // The possible status values:
   //    ABORT - Configuration was aborted. This is not an error, if
@@ -1300,6 +1296,10 @@ void ProfileSyncService::OnConfigureDone(
   // enabled, and yet we still think we require a passphrase for decryption.
   DCHECK(!IsPassphraseRequiredForDecryption() || IsEncryptedDatatypeEnabled());
 
+  // Notify listeners that configuration is done.
+  for (auto& observer : observers_)
+    observer.OnSyncConfigurationCompleted(this);
+
   // This must be done before we start syncing with the server to avoid
   // sending unencrypted data up on a first time sync.
   if (crypto_->encryption_pending())