[Password Manager] Setup experiment to restrict autofilling of sync credential

chrome/browser/password_manager/chrome_password_manager_client.cc

Index: chrome/browser/password_manager/chrome_password_manager_client.cc
diff --git a/chrome/browser/password_manager/chrome_password_manager_client.cc b/chrome/browser/password_manager/chrome_password_manager_client.cc
index df029aee357b8dec1ca6818c1e06058f092054f7..88d347a6df076b92b2b92a8dd5b0e4f48bed7a91 100644
--- a/chrome/browser/password_manager/chrome_password_manager_client.cc
+++ b/chrome/browser/password_manager/chrome_password_manager_client.cc
@@ -8,6 +8,7 @@
 #include "base/command_line.h"
 #include "base/memory/singleton.h"
 #include "base/metrics/histogram.h"
+#include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/password_manager/password_manager_util.h"
 #include "chrome/browser/password_manager/password_store_factory.h"
 #include "chrome/browser/password_manager/save_password_infobar_delegate.h"
@@ -33,6 +34,8 @@
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/web_contents.h"
+#include "google_apis/gaia/gaia_urls.h"
+#include "net/base/url_util.h"
 
 #if defined(OS_ANDROID)
 #include "chrome/browser/android/password_authentication_manager.h"
@@ -63,11 +66,13 @@ ChromePasswordManagerClient::ChromePasswordManagerClient(
       driver_(web_contents, this, autofill_client),
       observer_(NULL),
       weak_factory_(this),
-      can_use_log_router_(false) {
+      can_use_log_router_(false),
+      autofill_state_(ALLOW_SYNC_CREDENTIALS){
   PasswordManagerInternalsService* service =
       PasswordManagerInternalsServiceFactory::GetForBrowserContext(profile_);
   if (service)
     can_use_log_router_ = service->RegisterClient(this);
+  SetupAutofillSyncState();
 }
 
 ChromePasswordManagerClient::~ChromePasswordManagerClient() {
@@ -102,6 +107,22 @@ bool ChromePasswordManagerClient::IsPasswordManagerEnabledForCurrentPage()
   return entry->GetURL().host() != chrome::kChromeUIChromeSigninHost;
 }
 
+bool ChromePasswordManagerClient::ShouldFilterAutofillResult(
+    const autofill::PasswordForm& form) const {
+  if (!password_manager_sync_metrics::IsSyncAccountCredential(
+          profile_, base::UTF16ToUTF8(form.username_value), form.signon_realm))
+    return false;
+
+  if (autofill_state_ == DISALLOW_SYNC_CREDENTIALS)
+    return true;
+
+  if (autofill_state_ == DISALLOW_SYNC_CREDENTIALS_FOR_REAUTH &&
+      LastLoadWasTransactionalReauthPage())
+    return true;
+
+  return false;
+}
+
 bool ChromePasswordManagerClient::IsSyncAccountCredential(
     const std::string& username, const std::string& origin) const {
   return password_manager_sync_metrics::IsSyncAccountCredential(
@@ -353,6 +374,23 @@ void ChromePasswordManagerClient::CommitFillPasswordForm(
   driver_.FillPasswordForm(*data);
 }
 
+bool ChromePasswordManagerClient::LastLoadWasTransactionalReauthPage() const {
+  DCHECK(web_contents());
+  content::NavigationEntry* entry =
+      web_contents()->GetController().GetLastCommittedEntry();
+  if (!entry)
+    return false;
+
+  if (entry->GetURL().GetOrigin() !=
+      GaiaUrls::GetInstance()->gaia_url().GetOrigin())
+    return false;
+
+  std::string ignored_value;
+  return net::GetValueForKeyInQuery(entry->GetURL(),
+                                    "rart",

[Ilya Sherman, 2014/08/12 02:17:32]

What is "rart"?  Probably worth a comment, IMO.

[Garrett Casto, 2014/08/13 20:34:40]

Done.

+                                    &ignored_value);
+}
+
 bool ChromePasswordManagerClient::IsTheHotNewBubbleUIEnabled() {
 #if !defined(USE_AURA)
   return false;
@@ -386,3 +424,24 @@ bool ChromePasswordManagerClient::EnabledForSyncSignin() {
       base::FieldTrialList::FindFullName("PasswordManagerStateForSyncSignin");
   return group_name != "Disabled";
 }
+
+void ChromePasswordManagerClient::SetupAutofillSyncState() {
+  std::string group_name =
+      base::FieldTrialList::FindFullName("AutofillSyncCredential");
+
+  CommandLine* command_line = CommandLine::ForCurrentProcess();
+  if (command_line->HasSwitch(
+          password_manager::switches::kAllowAutofillSyncCredential)) {
+    autofill_state_ = ALLOW_SYNC_CREDENTIALS;
+    return;
+  }

[Ilya Sherman, 2014/08/12 02:17:32]

Hmm, what about the other switch values?

[Garrett Casto, 2014/08/13 20:34:40]

Fixed, with tests to verify.

+
+  if (group_name == "DisallowSyncCredentialsForReauth") {
+    autofill_state_ = DISALLOW_SYNC_CREDENTIALS_FOR_REAUTH;
+  } else if (group_name == "DisallowSyncCredentials") {
+      autofill_state_ = DISALLOW_SYNC_CREDENTIALS;
+  } else {
+    // Allow by default.
+    autofill_state_ = ALLOW_SYNC_CREDENTIALS;
+  }

[Ilya Sherman, 2014/08/12 02:17:32]

I recall seeing a message on the finch-team mailing list indicating that using
finch params might be preferable to using group names.  However, I'm not
completely up to speed on the latest thinking here.  You might want to check
with one of the Finch crew.

[Garrett Casto, 2014/08/13 20:34:40]

Sent an e-mail out asking. It doesn't seem like it's useful to me, but I could
be wrong.

[jww, 2014/08/14 01:16:25]

The explanation I've gotten in the past is that if you have continuous values,
rather than a clearly defined set of binary choices, parameters give you the
advantage of changing the experiment on the fly. Instead of checking the group
name, you'd check your parameter and use its value and completely ignore the
group name. Then, if you want to change the experiment later, you can just add a
new group with a new parameter. See a recent experiment that I committed for an
example: https://codereview.chromium.org/369703002/

In this case, it seems like there's only a small set of binary choices with no
chance of wanting/needing to change that later, so I suspect this is the right
choice.

+}