[Password Manager] Remove PSL matching for non-HTML forms

chrome/browser/password_manager/native_backend_gnome_x_unittest.cc

Index: chrome/browser/password_manager/native_backend_gnome_x_unittest.cc
diff --git a/chrome/browser/password_manager/native_backend_gnome_x_unittest.cc b/chrome/browser/password_manager/native_backend_gnome_x_unittest.cc
index 13905592a384c9e30992b1ea3206658cee8d84d5..726ade03a6d67adebabf67075a6d01dd82e19c7d 100644
--- a/chrome/browser/password_manager/native_backend_gnome_x_unittest.cc
+++ b/chrome/browser/password_manager/native_backend_gnome_x_unittest.cc
@@ -324,6 +324,12 @@ class NativeBackendGnomeTest : public testing::Test {
     form_isc_.password_value = UTF8ToUTF16("ihazabukkit");
     form_isc_.submit_element = UTF8ToUTF16("login");
     form_isc_.signon_realm = "http://www.isc.org/";
+
+    basic_auth_.origin = GURL("http://www.example.com/");
+    basic_auth_.username_value = UTF8ToUTF16("username");
+    basic_auth_.password_value = UTF8ToUTF16("pass");
+    basic_auth_.signon_realm = "http://www.example.com/Realm";
+    basic_auth_.scheme = PasswordForm::SCHEME_BASIC;
   }
 
   virtual void TearDown() {
@@ -407,6 +413,7 @@ class NativeBackendGnomeTest : public testing::Test {
   // derived from |credentials|.)
   bool CheckCredentialAvailability(const PasswordForm& credentials,
                                    const GURL& url,
+                                   const std::string& realm,

[vabr (Chromium), 2014/05/07 07:57:38]

Please explain the meaning of |realm| in the comment above. In particular the
connection to the SCHEME_BASIC is not obvious from its name.

[Garrett Casto, 2014/05/08 20:01:20]

Done.

                                    PasswordForm* result) {
     NativeBackendGnome backend(321);
     backend.Init();
@@ -421,6 +428,10 @@ class NativeBackendGnomeTest : public testing::Test {
     PasswordForm target_form;
     target_form.origin = url;
     target_form.signon_realm = url.spec();
+    if (!realm.empty()) {
+      target_form.signon_realm.append(realm);
+      target_form.scheme = PasswordForm::SCHEME_BASIC;
+    }
     std::vector<PasswordForm*> form_list;
     BrowserThread::PostTask(
         BrowserThread::DB,
@@ -435,6 +446,7 @@ class NativeBackendGnomeTest : public testing::Test {
     EXPECT_EQ(1u, mock_keyring_items.size());
     if (mock_keyring_items.size() > 0)
       CheckMockKeyringItem(&mock_keyring_items[0], credentials, "chrome-321");
+    mock_keyring_items.clear();
 
     if (form_list.empty())
       return false;
@@ -572,6 +584,7 @@ class NativeBackendGnomeTest : public testing::Test {
   PasswordForm form_google_;
   PasswordForm form_facebook_;
   PasswordForm form_isc_;
+  PasswordForm basic_auth_;
 };
 
 TEST_F(NativeBackendGnomeTest, BasicAddLogin) {
@@ -623,7 +636,8 @@ TEST_F(NativeBackendGnomeTest, PSLMatchingPositive) {
   const GURL kMobileURL("http://m.facebook.com/");
   password_manager::PSLMatchingHelper helper;
   ASSERT_TRUE(helper.IsMatchingEnabled());
-  EXPECT_TRUE(CheckCredentialAvailability(form_facebook_, kMobileURL, &result));
+  EXPECT_TRUE(CheckCredentialAvailability(
+      form_facebook_, kMobileURL, "", &result));
   EXPECT_EQ(kMobileURL, result.origin);
   EXPECT_EQ(kMobileURL.spec(), result.signon_realm);
 }
@@ -634,7 +648,7 @@ TEST_F(NativeBackendGnomeTest, PSLMatchingNegativeDomainMismatch) {
   password_manager::PSLMatchingHelper helper;
   ASSERT_TRUE(helper.IsMatchingEnabled());
   EXPECT_FALSE(CheckCredentialAvailability(
-      form_facebook_, GURL("http://m-facebook.com/"), NULL));
+      form_facebook_, GURL("http://m-facebook.com/"), "", NULL));
 }
 
 // Test PSL matching is off for domains excluded from it.
@@ -642,7 +656,22 @@ TEST_F(NativeBackendGnomeTest, PSLMatchingDisabledDomains) {
   password_manager::PSLMatchingHelper helper;
   ASSERT_TRUE(helper.IsMatchingEnabled());
   EXPECT_FALSE(CheckCredentialAvailability(
-      form_google_, GURL("http://one.google.com/"), NULL));
+      form_google_, GURL("http://one.google.com/"), "", NULL));
+}
+
+// Make sure PSL matches aren't available for non-HTML forms.
+TEST_F(NativeBackendGnomeTest, PSLMatchingDisabledForNonHTMLForms) {

[vabr (Chromium), 2014/05/07 07:57:38]

The code change applies to all possible schemes, but the tests only care about
SCHEME_BASIC and SCHEME_HTML.

I'm not sure if it's worth the effort to add also tests for SCHEME_DIGEST and
SCHEME_OTHER. Here it would be an easy copy-paste of the added test. For the
LoginDatabase it would be probably longer.

My vote would be to cover all schemes with tests, but I don't insist on it if
you believe it's not worth it.

[Garrett Casto, 2014/05/08 20:01:20]

Done.

+  password_manager::PSLMatchingHelper helper;
+  ASSERT_TRUE(helper.IsMatchingEnabled());
+  // Don't match basic auth form with an HTML form.
+  EXPECT_FALSE(CheckCredentialAvailability(
+      basic_auth_, GURL("http://www.example.com"), "", NULL));
+  // Don't match an HTML form with a basic auth form.
+  EXPECT_FALSE(CheckCredentialAvailability(
+      form_google_, GURL("http://www.google.com/"), "Realm", NULL));
+  // Don't match two different basic auth forms with different origin.
+  EXPECT_FALSE(CheckCredentialAvailability(
+      basic_auth_, GURL("http://first.example.com"), "Realm", NULL));
 }
 
 TEST_F(NativeBackendGnomeTest, PSLUpdatingStrictUpdateLogin) {