Address bug where the one-click sign-in bar would never show again once

a user (with associated profile) had clicked "Cancel". New behavior
is that we now remember for which email the bar was dismissed, and show
it again in case the user signs in with a different email. If the user
accepts the one-click sign-in at one point, the bar never shows again
for this profile

BUG=118280
TEST=Cancel the one-click sign-in bar, log-in with another Google account,
see that it still shows up.

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

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

chrome/browser/password_manager/password_form_manager.h

@@ -104,6 +104,11 @@ class PasswordFormManager : public PasswordStoreConsumer {
   void SubmitPassed();
   void SubmitFailed();
 
+  // Return the username associated with the credentials.
+  const string16& associated_username() const {
+    return pending_credentials_.username_value;
+  }
+
   // Returns the realm URL for the form managed my this manager.
   const std::string& realm() const {
     return pending_credentials_.signon_realm;

chrome/browser/password_manager/password_manager_delegate_impl.cc

@@ -6,6 +6,7 @@
 
 #include "base/memory/singleton.h"
 #include "base/metrics/histogram.h"
+#include "base/utf_string_conversions.h"
 #include "chrome/browser/autofill/autofill_manager.h"
 #include "chrome/browser/infobars/infobar_tab_helper.h"
 #include "chrome/browser/password_manager/password_form_manager.h"
@@ -139,8 +140,12 @@ void PasswordManagerDelegateImpl::AddSavePasswordInfoBarIfPermitted(
   // For now, one-click signin is fully implemented only on windows.
 #if defined(ENABLE_ONE_CLICK_SIGNIN)
   GURL realm(form_to_save->realm());
-  if (realm == GURL(GaiaUrls::GetInstance()->gaia_login_form_realm()) &&
-      OneClickSigninHelper::CanOffer(tab_contents_->web_contents(), true)) {
+  // TODO(mathp): Checking only against associated_username() causes a bug
+  // referenced here: crbug.com/133275
+  if ((realm == GURL(GaiaUrls::GetInstance()->gaia_login_form_realm()) ||
+      realm == GURL("https://www.google.com/")) &&
+      OneClickSigninHelper::CanOffer(tab_contents_->web_contents(),
+          UTF16ToUTF8(form_to_save->associated_username()), true)) {
     return;
   }
 #endif

chrome/browser/signin/signin_manager_factory.cc

@@ -36,6 +36,8 @@ void SigninManagerFactory::RegisterUserPrefs(PrefService* user_prefs) {
                                   PrefService::UNSYNCABLE_PREF);
   user_prefs->RegisterBooleanPref(prefs::kReverseAutologinEnabled, true,
                                   PrefService::UNSYNCABLE_PREF);
+  user_prefs->RegisterListPref(prefs::kReverseAutologinRejectedEmailList,
+                               new ListValue, PrefService::UNSYNCABLE_PREF);
   user_prefs->RegisterBooleanPref(prefs::kIsGooglePlusUser, false,
                                  PrefService::UNSYNCABLE_PREF);
 }

chrome/browser/ui/sync/one_click_signin_helper.cc

@@ -12,13 +12,13 @@
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/infobars/infobar_tab_helper.h"
 #include "chrome/browser/prefs/pref_service.h"
+#include "chrome/browser/prefs/scoped_user_pref_update.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_info_cache.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #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"
@@ -72,6 +72,10 @@ class OneClickLoginInfoBarDelegate : public ConfirmInfoBarDelegate {
   // show again in this profile.
   void DisableOneClickSignIn();
 
+  // Add a specific email to the list of emails rejected for one-click
+  // sign-in, for this profile.
+  void AddEmailToOneClickRejectedList(const std::string& email);
+
   // Record the specified action in the histogram for one-click sign in.
   void RecordHistogramAction(int action);
 
@@ -146,6 +150,8 @@ void StartSync(content::WebContents* web_contents,
 }  // namespace
 
 bool OneClickLoginInfoBarDelegate::Accept() {
+  // User has accepted one-click sign-in for this account. Never ask again for
+  // this profile.
   DisableOneClickSignIn();
   content::WebContents* web_contents = owner()->web_contents();
   RecordHistogramAction(one_click_signin::HISTOGRAM_ACCEPTED);
@@ -157,7 +163,7 @@ bool OneClickLoginInfoBarDelegate::Accept() {
 }
 
 bool OneClickLoginInfoBarDelegate::Cancel() {
-  DisableOneClickSignIn();
+  AddEmailToOneClickRejectedList(email_);
   RecordHistogramAction(one_click_signin::HISTOGRAM_REJECTED);
   button_pressed_ = true;
   return true;
@@ -190,6 +196,16 @@ void OneClickLoginInfoBarDelegate::DisableOneClickSignIn() {
   pref_service->SetBoolean(prefs::kReverseAutologinEnabled, false);
 }
 
+void OneClickLoginInfoBarDelegate::AddEmailToOneClickRejectedList(
+    const std::string& email) {
+  PrefService* pref_service =
+      TabContents::FromWebContents(owner()->web_contents())->
+          profile()->GetPrefs();
+  ListPrefUpdate updater(pref_service,
+                         prefs::kReverseAutologinRejectedEmailList);
+  updater->AppendIfNotPresent(base::Value::CreateStringValue(email));
+}
+
 void OneClickLoginInfoBarDelegate::RecordHistogramAction(int action) {
   UMA_HISTOGRAM_ENUMERATION("AutoLogin.Reverse", action,
                             one_click_signin::HISTOGRAM_MAX);
@@ -197,6 +213,7 @@ void OneClickLoginInfoBarDelegate::RecordHistogramAction(int action) {
 
 // static
 bool OneClickSigninHelper::CanOffer(content::WebContents* web_contents,
+                                    const std::string& email,
                                     bool check_connected) {
   if (!web_contents)
     return false;
@@ -227,20 +244,37 @@ 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)
+    // Make sure this username is not prohibited by policy.
+    if (!manager->IsAllowedUsername(email))
       return false;
 
-    if (service->FirstSetupInProgress())
-      return false;
+    // If some profile, not just the current one, is already connected to this
+    // account, don't show the infobar.
+    if (g_browser_process) {
+      ProfileManager* manager = g_browser_process->profile_manager();
+      if (manager) {
+        string16 email16 = UTF8ToUTF16(email);
+        ProfileInfoCache& cache = manager->GetProfileInfoCache();
+
+        for (size_t i = 0; i < cache.GetNumberOfProfiles(); ++i) {
+          if (email16 == cache.GetUserNameOfProfileAtIndex(i))
+            return false;
+        }
+      }
+    }
+
+    // If email was already rejected by this profile for one-click sign-in.
+    if (!email.empty()) {
+      const ListValue* rejected_emails = profile->GetPrefs()->GetList(
+          prefs::kReverseAutologinRejectedEmailList);
+      if (!rejected_emails->empty()) {
+        const Value* email_value = Value::CreateStringValue(email);
+        ListValue::const_iterator iter = rejected_emails->Find(
+                *email_value);
+        if (iter != rejected_emails->end())
+          return false;
+      }
+    }
   }
 
   return true;
@@ -298,29 +332,10 @@ void OneClickSigninHelper::ShowInfoBarUIThread(
 
   content::WebContents* web_contents = tab_util::GetWebContentsByID(child_id,
                                                                     route_id);
-  if (!web_contents || !CanOffer(web_contents, true))
-    return;
-
-  // If some profile, not just the current one, is already connected to this
-  // account, don't show the infobar.
-  if (g_browser_process) {
-    ProfileManager* manager = g_browser_process->profile_manager();
-    if (manager) {
-      string16 email16 = UTF8ToUTF16(email);
-      ProfileInfoCache& cache = manager->GetProfileInfoCache();
-
-      for (size_t i = 0; i < cache.GetNumberOfProfiles(); ++i) {
-        if (email16 == cache.GetUserNameOfProfileAtIndex(i))
-          return;
-      }
-    }
-  }
 
-  // Make sure this username is not prohibited by policy.
-  Profile* profile =
-      Profile::FromBrowserContext(web_contents->GetBrowserContext());
-  SigninManager* signin = SigninManagerFactory::GetForProfile(profile);
-  if (!signin->IsAllowedUsername(email))
+  // TODO(mathp): The appearance of this infobar should be tested using a
+  // browser_test.
+  if (!web_contents || !CanOffer(web_contents, email, true))
     return;
 
   TabContents* tab_contents = TabContents::FromWebContents(web_contents);

chrome/browser/ui/sync/one_click_signin_helper.h

@@ -27,9 +27,11 @@ class OneClickSigninHelper : public content::WebContentsObserver {
   // Returns true if the one-click signin feature can be offered at this time.
   // It can be offered if the contents is not in an incognito window.  If
   // |check_connected| is true, then the profile is checked to see if it's
-  // already connected to a google account, in which case a one click signin
+  // already connected to a google account or if the user has already rejected
+  // one-click sign-in with this email, in which cases a one click signin
   // should not be offered.
   static bool CanOffer(content::WebContents* web_contents,
+                       const std::string& email,
                        bool check_connected);
 
   // Looks for the Google-Accounts-SignIn response header, and if found,

chrome/browser/ui/tab_contents/tab_contents.cc

@@ -170,7 +170,7 @@ TabContents::TabContents(WebContents* contents)
   // one-click signin helper attached does not cause problems if the profile
   // happens to be already connected.
 #if defined(ENABLE_ONE_CLICK_SIGNIN)
-  if (OneClickSigninHelper::CanOffer(contents, false))
+  if (OneClickSigninHelper::CanOffer(contents, "", false))
       one_click_signin_helper_.reset(new OneClickSigninHelper(contents));
 #endif
 }

chrome/common/pref_names.cc

@@ -314,6 +314,11 @@ const char kPasswordGenerationEnabled[] = "password_generation.enabled";
 const char kAutologinEnabled[] = "autologin.enabled";
 const char kReverseAutologinEnabled[] = "reverse_autologin.enabled";
 
+// List to keep track of emails for which the user has rejected one-click
+// sign-in.
+const char kReverseAutologinRejectedEmailList[] =
+    "reverse_autologin.rejected_email_list";
+
 // Boolean that is true when SafeBrowsing is enabled.
 const char kSafeBrowsingEnabled[] = "safebrowsing.enabled";
 

chrome/common/pref_names.h

@@ -139,6 +139,7 @@ extern const char kPasswordManagerAllowShowPasswords[];
 extern const char kPasswordGenerationEnabled[];
 extern const char kAutologinEnabled[];
 extern const char kReverseAutologinEnabled[];
+extern const char kReverseAutologinRejectedEmailList[];
 extern const char kSafeBrowsingEnabled[];
 extern const char kSafeBrowsingReportingEnabled[];
 extern const char kSafeBrowsingProceedAnywayDisabled[];