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..f1594facf35146ef959721195c3759966b4e7a87 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"
@@ -46,6 +46,15 @@ class LocalDeviceInfoProvider;
 class OpenTabsUIDelegate;
 class SyncClient;
 
+class SyncSetupInProgressHandle {

[maxbogue, 2016/06/09 20:04:14]

Comment explaining the purpose of this class?

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

Done.

+ public:
+  explicit SyncSetupInProgressHandle(base::Closure on_destroy);
+  ~SyncSetupInProgressHandle();
+
+ private:
+  base::Closure on_destroy_;
+};
+
 class SyncService : public DataTypeEncryptionHandler {
  public:
   // Used to specify the kind of passphrase with which sync data is encrypted.
@@ -67,7 +76,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 +184,12 @@ 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.
+  std::unique_ptr<SyncSetupInProgressHandle> GetSetupInProgressHandle();

[maxbogue, 2016/06/09 20:04:14]

Why is the implementation of this on this class and not ProfileSyncService? That
would let you get rid of the SetSetupInProgress method entirely, and move the
counter to PSS as well.

The split between this class and PSS is pretty weird right now, but if we're
gonna move things here I'd rather it be a separate process.

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

Done.

 
   // Used by tests.
   virtual bool IsSetupInProgress() const = 0;
@@ -325,9 +333,19 @@ class SyncService : public DataTypeEncryptionHandler {
           callback) = 0;
 
  protected:
-  SyncService() {}
+  friend class SyncSetupInProgressHandle;

[maxbogue, 2016/06/09 20:04:13]

This shouldn't be necessary should it? That class doesn't actually require
direct access to this one.

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

Done.

+
+  SyncService() : outstanding_setup_in_progress_handles_(0) {}

[maxbogue, 2016/06/09 20:04:13]

You can just use = 0 on the declaration below now.

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

Done.

+
+  // Called internally by the GetSetupInProgressHandle mechanism.
+  // This should not be called directly (except by the Android wrapper).

[maxbogue, 2016/06/09 20:04:14]

What Android wrapper?

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

Done. Method removed.

+  virtual void SetSetupInProgress(bool setup_in_progress) = 0;
+
+  int outstanding_setup_in_progress_handles_;

[maxbogue, 2016/06/09 20:04:13]

Comment please.

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

Done.

 
  private:
+  void OnSetupInProgressHandleDestroyed();
+
   DISALLOW_COPY_AND_ASSIGN(SyncService);
 };