[Password Manager] Show omnibox icon and anchored prompt once user start typing password

chrome/browser/password_manager/password_manager_test_base.cc

Index: chrome/browser/password_manager/password_manager_test_base.cc
diff --git a/chrome/browser/password_manager/password_manager_test_base.cc b/chrome/browser/password_manager/password_manager_test_base.cc
index b5e3d1239bb012f819ba8b6529d0fbade0cdf267..fa54dacd1b77a073dd8d9b48191a62f05612b180 100644
--- a/chrome/browser/password_manager/password_manager_test_base.cc
+++ b/chrome/browser/password_manager/password_manager_test_base.cc
@@ -66,11 +66,20 @@ class CustomManagePasswordsUIController : public ManagePasswordsUIController {
 
   void WaitForState(password_manager::ui::State target_state);
 
+  bool was_prompt_shown() { return was_prompt_shown_; }
+
  private:
   // PasswordsClientUIDelegate:
   void OnPasswordSubmitted(
       std::unique_ptr<password_manager::PasswordFormManager> form_manager)
       override;
+  void OnUpdatePasswordSubmitted(
+      std::unique_ptr<password_manager::PasswordFormManager> form_manager)
+      override;
+  void OnShowManualFallbackForSaving(
+      std::unique_ptr<password_manager::PasswordFormManager> form_manager,
+      bool has_generated_password,
+      bool is_update) override;
   bool OnChooseCredentials(
       std::vector<std::unique_ptr<autofill::PasswordForm>> local_credentials,
       const GURL& origin,
@@ -88,6 +97,8 @@ class CustomManagePasswordsUIController : public ManagePasswordsUIController {
   // The state CustomManagePasswordsUIController is currently waiting for.
   password_manager::ui::State target_state_;
 
+  bool was_prompt_shown_;

[vasilii, 2017/08/03 09:55:38]

was_prompt_automatically_shown_?

[kolos1, 2017/08/04 16:36:17]

Done.

+
   DISALLOW_COPY_AND_ASSIGN(CustomManagePasswordsUIController);
 };
 
@@ -95,7 +106,8 @@ CustomManagePasswordsUIController::CustomManagePasswordsUIController(
     content::WebContents* web_contents)
     : ManagePasswordsUIController(web_contents),
       run_loop_(nullptr),
-      target_state_(password_manager::ui::INACTIVE_STATE) {
+      target_state_(password_manager::ui::INACTIVE_STATE),
+      was_prompt_shown_(false) {
   // Attach CustomManagePasswordsUIController to |web_contents| so the default
   // ManagePasswordsUIController isn't created.
   // Do not silently replace an existing ManagePasswordsUIController because it
@@ -119,10 +131,31 @@ void CustomManagePasswordsUIController::OnPasswordSubmitted(
     run_loop_ = nullptr;
     target_state_ = password_manager::ui::INACTIVE_STATE;
   }
+  was_prompt_shown_ = true;
   return ManagePasswordsUIController::OnPasswordSubmitted(
       std::move(form_manager));
 }
 
+void CustomManagePasswordsUIController::OnUpdatePasswordSubmitted(
+    std::unique_ptr<password_manager::PasswordFormManager> form_manager) {
+  was_prompt_shown_ = true;
+  return ManagePasswordsUIController::OnUpdatePasswordSubmitted(
+      std::move(form_manager));
+}
+
+void CustomManagePasswordsUIController::OnShowManualFallbackForSaving(
+    std::unique_ptr<password_manager::PasswordFormManager> form_manager,
+    bool has_generated_password,
+    bool is_update) {
+  if (target_state_ == password_manager::ui::PENDING_PASSWORD_STATE) {

[vasilii, 2017/08/03 09:55:39]

I think this all the existing callers of WaitForSavePrompt() are looking forward
to the automatic prompt. We shouldn't downgrade their expectations to the manual
fallback.

[kolos1, 2017/08/04 16:36:17]

Added special processing for fallbacks. 

+    run_loop_->Quit();
+    run_loop_ = nullptr;
+    target_state_ = password_manager::ui::INACTIVE_STATE;
+  }
+  ManagePasswordsUIController::OnShowManualFallbackForSaving(
+      std::move(form_manager), has_generated_password, is_update);
+}
+
 bool CustomManagePasswordsUIController::OnChooseCredentials(
     std::vector<std::unique_ptr<autofill::PasswordForm>> local_credentials,
     const GURL& origin,
@@ -204,16 +237,32 @@ BubbleObserver::BubbleObserver(content::WebContents* web_contents)
     : passwords_ui_controller_(
           ManagePasswordsUIController::FromWebContents(web_contents)) {}
 
-bool BubbleObserver::IsShowingSavePrompt() const {
+bool BubbleObserver::IsSavePromptAvailable() const {
   return passwords_ui_controller_->GetState() ==
       password_manager::ui::PENDING_PASSWORD_STATE;
 }
 
-bool BubbleObserver::IsShowingUpdatePrompt() const {
+bool BubbleObserver::IsUpdatePromptAvailable() const {
   return passwords_ui_controller_->GetState() ==
       password_manager::ui::PENDING_PASSWORD_UPDATE_STATE;
 }
 
+bool BubbleObserver::WasSavePromptShown() const {

[vasilii, 2017/08/03 09:55:38]

Not sure why those methods are called "Was..". Shouldn't it be Is.. ?

[kolos1, 2017/08/04 16:36:17]

Done.

+  if (!IsSavePromptAvailable())
+    return false;
+  return static_cast<CustomManagePasswordsUIController*>(
+             passwords_ui_controller_)
+      ->was_prompt_shown();
+}
+
+bool BubbleObserver::WasUpdatePromptShown() const {
+  if (!IsUpdatePromptAvailable())
+    return false;
+  return static_cast<CustomManagePasswordsUIController*>(
+             passwords_ui_controller_)
+      ->was_prompt_shown();
+}
+
 void BubbleObserver::Dismiss() const  {
   passwords_ui_controller_->OnBubbleHidden();
   ASSERT_EQ(password_manager::ui::INACTIVE_STATE,
@@ -221,17 +270,24 @@ void BubbleObserver::Dismiss() const  {
 }
 
 void BubbleObserver::AcceptSavePrompt() const {
-  ASSERT_TRUE(IsShowingSavePrompt());
+  ASSERT_TRUE(WasSavePromptShown());
   passwords_ui_controller_->SavePassword(
       passwords_ui_controller_->GetPendingPassword().username_value);
-  EXPECT_FALSE(IsShowingSavePrompt());
+  EXPECT_FALSE(IsSavePromptAvailable());
+}
+
+void BubbleObserver::OpenAndAcceptSavePrompt() const {

[vasilii, 2017/08/03 09:55:38]

Not sure why we need this method. It's almost a duplicate of AcceptSavePrompt.
For AcceptSavePrompt we checked only the state just to make sure that
passwords_ui_controller_->SavePassword won't crash. It seems OK to continue
doing so. If a test cares about automatic save prompt then it should call
WaitForSavePrompt() or ASSERT_TRUE(WasSavePromptShown()) explicitly.

[kolos1, 2017/08/04 16:36:17]

Let me not inline the method. I added a parameter to
BubbleObserver::AcceptSavePrompt. I guess it will be useful for more tests of
manual fallback for saving.

+  ASSERT_TRUE(IsSavePromptAvailable());
+  passwords_ui_controller_->SavePassword(
+      passwords_ui_controller_->GetPendingPassword().username_value);
+  EXPECT_FALSE(IsSavePromptAvailable());
 }
 
 void BubbleObserver::AcceptUpdatePrompt(
     const autofill::PasswordForm& form) const {
-  ASSERT_TRUE(IsShowingUpdatePrompt());
+  ASSERT_TRUE(WasUpdatePromptShown());
   passwords_ui_controller_->UpdatePassword(form);
-  EXPECT_FALSE(IsShowingUpdatePrompt());
+  EXPECT_FALSE(IsUpdatePromptAvailable());
 }
 
 void BubbleObserver::WaitForAccountChooser() const {
@@ -500,3 +556,16 @@ void PasswordManagerBrowserTestBase::AddHSTSHost(const std::string& host) {
 
   run_loop.Run();
 }
+
+void PasswordManagerBrowserTestBase::CheckThatCredentialsStored(
+    password_manager::TestPasswordStore* password_store,
+    const base::string16& username,
+    const base::string16& password) {
+  auto& passwords_map = password_store->stored_passwords();
+  ASSERT_EQ(1u, passwords_map.size());
+  auto& passwords_vector = passwords_map.begin()->second;
+  ASSERT_EQ(1u, passwords_vector.size());
+  const autofill::PasswordForm& form = passwords_vector[0];
+  EXPECT_EQ(username, form.username_value);
+  EXPECT_EQ(password, form.password_value);
+}