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

components/password_manager/core/browser/password_manager_unittest.cc

Index: components/password_manager/core/browser/password_manager_unittest.cc
diff --git a/components/password_manager/core/browser/password_manager_unittest.cc b/components/password_manager/core/browser/password_manager_unittest.cc
index 12c762a4bc60d557932834dd4f6f8148ef544474..148fea7888e715c91e3ad0b7a0517e39978131e0 100644
--- a/components/password_manager/core/browser/password_manager_unittest.cc
+++ b/components/password_manager/core/browser/password_manager_unittest.cc
@@ -65,6 +65,8 @@ class MockPasswordManagerClient : public StubPasswordManagerClient {
   // The code inside EXPECT_CALL for PromptUserToSaveOrUpdatePasswordPtr owns
   // the PasswordFormManager* argument.
   MOCK_METHOD1(PromptUserToSaveOrUpdatePasswordPtr, void(PasswordFormManager*));
+  MOCK_METHOD2(ShowManualFallbackPtr, void(PasswordFormManager*, bool));
+  MOCK_METHOD0(HideManualFallback, void());
   MOCK_METHOD1(NotifySuccessfulLoginWithExistingPassword,
                void(const autofill::PasswordForm&));
   MOCK_METHOD0(AutomaticPasswordSaveIndicator, void());
@@ -80,6 +82,10 @@ class MockPasswordManagerClient : public StubPasswordManagerClient {
     PromptUserToSaveOrUpdatePasswordPtr(manager.release());
     return false;
   }
+  void ShowManualFallback(std::unique_ptr<PasswordFormManager> manager,
+                          bool is_update) override {
+    ShowManualFallbackPtr(manager.release(), is_update);

[vasilii, 2017/07/21 12:48:21]

Memory leak here?

[kolos1, 2017/07/24 15:33:31]

Fixed.

+  }
   void AutomaticPasswordSave(
       std::unique_ptr<PasswordFormManager> manager) override {
     AutomaticPasswordSaveIndicator();
@@ -113,6 +119,10 @@ ACTION(InvokeEmptyConsumerWithForms) {
 
 ACTION_P(SaveToScopedPtr, scoped) { scoped->reset(arg0); }
 
+ACTION_P(SaveConsumerToList, list) {
+  list->push_back(arg0);
+}
+
 }  // namespace
 
 class PasswordManagerTest : public testing::Test {
@@ -1805,4 +1815,63 @@ TEST_F(PasswordManagerTest, NotSavingSyncPasswordHash_NotSyncCredentials) {
 }
 #endif
 
+TEST_F(PasswordManagerTest, ManualFallbackForSaving) {
+  EXPECT_CALL(client_, IsSavingAndFillingEnabledForCurrentPage())
+      .WillRepeatedly(Return(true));
+
+  std::vector<PasswordForm> observed;
+  PasswordForm form(MakeSimpleForm());
+  observed.push_back(form);
+  PasswordForm stored_form = form;
+  stored_form.password_value = ASCIIToUTF16("old_password");
+  EXPECT_CALL(*store_, GetLogins(_, _))
+      .WillRepeatedly(WithArg<1>(InvokeConsumer(stored_form)));

[vasilii, 2017/07/21 12:48:21]

I'm concerned if it happens more than once. Is it true?

[kolos1, 2017/07/24 15:33:30]

Yes, it is called twice.

In tests above, WillRepeatedly is used.

[vasilii, 2017/07/25 11:17:46]

Why is it called twice if the form is only one?

[kolos1, 2017/07/26 13:40:57]

Investigated the case. Here it is called once actually. Fixed.

In the test below, it is called twice, because of cloning PasswordFormManager
for showing the fallback. It clones FormFetcher too. Since the original 
FormFetcher is still waiting for credentials, the clone will fetch credentials
too.
Fixed the second test and added a comment.

+  EXPECT_CALL(driver_, FillPasswordForm(_)).Times(2);
+  manager()->OnPasswordFormsParsed(&driver_, observed);
+  manager()->OnPasswordFormsRendered(&driver_, observed, true);
+
+  // The username of the stored form is the same, there should be update bubble.
+  EXPECT_CALL(client_, ShowManualFallbackPtr(_, true));

[vasilii, 2017/07/21 12:48:21]

Why don't we check anything about the form being passed

[kolos1, 2017/07/24 15:33:31]

Added some checks.

+  manager()->ShowManualFallback(&driver_, form);
+
+  // The username of the stored form is different, there should be save bubble.
+  PasswordForm new_form = form;
+  new_form.username_value = ASCIIToUTF16("another_username");
+  EXPECT_CALL(client_, ShowManualFallbackPtr(_, false));
+  manager()->ShowManualFallback(&driver_, new_form);
+
+  // Hide the manual fallback.
+  EXPECT_CALL(client_, HideManualFallback());
+  manager()->HideManualFallback();
+}
+
+// Tests that the manual fallback for saving isn't shown if there is no response
+// from the password storage. When crbug.com/741537 is fixed, change this test.
+TEST_F(PasswordManagerTest, ManualFallbackForSaving_SlowBackend) {
+  EXPECT_CALL(client_, IsSavingAndFillingEnabledForCurrentPage())
+      .WillRepeatedly(Return(true));
+
+  std::vector<PasswordForm> observed;
+  PasswordForm form(MakeSimpleForm());
+  observed.push_back(form);
+  std::vector<PasswordStoreConsumer*> consumers;
+  EXPECT_CALL(*store_, GetLogins(_, _))
+      .WillRepeatedly(WithArg<1>(SaveConsumerToList(&consumers)));
+  manager()->OnPasswordFormsParsed(&driver_, observed);
+  manager()->OnPasswordFormsRendered(&driver_, observed, true);
+
+  // There is no response from the store. Don't show the fallback.
+  EXPECT_CALL(client_, ShowManualFallbackPtr(_, _)).Times(0);
+  manager()->ShowManualFallback(&driver_, form);
+
+  // The storage responded. The fallback can be shown.
+  EXPECT_CALL(client_, ShowManualFallbackPtr(_, false));

[vasilii, 2017/07/21 12:48:21]

Move down to ShowManualFallback. Do I understand right that the fallback isn't
shown automatically upon receiving the forms?

[kolos1, 2017/07/24 15:33:31]

Good point. Done.

+  for (PasswordStoreConsumer* consumer : consumers) {
+    ASSERT_TRUE(consumer);
+    consumer->OnGetPasswordStoreResults(
+        std::vector<std::unique_ptr<PasswordForm>>());
+  }
+  manager()->ShowManualFallback(&driver_, form);
+}
+
 }  // namespace password_manager