[Merge] [PasswordManager] Do not save change password forms

This is a merge of https://crrev.com/312179, approved in http://crbug.com/447558#c11.

Original description:
********************************************
Change password forms are not yet supported in PasswordManager. But after adding pushState support recently, they could be offered to the user for saving.

This CL adds a check to avoid change password forms being saved.

Review URL: https://codereview.chromium.org/812033010

Cr-Commit-Position: refs/heads/master@{#312179}
********************************************
(cherry picked from commit 79a244fb1a52c5f3594e31113d606c80000e90dd)

BUG=447558, 448351
TBR=gcasto@chromium.org

Review URL: https://codereview.chromium.org/884093002

Cr-Commit-Position: refs/branch-heads/2272@{#143}
Cr-Branched-From: 827a380cfdb31aa54c8d56e63ce2c3fd8c3ba4d4-refs/heads/master@{#310958}

12 files changed, 199 insertions, 14 deletions

diff --git a/chrome/browser/password_manager/password_manager_browsertest.cc b/chrome/browser/password_manager/password_manager_browsertest.cc
index 517dbc5..4e25673 100644
--- a/chrome/browser/password_manager/password_manager_browsertest.cc
+++ b/chrome/browser/password_manager/password_manager_browsertest.cc
@@ -349,6 +349,15 @@ class PasswordManagerBrowserTest : public InProcessBrowserTest {
   void CheckElementValue(const std::string& element_id,
                          const std::string& expected_value);
 
+  // TODO(dvadym): Remove this once history.pushState() handling is not behind
+  // a flag.
+  // Calling this will activate handling of pushState()-initiated form submits.
+  // This feature is currently behind a renderer flag. Just setting the flag in
+  // the browser will not change it for the already running renderer at tab 0.
+  // Therefore this method first sets the flag and then opens a new tab at
+  // position 0. This new tab gets the flag copied from the browser process.
+  void ActivateHistoryPushState();
+
  private:
   DISALLOW_COPY_AND_ASSIGN(PasswordManagerBrowserTest);
 };
@@ -416,6 +425,12 @@ void PasswordManagerBrowserTest::CheckElementValue(
                             << ", expected_value = " << expected_value;
 }
 
+void PasswordManagerBrowserTest::ActivateHistoryPushState() {
+  base::CommandLine::ForCurrentProcess()->AppendSwitch(
+      autofill::switches::kEnablePasswordSaveOnInPageNavigation);
+  AddTabAtIndex(0, GURL(url::kAboutBlankURL), ui::PAGE_TRANSITION_TYPED);
+}
+
 // Actual tests ---------------------------------------------------------------
 IN_PROC_BROWSER_TEST_F(PasswordManagerBrowserTest,
                        PromptForNormalSubmit) {
@@ -1401,9 +1416,7 @@ IN_PROC_BROWSER_TEST_F(PasswordManagerBrowserTest,
 }
 
 IN_PROC_BROWSER_TEST_F(PasswordManagerBrowserTest, PromptForPushState) {
-  base::CommandLine::ForCurrentProcess()->AppendSwitch(
-      autofill::switches::kEnablePasswordSaveOnInPageNavigation);
-  AddTabAtIndex(0, GURL(url::kAboutBlankURL), ui::PAGE_TRANSITION_TYPED);
+  ActivateHistoryPushState();
   NavigateToFile("/password/password_push_state.html");
 
   // Verify that we show the save password prompt if 'history.pushState()'
@@ -1484,3 +1497,80 @@ IN_PROC_BROWSER_TEST_F(PasswordManagerBrowserTest,
   // Make sure the popup would be shown.
   observing_autofill_client.Wait();
 }
+
+// Passwords from change password forms should only be offered for saving when
+// it is certain that the username is correct.
+IN_PROC_BROWSER_TEST_F(PasswordManagerBrowserTest, ChangePwdCorrect) {
+  NavigateToFile("/password/password_form.html");
+
+  NavigationObserver observer(WebContents());
+  scoped_ptr<PromptObserver> prompt_observer(
+      PromptObserver::Create(WebContents()));
+  std::string fill_and_submit =
+      "document.getElementById('mark_chg_username_field').value = 'temp';"
+      "document.getElementById('mark_chg_password_field').value = 'random';"
+      "document.getElementById('mark_chg_new_password_1').value = 'random1';"
+      "document.getElementById('mark_chg_new_password_2').value = 'random1';"
+      "document.getElementById('mark_chg_submit_button').click()";
+  ASSERT_TRUE(content::ExecuteScript(RenderViewHost(), fill_and_submit));
+  observer.Wait();
+  EXPECT_TRUE(prompt_observer->IsShowingPrompt());
+}
+
+IN_PROC_BROWSER_TEST_F(PasswordManagerBrowserTest, ChangePwdIncorrect) {
+  NavigateToFile("/password/password_form.html");
+
+  NavigationObserver observer(WebContents());
+  scoped_ptr<PromptObserver> prompt_observer(
+      PromptObserver::Create(WebContents()));
+  std::string fill_and_submit =
+      "document.getElementById('chg_not_username_field').value = 'temp';"
+      "document.getElementById('chg_password_field').value = 'random';"
+      "document.getElementById('chg_new_password_1').value = 'random1';"
+      "document.getElementById('chg_new_password_2').value = 'random1';"
+      "document.getElementById('chg_submit_button').click()";
+  ASSERT_TRUE(content::ExecuteScript(RenderViewHost(), fill_and_submit));
+  observer.Wait();
+  EXPECT_FALSE(prompt_observer->IsShowingPrompt());
+}
+
+// As the two ChangePwd* tests above, only with submitting through
+// history.pushState().
+IN_PROC_BROWSER_TEST_F(PasswordManagerBrowserTest, ChangePwdPushStateCorrect) {
+  ActivateHistoryPushState();
+  NavigateToFile("/password/password_push_state.html");
+
+  NavigationObserver observer(WebContents());
+  observer.SetQuitOnEntryCommitted(true);
+  scoped_ptr<PromptObserver> prompt_observer(
+      PromptObserver::Create(WebContents()));
+  std::string fill_and_submit =
+      "document.getElementById('mark_chg_username_field').value = 'temp';"
+      "document.getElementById('mark_chg_password_field').value = 'random';"
+      "document.getElementById('mark_chg_new_password_1').value = 'random1';"
+      "document.getElementById('mark_chg_new_password_2').value = 'random1';"
+      "document.getElementById('mark_chg_submit_button').click()";
+  ASSERT_TRUE(content::ExecuteScript(RenderViewHost(), fill_and_submit));
+  observer.Wait();
+  EXPECT_TRUE(prompt_observer->IsShowingPrompt());
+}
+
+IN_PROC_BROWSER_TEST_F(PasswordManagerBrowserTest,
+                       ChangePwdPushStateIncorrect) {
+  ActivateHistoryPushState();
+  NavigateToFile("/password/password_push_state.html");
+
+  NavigationObserver observer(WebContents());
+  observer.SetQuitOnEntryCommitted(true);
+  scoped_ptr<PromptObserver> prompt_observer(
+      PromptObserver::Create(WebContents()));
+  std::string fill_and_submit =
+      "document.getElementById('chg_not_username_field').value = 'temp';"
+      "document.getElementById('chg_password_field').value = 'random';"
+      "document.getElementById('chg_new_password_1').value = 'random1';"
+      "document.getElementById('chg_new_password_2').value = 'random1';"
+      "document.getElementById('chg_submit_button').click()";
+  ASSERT_TRUE(content::ExecuteScript(RenderViewHost(), fill_and_submit));
+  observer.Wait();
+  EXPECT_FALSE(prompt_observer->IsShowingPrompt());
+}
diff --git a/chrome/test/data/password/password_form.html b/chrome/test/data/password/password_form.html
index c3c2193..725c03e 100644
--- a/chrome/test/data/password/password_form.html
+++ b/chrome/test/data/password/password_form.html
@@ -60,5 +60,32 @@ needs to access them by their offsets in the array of the form children.
   <input type="submit" id="submit_failed" name="submit_failed">
 </form>
 
+<!-- Change password form without the username explicitly marked. -->
+<form action="done.html" id="chg_testform">
+  <!-- No autocomplete=username. -->
+  <input type="text" id="chg_not_username_field" name="chg_not_username_field">
+  <input type="password" id="chg_password_field" name="chg_password_field"
+         autocomplete="current-password">
+  <input type="password" id="chg_new_password_1" name="chg_new_password_1"
+         autocomplete="new-password">
+  <input type="password" id="chg_new_password_2" name="chg_new_password_2"
+         autocomplete="new-password">
+  <input type="submit" id="chg_submit_button" name="chg_submit_button">
+</form>
+
+<!-- Change password form with the username explicitly marked. -->
+<form action="done.html" id="mark_chg_testform">
+  <input type="text" id="mark_chg_username_field" name="mark_chg_username_field"
+         autocomplete="username">
+  <input type="password" id="mark_chg_password_field"
+         name="mark_chg_password_field" autocomplete="current-password">
+  <input type="password" id="mark_chg_new_password_1"
+         name="mark_chg_new_password_1" autocomplete="new-password">
+  <input type="password" id="mark_chg_new_password_2"
+         name="mark_chg_new_password_2" autocomplete="new-password">
+  <input type="submit" id="mark_chg_submit_button"
+         name="mark_chg_submit_button">
+</form>
+
 </body>
 </html>
diff --git a/chrome/test/data/password/password_push_state.html b/chrome/test/data/password/password_push_state.html
index 58d8e1a..c24b510 100644
--- a/chrome/test/data/password/password_push_state.html
+++ b/chrome/test/data/password/password_push_state.html
@@ -2,15 +2,22 @@
 <head>
 <script>
 
-window.onload = function() {
-  document.getElementById("testform").addEventListener("submit", function(e) {
+function handleSubmitFormEvent(e) {
     e.preventDefault();
     var xhr = new XMLHttpRequest();
     xhr.onreadystatechange = function() {
       history.pushState({}, "", "password_push_state.html"); };
     xhr.open("GET", "password_push_state.html", true);
     xhr.send(null);
-});
+}
+
+window.onload = function() {
+  document.getElementById("testform")
+          .addEventListener("submit", handleSubmitFormEvent);
+  document.getElementById("chg_testform")
+          .addEventListener("submit", handleSubmitFormEvent);
+  document.getElementById("mark_chg_testform")
+          .addEventListener("submit", handleSubmitFormEvent);
 }
 
 </script>
@@ -23,6 +30,33 @@ window.onload = function() {
   <input type="submit" id="submit_button" name="submit_button">
 </form>
 
+<!-- Change password form without the username explicitly marked. -->
+<form action="password_push_state.html" id="chg_testform">
+  <!-- No autocomplete=username. -->
+  <input type="text" id="chg_not_username_field" name="chg_not_username_field">
+  <input type="password" id="chg_password_field" name="chg_password_field"
+         autocomplete="current-password">
+  <input type="password" id="chg_new_password_1" name="chg_new_password_1"
+         autocomplete="new-password">
+  <input type="password" id="chg_new_password_2" name="chg_new_password_2"
+         autocomplete="new-password">
+  <input type="submit" id="chg_submit_button" name="chg_submit_button">
+</form>
+
+<!-- Change password form with the username explicitly marked. -->
+<form action="password_push_state.html" id="mark_chg_testform">
+  <input type="text" id="mark_chg_username_field" name="mark_chg_username_field"
+         autocomplete="username">
+  <input type="password" id="mark_chg_password_field"
+         name="mark_chg_password_field" autocomplete="current-password">
+  <input type="password" id="mark_chg_new_password_1"
+         name="mark_chg_new_password_1" autocomplete="new-password">
+  <input type="password" id="mark_chg_new_password_2"
+         name="mark_chg_new_password_2" autocomplete="new-password">
+  <input type="submit" id="mark_chg_submit_button"
+         name="mark_chg_submit_button">
+</form>
+
 </body>
 </html>
 
diff --git a/components/autofill/content/common/autofill_param_traits_macros.h b/components/autofill/content/common/autofill_param_traits_macros.h
index 20a7779..eca7eb5 100644
--- a/components/autofill/content/common/autofill_param_traits_macros.h
+++ b/components/autofill/content/common/autofill_param_traits_macros.h
@@ -32,6 +32,7 @@ IPC_STRUCT_TRAITS_BEGIN(autofill::PasswordForm)
   IPC_STRUCT_TRAITS_MEMBER(action)
   IPC_STRUCT_TRAITS_MEMBER(submit_element)
   IPC_STRUCT_TRAITS_MEMBER(username_element)
+  IPC_STRUCT_TRAITS_MEMBER(username_marked_by_site)
   IPC_STRUCT_TRAITS_MEMBER(username_value)
   IPC_STRUCT_TRAITS_MEMBER(other_possible_usernames)
   IPC_STRUCT_TRAITS_MEMBER(password_element)
diff --git a/components/autofill/content/renderer/password_form_conversion_utils.cc b/components/autofill/content/renderer/password_form_conversion_utils.cc
index 52c510a6..af3cbd8 100644
--- a/components/autofill/content/renderer/password_form_conversion_utils.cc
+++ b/components/autofill/content/renderer/password_form_conversion_utils.cc
@@ -116,9 +116,7 @@ void GetPasswordForm(const WebFormElement& form,
                                     blink::WebString>* user_modified_elements) {
   WebInputElement latest_input_element;
   WebInputElement username_element;
-  // Caches whether |username_element| is marked with autocomplete='username'.
-  // Needed for performance reasons to avoid recalculating this multiple times.
-  bool has_seen_element_with_autocomplete_username_before = false;
+  password_form->username_marked_by_site = false;
   std::vector<WebInputElement> passwords;
   std::vector<base::string16> other_possible_usernames;
 
@@ -155,7 +153,7 @@ void GetPasswordForm(const WebFormElement& form,
     // Various input types such as text, url, email can be a username field.
     if (input_element->isTextField() && !input_element->isPasswordField()) {
       if (HasAutocompleteAttributeValue(*input_element, "username")) {
-        if (has_seen_element_with_autocomplete_username_before) {
+        if (password_form->username_marked_by_site) {
           // A second or subsequent element marked with autocomplete='username'.
           // This makes us less confident that we have understood the form. We
           // will stick to our choice that the first such element was the real
@@ -173,11 +171,11 @@ void GetPasswordForm(const WebFormElement& form,
           // usernames we have accrued so far: they come from fields not marked
           // with the autocomplete attribute, making them unlikely alternatives.
           username_element = *input_element;
-          has_seen_element_with_autocomplete_username_before = true;
+          password_form->username_marked_by_site = true;
           other_possible_usernames.clear();
         }
       } else {
-        if (has_seen_element_with_autocomplete_username_before) {
+        if (password_form->username_marked_by_site) {
           // Having seen elements with autocomplete='username', elements without
           // this attribute are no longer interesting. No-op.
         } else {
diff --git a/components/autofill/core/common/password_form.cc b/components/autofill/core/common/password_form.cc
index 7c92e0b..73a2fd5 100644
--- a/components/autofill/core/common/password_form.cc
+++ b/components/autofill/core/common/password_form.cc
@@ -13,6 +13,7 @@ namespace autofill {
 
 PasswordForm::PasswordForm()
     : scheme(SCHEME_HTML),
+      username_marked_by_site(false),
       password_autocomplete_set(true),
       ssl_valid(false),
       preferred(false),
@@ -35,6 +36,7 @@ bool PasswordForm::operator==(const PasswordForm& form) const {
       action == form.action &&
       submit_element == form.submit_element &&
       username_element == form.username_element &&
+      username_marked_by_site == form.username_marked_by_site &&
       username_value == form.username_value &&
       other_possible_usernames == form.other_possible_usernames &&
       password_element == form.password_element &&
@@ -67,6 +69,7 @@ std::ostream& operator<<(std::ostream& os, const PasswordForm& form) {
             << " action: " << form.action
             << " submit_element: " << base::UTF16ToUTF8(form.submit_element)
             << " username_elem: " << base::UTF16ToUTF8(form.username_element)
+            << " username_marked_by_site: " << form.username_marked_by_site
             << " username_value: " << base::UTF16ToUTF8(form.username_value)
             << " password_elem: " << base::UTF16ToUTF8(form.password_element)
             << " password_value: " << base::UTF16ToUTF8(form.password_value)
diff --git a/components/autofill/core/common/password_form.h b/components/autofill/core/common/password_form.h
index 77051bb..039e644 100644
--- a/components/autofill/core/common/password_form.h
+++ b/components/autofill/core/common/password_form.h
@@ -104,6 +104,10 @@ struct PasswordForm {
   // When parsing an HTML form, this must always be set.
   base::string16 username_element;
 
+  // Whether the |username_element| has an autocomplete=username attribute. This
+  // is only used in parsed HTML forms.
+  bool username_marked_by_site;
+
   // The username. Optional.
   //
   // When parsing an HTML form, this is typically empty unless the site
diff --git a/components/autofill/core/common/save_password_progress_logger.cc b/components/autofill/core/common/save_password_progress_logger.cc
index 872ac79..04e9e68 100644
--- a/components/autofill/core/common/save_password_progress_logger.cc
+++ b/components/autofill/core/common/save_password_progress_logger.cc
@@ -209,6 +209,8 @@ std::string GetStringFromID(SavePasswordProgressLogger::StringID id) {
       return "The new state of the UI";
     case SavePasswordProgressLogger::STRING_FORM_NOT_AUTOFILLED:
       return "The observed form will not be autofilled";
+    case SavePasswordProgressLogger::STRING_CHANGE_PASSWORD_FORM:
+      return "Not saving password for a change password form";
     case SavePasswordProgressLogger::STRING_INVALID:
       return "INVALID";
       // Intentionally no default: clause here -- all IDs need to get covered.
diff --git a/components/autofill/core/common/save_password_progress_logger.h b/components/autofill/core/common/save_password_progress_logger.h
index cf1ed6f..4c5eec4 100644
--- a/components/autofill/core/common/save_password_progress_logger.h
+++ b/components/autofill/core/common/save_password_progress_logger.h
@@ -121,6 +121,7 @@ class SavePasswordProgressLogger {
     STRING_SHOW_LOGIN_PROMPT_METHOD,
     STRING_NEW_UI_STATE,
     STRING_FORM_NOT_AUTOFILLED,
+    STRING_CHANGE_PASSWORD_FORM,
     STRING_INVALID,  // Represents a string returned in a case of an error.
     STRING_MAX = STRING_INVALID
   };
diff --git a/components/password_manager/core/browser/password_form_manager.cc b/components/password_manager/core/browser/password_form_manager.cc
index 212d6a4..155c738 100644
--- a/components/password_manager/core/browser/password_form_manager.cc
+++ b/components/password_manager/core/browser/password_form_manager.cc
@@ -402,6 +402,13 @@ bool PasswordFormManager::HasCompletedMatching() const {
   return state_ == POST_MATCHING_PHASE;
 }
 
+bool PasswordFormManager::IsIgnorableChangePasswordForm() const {
+  bool is_change_password_form = !observed_form_.new_password_element.empty() &&
+                                 !observed_form_.password_element.empty();
+  bool is_username_certainly_correct = observed_form_.username_marked_by_site;
+  return is_change_password_form && !is_username_certainly_correct;
+}
+
 void PasswordFormManager::OnRequestDone(
     const std::vector<PasswordForm*>& logins_result) {
   // Note that the result gets deleted after this call completes, but we own
diff --git a/components/password_manager/core/browser/password_form_manager.h b/components/password_manager/core/browser/password_form_manager.h
index dd51c6a..5764649 100644
--- a/components/password_manager/core/browser/password_form_manager.h
+++ b/components/password_manager/core/browser/password_form_manager.h
@@ -84,6 +84,16 @@ class PasswordFormManager : public PasswordStoreConsumer {
   // the same thread!
   bool HasCompletedMatching() const;
 
+  // Returns true if the observed form has both the current and new password
+  // fields, and the username field was not explicitly marked with
+  // autocomplete=username. In these cases it is not clear whether the username
+  // field is the right guess (often such change password forms do not contain
+  // the username at all), and the user should not be bothered with saving a
+  // potentially malformed credential. Once we handle change password forms
+  // correctly, or http://crbug.com/448351 gets implemented, this method should
+  // be replaced accordingly.
+  bool IsIgnorableChangePasswordForm() const;
+
   // Determines if the user opted to 'never remember' passwords for this form.
   bool IsBlacklisted() const;
 
diff --git a/components/password_manager/core/browser/password_manager.cc b/components/password_manager/core/browser/password_manager.cc
index 76f0532..298919c 100644
--- a/components/password_manager/core/browser/password_manager.cc
+++ b/components/password_manager/core/browser/password_manager.cc
@@ -234,9 +234,17 @@ void PasswordManager::ProvisionallySavePassword(const PasswordForm& form) {
        ++iter) {
     PasswordFormManager::MatchResultMask result = (*iter)->DoesManage(form);
 
+    if (result == PasswordFormManager::RESULT_NO_MATCH)
+      continue;
+
+    if ((*iter)->IsIgnorableChangePasswordForm()) {
+      if (logger)
+        logger->LogMessage(Logger::STRING_CHANGE_PASSWORD_FORM);
+      continue;
+    }
+
     if (!(*iter)->HasCompletedMatching()) {
-      if (result != PasswordFormManager::RESULT_NO_MATCH)
-        has_found_matching_managers_which_were_not_ready = true;
+      has_found_matching_managers_which_were_not_ready = true;
       continue;
     }