Sync: Support multiple setup UIs.

components/sync_driver/sync_service.h

 // Copyright 2015 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.
 
 #ifndef COMPONENTS_SYNC_DRIVER_SYNC_SERVICE_H_
 #define COMPONENTS_SYNC_DRIVER_SYNC_SERVICE_H_
 
 #include <memory>
 #include <string>
 
-#include "base/callback_forward.h"
+#include "base/callback.h"
 #include "base/location.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/time/time.h"
 #include "components/sync_driver/data_type_encryption_handler.h"
 #include "components/sync_driver/sync_service_observer.h"
 #include "google_apis/gaia/google_service_auth_error.h"
 #include "sync/internal_api/public/base/model_type.h"
 #include "sync/internal_api/public/connection_status.h"
 
 class GoogleServiceAuthError;
+class ProfileSyncService;
 
 namespace browser_sync {
 class ProtocolEventObserver;
 }
 
 namespace syncer {
 
 class BaseTransaction;
 class JsController;
 class TypeDebugInfoObserver;
 struct SyncStatus;
 struct UserShare;
 
 namespace sessions {
 class SyncSessionSnapshot;
 }  // namespace sessions
 
 }  // namespace syncer
 
 namespace sync_driver {
 
 class DataTypeController;
 class LocalDeviceInfoProvider;
 class OpenTabsUIDelegate;
 class SyncClient;
 
+// UIs that need to prevent Sync startup should hold an instance of this class
+// until the user has finished modifying sync settings.
+class SyncSetupInProgressHandle {
+ public:
+  ~SyncSetupInProgressHandle();
+
+ private:
+  friend class ::ProfileSyncService;

[tommycli, 2016/06/10 00:04:16]

I added a friend here, since I wanted to ensure that readers knew that they
weren't supposed to construct this class directly.

I did not make this an inner class of SyncService because that makes forward
declarations impossible.

[maxbogue, 2016/06/10 17:28:46]

I think I would rather just have the constructor be public and have a comment
about not constructing it directly than have this weird reference to PSS in
sync_driver. Could you also mention the reasoning for not making it an inner
class in its class comment?

[tommycli, 2016/06/10 18:58:20]

Done.

+  explicit SyncSetupInProgressHandle(base::Closure on_destroy);
+
+  base::Closure on_destroy_;
+};
+
 class SyncService : public DataTypeEncryptionHandler {
  public:
   // Used to specify the kind of passphrase with which sync data is encrypted.
   enum PassphraseType {
     IMPLICIT,  // The user did not provide a custom passphrase for encryption.
                // We implicitly use the GAIA password in such cases.
     EXPLICIT,  // The user selected the "use custom passphrase" radio button
                // during sync setup and provided a passphrase.
   };
 
   // Passed as an argument to RequestStop to control whether or not the sync
   // backend should clear its data directory when it shuts down. See
   // RequestStop for more information.
   enum SyncStopDataFate {
     KEEP_DATA,
     CLEAR_DATA,
   };
 
   // Status of sync server connection, sync token and token request.
   struct SyncTokenStatus {
     SyncTokenStatus();
-    ~SyncTokenStatus();
 
     // Sync server connection status reported by sync backend.
     base::Time connection_status_update_time;
     syncer::ConnectionStatus connection_status;
 
     // Times when OAuth2 access token is requested and received.
     base::Time token_request_time;
     base::Time token_receive_time;
 
     // Error returned by OAuth2TokenService for token request and time when
@@ skipping 88 matching lines @@
 
   // Returns true if initial sync setup is in progress (does not return true
   // if the user is customizing sync after already completing setup once).
   // SyncService uses this to determine if it's OK to start syncing, or if the
   // user is still setting up the initial sync configuration.
   virtual bool IsFirstSetupInProgress() const = 0;
 
   // Called by the UI to notify the SyncService that UI is visible so it will
   // not start syncing. This tells sync whether it's safe to start downloading
   // data types yet (we don't start syncing until after sync setup is complete).
-  // The UI calls this as soon as any part of the signin wizard is displayed
-  // (even just the login UI).
-  // If |setup_in_progress| is false, this also kicks the sync engine to ensure
-  // that data download starts. In this case, |ReconfigureDatatypeManager| will
-  // get triggered.
-  virtual void SetSetupInProgress(bool setup_in_progress) = 0;
+  // The UI calls this and holds onto the instance for as long as any part of
+  // the signin wizard is displayed (even just the login UI).
+  // When the last outstanding handle is deleted, this kicks off the sync engine
+  // to ensure that data download starts. In this case,
+  // |ReconfigureDatatypeManager| will get triggered.
+  virtual std::unique_ptr<SyncSetupInProgressHandle>
+  GetSetupInProgressHandle() = 0;
 
   // Used by tests.
   virtual bool IsSetupInProgress() const = 0;
 
   // Whether the data types active for the current mode have finished
   // configuration.
   virtual bool ConfigurationDone() const = 0;
 
   virtual const GoogleServiceAuthError& GetAuthError() const = 0;
   virtual bool HasUnrecoverableError() const = 0;
@@ skipping 132 matching lines @@
  protected:
   SyncService() {}
 
  private:
   DISALLOW_COPY_AND_ASSIGN(SyncService);
 };
 
 }  // namespace sync_driver
 
 #endif  // COMPONENTS_SYNC_DRIVER_SYNC_SERVICE_H_