sync: don't display one-click if setup is in progress

Also makes ProfileSyncService::SetSetupInProgress(false) do nothing if the value was already false.

BUG=138343
TEST=unit_tests


Review URL: https://chromiumcodereview.appspot.com/10829029

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@148736 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/sync/profile_sync_service.cc

@@ -992,8 +992,9 @@ bool ProfileSyncService::FirstSetupInProgress() const {
 }
 
 void ProfileSyncService::SetSetupInProgress(bool setup_in_progress) {
+  bool was_in_progress = setup_in_progress_;
   setup_in_progress_ = setup_in_progress;
-  if (!setup_in_progress) {
+  if (!setup_in_progress && was_in_progress) {
     if (sync_initialized()) {
       ReconfigureDatatypeManager();
     }

chrome/browser/sync/test_profile_sync_service.h

@@ -124,6 +124,4 @@ class TestProfileSyncService : public ProfileSyncService {
   bool use_real_database_;
 };
 
-
-
 #endif  // CHROME_BROWSER_SYNC_TEST_PROFILE_SYNC_SERVICE_H_

chrome/browser/ui/sync/one_click_signin_helper.cc

@@ -18,6 +18,7 @@
 #include "chrome/browser/signin/signin_manager.h"
 #include "chrome/browser/signin/signin_manager_factory.h"
 #include "chrome/browser/sync/profile_sync_service.h"
+#include "chrome/browser/sync/profile_sync_service_factory.h"
 #include "chrome/browser/tab_contents/confirm_infobar_delegate.h"
 #include "chrome/browser/tab_contents/tab_util.h"
 #include "chrome/browser/ui/browser_finder.h"
@@ -225,6 +226,21 @@ bool OneClickSigninHelper::CanOffer(content::WebContents* web_contents,
 
     if (!manager->GetAuthenticatedUsername().empty())
       return false;
+
+    // If we're about to show a one-click infobar but the user has started
+    // a concurrent signin flow (perhaps via the promo), we may not have yet
+    // established an authenticated username but we still shouldn't move
+    // forward with two simultaneous signin processes.  This is a bit
+    // contentious as the one-click flow is a much smoother flow from the user
+    // perspective, but it's much more difficult to hijack the other flow from
+    // here as it is to bail.
+    ProfileSyncService* service =
+        ProfileSyncServiceFactory::GetForProfile(profile);
+    if (!service)
+      return false;
+
+    if (service->FirstSetupInProgress())
+      return false;
   }
 
   return true;

chrome/browser/ui/sync/one_click_signin_helper_unittest.cc

@@ -7,6 +7,8 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/signin/signin_manager_factory.h"
 #include "chrome/browser/signin/signin_manager_fake.h"
+#include "chrome/browser/sync/profile_sync_service_factory.h"
+#include "chrome/browser/sync/test_profile_sync_service.h"
 #include "chrome/browser/ui/sync/one_click_signin_helper.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/testing_profile.h"
@@ -53,6 +55,42 @@ void OneClickSigninHelperTest::SetUp() {
   // as needed.
 }
 
+class OneClickTestProfileSyncService : public TestProfileSyncService {
+ public:
+  virtual ~OneClickTestProfileSyncService() {}
+
+  // Helper routine to be used in conjunction with
+  // ProfileKeyedServiceFactory::SetTestingFactory().
+  static ProfileKeyedService* Build(Profile* profile) {
+    return new OneClickTestProfileSyncService(profile);
+  }
+
+  // Need to control this for certain tests.
+  virtual bool FirstSetupInProgress() const OVERRIDE {
+    return first_setup_in_progress_;
+  }
+
+  // Controls return value of FirstSetupInProgress. Because some bits
+  // of UI depend on that value, it's useful to control it separately
+  // from the internal work and components that are triggered (such as
+  // ReconfigureDataTypeManager) to facilitate unit tests.
+  void set_first_setup_in_progress(bool in_progress) {
+    first_setup_in_progress_ = in_progress;
+  }
+
+ private:
+  explicit OneClickTestProfileSyncService(Profile* profile)
+      : TestProfileSyncService(NULL,
+                               profile,
+                               NULL,
+                               ProfileSyncService::MANUAL_START,
+                               false,  // synchronous_backend_init
+                               base::Closure()),
+        first_setup_in_progress_(false) {}
+
+  bool first_setup_in_progress_;
+};
+
 content::WebContents* OneClickSigninHelperTest::CreateMockWebContents(
     bool use_incognito,
     const std::string& username) {
@@ -67,7 +105,6 @@ content::WebContents* OneClickSigninHelperTest::CreateMockWebContents(
     signin_manager->StartSignIn(username, std::string(), std::string(),
                                 std::string());
   }
-
   return CreateTestWebContents();
 }
 
@@ -104,6 +141,22 @@ TEST_F(OneClickSigninHelperTest, CanOffer) {
   EXPECT_FALSE(OneClickSigninHelper::CanOffer(web_contents, false));
 }
 
+TEST_F(OneClickSigninHelperTest, CanOfferFirstSetup) {
+  content::WebContents* web_contents = CreateMockWebContents(false, "");
+
+  // Invoke OneClickTestProfileSyncService factory function and grab result.
+  OneClickTestProfileSyncService* sync =
+      static_cast<OneClickTestProfileSyncService*>(
+          ProfileSyncServiceFactory::GetInstance()->SetTestingFactoryAndUse(
+              static_cast<Profile*>(browser_context()),
+              OneClickTestProfileSyncService::Build));
+
+  sync->set_first_setup_in_progress(true);
+
+  EXPECT_FALSE(OneClickSigninHelper::CanOffer(web_contents, true));
+  EXPECT_TRUE(OneClickSigninHelper::CanOffer(web_contents, false));
+}
+
 TEST_F(OneClickSigninHelperTest, CanOfferProfileConnected) {
   content::WebContents* web_contents = CreateMockWebContents(false,
                                                              "foo@gmail.com");