[Password Generation] Save passwords even if heuristics determine submit failed

chrome/browser/password_manager/password_generation_interactive_uitest.cc

Index: chrome/browser/password_manager/password_generation_interactive_uitest.cc
diff --git a/chrome/browser/password_manager/password_generation_interactive_uitest.cc b/chrome/browser/password_manager/password_generation_interactive_uitest.cc
index 334367e496604eac783ad20435203e7c615500e8..2fb28c0d6b73759562d0889cd22b6e2a4535f404 100644
--- a/chrome/browser/password_manager/password_generation_interactive_uitest.cc
+++ b/chrome/browser/password_manager/password_generation_interactive_uitest.cc
@@ -3,7 +3,10 @@
 // found in the LICENSE file.
 
 #include "base/command_line.h"
+#include "base/strings/utf_string_conversions.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/profiles/profile.h"
 #include "chrome/browser/ui/autofill/password_generation_popup_observer.h"
 #include "chrome/browser/ui/browser.h"
@@ -13,6 +16,7 @@
 #include "components/autofill/core/browser/autofill_test_utils.h"
 #include "components/autofill/core/common/autofill_switches.h"
 #include "components/password_manager/core/browser/password_generation_manager.h"
+#include "components/password_manager/core/browser/test_password_store.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/test/browser_test_utils.h"
@@ -46,7 +50,8 @@ class TestPopupObserver : public autofill::PasswordGenerationPopupObserver {
 
 }  // namespace
 
