Update Confirmation UI for saved password change

chrome/browser/password_manager/password_manager_browsertest.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <string>
 
 #include "base/command_line.h"
 #include "base/metrics/histogram_samples.h"
 #include "base/metrics/statistics_recorder.h"
 #include "base/path_service.h"
 #include "base/run_loop.h"
 #include "base/stl_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/password_manager/chrome_password_manager_client.h"
 #include "chrome/browser/password_manager/password_manager_test_base.h"
 #include "chrome/browser/password_manager/password_store_factory.h"
 #include "chrome/browser/password_manager/test_password_store_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/login/login_prompt.h"
 #include "chrome/browser/ui/login/login_prompt_test_utils.h"
+#include "chrome/browser/ui/passwords/manage_passwords_ui_controller.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/chrome_version_info.h"
 #include "chrome/test/base/test_switches.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "components/autofill/content/common/autofill_messages.h"
 #include "components/autofill/core/browser/autofill_test_utils.h"
 #include "components/autofill/core/browser/test_autofill_client.h"
 #include "components/autofill/core/common/password_form.h"
@@ skipping 1506 matching lines @@
       "window.domAutomationController.send(usernameRect.left);",
       &left));
 
   content::SimulateMouseClickAt(
       WebContents(), 0, blink::WebMouseEvent::ButtonLeft, gfx::Point(left + 1,
                                                                      top + 1));
   // 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(PasswordManagerBrowserTestBase, ChangePwdCorrect) {
