Sync: Support multiple setup UIs.

components/sync_driver/sync_service.h

Index: components/sync_driver/sync_service.h
diff --git a/components/sync_driver/sync_service.h b/components/sync_driver/sync_service.h
index 4ca8b42e870f5ea9939a74acd6d0cd43f8746cdd..95f191014847c6d6673b482d93a06cbf7fbe7d2e 100644
--- a/components/sync_driver/sync_service.h
+++ b/components/sync_driver/sync_service.h
@@ -8,7 +8,7 @@
 #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"
@@ -20,6 +20,7 @@
 #include "sync/internal_api/public/connection_status.h"
 
 class GoogleServiceAuthError;
+class ProfileSyncService;
 
 namespace browser_sync {
 class ProtocolEventObserver;
@@ -46,6 +47,19 @@ 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.
@@ -67,7 +81,6 @@ class SyncService : public DataTypeEncryptionHandler {
   // 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;
@@ -176,12 +189,13 @@ class SyncService : public DataTypeEncryptionHandler {
   // 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;