-class PasswordGenerationInteractiveTest : public InProcessBrowserTest {
+class PasswordGenerationInteractiveTest :
+    public PasswordManagerBrowserTestBase {
  public:
   void SetUpCommandLine(base::CommandLine* command_line) override {
     // Make sure the feature is enabled.
@@ -58,39 +63,33 @@ class PasswordGenerationInteractiveTest : public InProcessBrowserTest {
   }
 
   void SetUpOnMainThread() override {
+    PasswordManagerBrowserTestBase::SetUpOnMainThread();
+
     // Disable Autofill requesting access to AddressBook data. This will cause
     // the tests to hang on Mac.
     autofill::test::DisableSystemServices(browser()->profile()->GetPrefs());
 
     // Set observer for popup.
     ChromePasswordManagerClient* client =
-        ChromePasswordManagerClient::FromWebContents(GetWebContents());
+        ChromePasswordManagerClient::FromWebContents(WebContents());
     client->SetTestObserver(&observer_);
 
-    ASSERT_TRUE(embedded_test_server()->InitializeAndWaitUntilReady());
-    GURL url = embedded_test_server()->GetURL("/password/signup_form.html");
-    ui_test_utils::NavigateToURL(browser(), url);
+    NavigateToFile("/password/signup_form.html");
   }
 
   void TearDownOnMainThread() override {
+    PasswordManagerBrowserTestBase::TearDownOnMainThread();
+
     // Clean up UI.
     ChromePasswordManagerClient* client =
-        ChromePasswordManagerClient::FromWebContents(GetWebContents());
+        ChromePasswordManagerClient::FromWebContents(WebContents());
     client->HidePasswordGenerationPopup();
   }
 
-  content::WebContents* GetWebContents() {
-    return browser()->tab_strip_model()->GetActiveWebContents();
-  }
-
-  content::RenderViewHost* GetRenderViewHost() {
-    return GetWebContents()->GetRenderViewHost();
-  }
-
   std::string GetFieldValue(const std::string& field_id) {
     std::string value;
     EXPECT_TRUE(content::ExecuteScriptAndExtractString(
-        GetRenderViewHost(),
+        RenderViewHost(),
         "window.domAutomationController.send("
         "    document.getElementById('" + field_id + "').value);",
         &value));
@@ -100,7 +99,7 @@ class PasswordGenerationInteractiveTest : public InProcessBrowserTest {
   std::string GetFocusedElement() {
     std::string focused_element;
     EXPECT_TRUE(content::ExecuteScriptAndExtractString(
-        GetRenderViewHost(),
+        RenderViewHost(),
         "window.domAutomationController.send("
         "    document.activeElement.id)",
         &focused_element));
@@ -109,7 +108,7 @@ class PasswordGenerationInteractiveTest : public InProcessBrowserTest {
 
   void FocusPasswordField() {
     ASSERT_TRUE(content::ExecuteScript(
-        GetRenderViewHost(),
+        RenderViewHost(),
         "document.getElementById('password_field').focus()"));
   }
 
@@ -117,7 +116,7 @@ class PasswordGenerationInteractiveTest : public InProcessBrowserTest {
     content::NativeWebKeyboardEvent event;
     event.windowsKeyCode = key;
     event.type = blink::WebKeyboardEvent::RawKeyDown;
-    GetRenderViewHost()->ForwardKeyboardEvent(event);
+    RenderViewHost()->ForwardKeyboardEvent(event);
   }
 
   bool GenerationPopupShowing() {
@@ -170,7 +169,7 @@ IN_PROC_BROWSER_TEST_F(PasswordGenerationInteractiveTest,
   FocusPasswordField();
   EXPECT_TRUE(GenerationPopupShowing());
 
-  ASSERT_TRUE(content::ExecuteScript(GetRenderViewHost(),
+  ASSERT_TRUE(content::ExecuteScript(RenderViewHost(),
                                      "window.scrollTo(100, 0);"));
 
   EXPECT_FALSE(GenerationPopupShowing());
@@ -179,15 +178,48 @@ IN_PROC_BROWSER_TEST_F(PasswordGenerationInteractiveTest,
 // Disabled due to flakiness due to resizes, see http://crbug.com/407998.
 IN_PROC_BROWSER_TEST_F(PasswordGenerationInteractiveTest,
                        DISABLED_GenerationTriggeredInIFrame) {
-  GURL url = embedded_test_server()->GetURL(
-      "/password/framed_signup_form.html");
-  ui_test_utils::NavigateToURL(browser(), url);
+  NavigateToFile("/password/framed_signup_form.html");
 
   std::string focus_script =
       "var frame = document.getElementById('signup_iframe');"
       "var frame_doc = frame.contentDocument;"
       "frame_doc.getElementById('password_field').focus();";
 
-  ASSERT_TRUE(content::ExecuteScript(GetRenderViewHost(), focus_script));
+  ASSERT_TRUE(content::ExecuteScript(RenderViewHost(), focus_script));
   EXPECT_TRUE(GenerationPopupShowing());
 }
+
+IN_PROC_BROWSER_TEST_F(PasswordGenerationInteractiveTest,
+                       DISABLED_AutoSavingGeneratedPassword) {

[vabr (Chromium), 2015/07/13 14:57:00]

nit: Could you please add a bugtracker URL associated with enabling this test?

[Garrett Casto, 2015/07/13 17:20:13]

Done.

+  password_manager::TestPasswordStore* password_store =
+      static_cast<password_manager::TestPasswordStore*>(
+          PasswordStoreFactory::GetForProfile(
+              browser()->profile(), ServiceAccessType::IMPLICIT_ACCESS).get());
+
+  FocusPasswordField();
+  EXPECT_TRUE(GenerationPopupShowing());
+  SendKeyToPopup(ui::VKEY_DOWN);
+  SendKeyToPopup(ui::VKEY_RETURN);
+
+  // Change the action so that it looks like the submission failed.

[vabr (Chromium), 2015/07/13 14:57:00]

I'm confused -- where is the script below changing the action? And if you meant
"change the username", how does that make the submission look failed?

[Garrett Casto, 2015/07/13 17:20:13]

I have no idea what I was thinking when I wrote this. Changed to match the test.

+  NavigationObserver observer(WebContents());
+  std::string submit_script =
+      "document.getElementById('username_field').value = 'something';"
+      "document.getElementById('input_submit_button').click()";
+  ASSERT_TRUE(content::ExecuteScript(RenderViewHost(), submit_script));
+  observer.Wait();
+
+  // 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());
+
+  // Make sure the username is correct.
+  password_manager::TestPasswordStore::PasswordMap stored_passwords =
+      password_store->stored_passwords();
+  EXPECT_EQ(1u, stored_passwords.size());
+  EXPECT_EQ(1u, stored_passwords.begin()->second.size());
+  EXPECT_EQ(base::UTF8ToUTF16("something"),
+            (stored_passwords.begin()->second)[0].username_value);
+}