+IN_PROC_BROWSER_TEST_F(PasswordManagerBrowserTestBase,
+                       ChangePwdMarkedWithUsername) {
   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(PasswordManagerBrowserTestBase, ChangePwdIncorrect) {
+IN_PROC_BROWSER_TEST_F(PasswordManagerBrowserTestBase, ChangePwdWithUsername) {
   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());
+  EXPECT_TRUE(prompt_observer->IsShowingPrompt());
 }
 
 // As the two ChangePwd* tests above, only with submitting through
 // history.pushState().
 IN_PROC_BROWSER_TEST_F(PasswordManagerBrowserTestBase,
-                       ChangePwdPushStateCorrect) {
+                       ChangePwdPushStateMarkedWithUsername) {
   NavigateToFile("/password/password_push_state.html");
 
   NavigationObserver observer(WebContents());
   observer.set_quit_on_entry_committed(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(PasswordManagerBrowserTestBase,
-                       ChangePwdPushStateIncorrect) {
+                       ChangePwdPushStateWithUsername) {
   NavigateToFile("/password/password_push_state.html");
 
   NavigationObserver observer(WebContents());
   observer.set_quit_on_entry_committed(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());
+  EXPECT_TRUE(prompt_observer->IsShowingPrompt());
 }
 
 IN_PROC_BROWSER_TEST_F(PasswordManagerBrowserTestBase, NoPromptOnBack) {
   // Go to a successful landing page through submitting first, so that it is
   // reachable through going back, and the remembered page transition is form
   // submit. There is no need to submit non-empty strings.
   NavigateToFile("/password/password_form.html");
 
   NavigationObserver dummy_submit_observer(WebContents());
   std::string just_submit =
@@ skipping 379 matching lines @@
   content::RenderProcessHostWatcher iframe_killed(
       iframe->GetProcess(),
       content::RenderProcessHostWatcher::WATCH_FOR_PROCESS_EXIT);
 
   IPC::IpcSecurityTestUtil::PwnMessageReceived(
       iframe->GetProcess()->GetChannel(), illegal_forms_parsed);
 
   iframe_killed.Wait();
 }
 
+IN_PROC_BROWSER_TEST_F(PasswordManagerBrowserTestBase,
+                       ChangePwdNoAccountStored) {
+  ASSERT_TRUE(ChromePasswordManagerClient::IsTheHotNewBubbleUIEnabled());
+  NavigateToFile("/password/password_form.html");
+
+  base::CommandLine::ForCurrentProcess()->AppendSwitch(
+      password_manager::switches::kEnablePasswordChangeSupport);
+
+  // Fill a form and submit through a <input type="submit"> button.
+  NavigationObserver observer(WebContents());
+  scoped_ptr<PromptObserver> prompt_observer(

[vabr (Chromium), 2015/07/31 12:48:13]

Looks unused.

[dvadym, 2015/08/03 15:44:07]

Not it's used after adding checking of credentials

+      PromptObserver::Create(WebContents()));
+  ManagePasswordsUIController* ui_controller =
+      ManagePasswordsUIController::FromWebContents(WebContents());
+
+  std::string fill_and_submit =
+      "document.getElementById('chg_password_wo_username_field').value = "
+      "'old_pw';"
+      "document.getElementById('chg_new_password_wo_username_1').value = "
+      "'new_pw';"
+      "document.getElementById('chg_new_password_wo_username_2').value = "
+      "'new_pw';"
+      "document.getElementById('chg_submit_wo_username_button').click()";
+  ASSERT_TRUE(content::ExecuteScript(RenderViewHost(), fill_and_submit));
+  observer.Wait();
+  // No credentials stored, so save bubble is shown.
+  EXPECT_EQ(password_manager::ui::PENDING_PASSWORD_STATE,
+            ui_controller->state());
+}

[vabr (Chromium), 2015/07/31 12:48:13]

Is there a way (here and in the other test) to check that the correct password
was saved?

[dvadym, 2015/08/03 15:44:07]

Good point, I've added check for these tests

+
+IN_PROC_BROWSER_TEST_F(PasswordManagerBrowserTestBase,
+                       ChangePwd1AccountStored) {
+  ASSERT_TRUE(ChromePasswordManagerClient::IsTheHotNewBubbleUIEnabled());
+  base::CommandLine::ForCurrentProcess()->AppendSwitch(
+      password_manager::switches::kEnablePasswordChangeSupport);
+  // At first let us save credentials to the PasswordManager.
+  NavigateToFile("/password/password_form.html");
+  NavigationObserver observer1(WebContents());
+  scoped_ptr<PromptObserver> prompt_observer(
+      PromptObserver::Create(WebContents()));
+  ManagePasswordsUIController* ui_controller =
+      ManagePasswordsUIController::FromWebContents(WebContents());
+  std::string fill_and_submit_signin =
+      "document.getElementById('username_field').value = 'temp';"
+      "document.getElementById('password_field').value = 'random';"
+      "document.getElementById('input_submit_button').click()";
+  ASSERT_TRUE(content::ExecuteScript(RenderViewHost(), fill_and_submit_signin));
+  observer1.Wait();
+  EXPECT_EQ(password_manager::ui::PENDING_PASSWORD_STATE,
+            ui_controller->state());
+  prompt_observer->Accept();
+  // Check that credentials are stored.
+  password_manager::TestPasswordStore* password_store =

[vabr (Chromium), 2015/07/31 12:48:13]

Is it actually possible to just store the credentials to the store directly
without loading the page for that? I'm concerned about lengthy tests, which are
prone to flaking (also, I acknowledge that this is the case already for some
existing tests, but still would be nice to fix).

[vabr (Chromium), 2015/07/31 12:48:13]

Keeping the test password store in a naked pointer is not the best practice, but
since your CL did not start this you have a choice:
(1) Fix the new tests to use scoped_refptr, or
(2) at least insert a TODO(vabr) pointing to http://crbug.com/515870.

[dvadym, 2015/08/03 15:44:07]

Here we don't owe this TestPasswordStore, so it seems ok to use raw pointers.
Isn't it true?

[dvadym, 2015/08/03 15:44:07]

I've implemented such possibility and made direct adding PasswordForm to
PasswordStore

[vabr (Chromium), 2015/08/04 08:59:07]

For scoped_refptr (unlike scoped_ptr), there is no notion of a single owner --
anybody needing the object to live should increase its refcount. We might argue
whether some other piece of code keeps the store alive long enough for us not to
care, but this does not make the code clearer. Let's follow the standard
practice of keeping refcounted objects in scoped_refptr. (Note: the situation is
very different in Blink's refcounting, where performance is a big concern.)

[dvadym, 2015/08/04 14:34:46]

Done.

+      static_cast<password_manager::TestPasswordStore*>(
+          PasswordStoreFactory::GetForProfile(
+              browser()->profile(), ServiceAccessType::IMPLICIT_ACCESS)
+              .get());
+  // Spin the message loop to make sure the password store had a chance to save
+  // the password.
+  base::RunLoop run_loop;
+  run_loop.RunUntilIdle();
+  EXPECT_FALSE(password_store->IsEmpty());
+
+  // Check that password update bubble is shown.
+  NavigateToFile("/password/password_form.html");
+  NavigationObserver observer2(WebContents());
+  std::string fill_and_submit_change_password =
+      "document.getElementById('chg_password_wo_username_field').value = "
+      "'random';"
+      "document.getElementById('chg_new_password_wo_username_1').value = "
+      "'new_pw';"
+      "document.getElementById('chg_new_password_wo_username_2').value = "
+      "'new_pw';"
+      "document.getElementById('chg_submit_wo_username_button').click()";
+  ASSERT_TRUE(content::ExecuteScript(RenderViewHost(),
+                                     fill_and_submit_change_password));
+  observer2.Wait();
+  EXPECT_EQ(password_manager::ui::PENDING_PASSWORD_UPDATE_STATE,
+            ui_controller->state());
+}
+
 }  // namespace password_manager