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

chrome/browser/password_manager/password_manager_test_base.h

Index: chrome/browser/password_manager/password_manager_test_base.h
diff --git a/chrome/browser/password_manager/password_manager_test_base.h b/chrome/browser/password_manager/password_manager_test_base.h
index 288facf38d4c76a9b2282d42e9a10695749d1383..2857aae21c7a22fbc809c41a7ff03ea66500491e 100644
--- a/chrome/browser/password_manager/password_manager_test_base.h
+++ b/chrome/browser/password_manager/password_manager_test_base.h
@@ -65,39 +65,45 @@ class BubbleObserver {
  public:
   explicit BubbleObserver(content::WebContents* web_contents);
 
-  // Checks if the save prompt is being currently shown.
-  bool IsShowingSavePrompt() const;
+  // Checks if the save prompt is being currently available due to both manual
+  // fallback or successful login.
+  bool IsSavePromptAvailable() const;
 
-  // Checks if the update prompt is being currently shown.
-  bool IsShowingUpdatePrompt() const;
+  // Checks if the update prompt is being currently available due to both manual
+  // fallback or successful login.
+  bool IsUpdatePromptAvailable() const;
+
+  // Checks if the save prompt was shown automatically.
+  bool IsSavePromptShownAutomatically() const;
+
+  // Checks if the update prompt was shown automatically.
+  bool IsUpdatePromptShownAutomatically() const;
 
   // Dismisses the prompt currently open and moves the controller to the
   // inactive state.
   void Dismiss() const;
 
-  // Expecting that the prompt is shown, saves the password. Checks that the
-  // prompt is no longer visible afterwards.
-  void AcceptSavePrompt() const;
+  // Saves the password. If |expected_automatic_prompt|, expecting that the
+  // prompt is automatically shown. At the end, checks that the prompt is no
+  // longer available.
+  void AcceptSavePrompt(bool expected_automatic_prompt) const;

[vasilii, 2017/08/07 17:13:24]

No, I don't like this interface. This method isn't about expecting a bubble or
not. It just emulates "Save" click.
If a test has some expectations regarding the bubble like the state is
"pending", the bubble was automatically shown etc. then it should be part of the
test. Actually most of the tests do it anyway already.

[kolos1, 2017/08/08 12:37:16]

Ok, reverted to the old version, but edited the comment. Prompt must be
available, but hasn't to be shown.

 
   // Expecting that the prompt is shown, update |form| with the password from
   // observed form. Checks that the prompt is no longer visible afterwards.
   void AcceptUpdatePrompt(const autofill::PasswordForm& form) const;
 
   // Returns once the account chooser pops up or it's already shown.
-  // |web_contents| must be the custom one returned by
-  // PasswordManagerBrowserTestBase.

[vasilii, 2017/08/07 17:13:24]

Any reason why you don't like this comment? It's very important. Also it's
applicable to your methods

[kolos1, 2017/08/08 12:37:17]

Can we mention here something except the parameters of the method? It looks like
an outdated comment.

[vasilii, 2017/08/08 13:26:31]

Why is it outdated? Everything will crash if you call any Wait* method on a new
tab.

[kolos1, 2017/08/08 13:41:57]

I mean that it is unclear what |web_contents| is. It is not a parameter, there
is even no occurrence of web_contents or so within the method. Well, for someone
who knows the browser tests code well, the comment may make sense. I'll keep it
if you want. 

What if we say something about CustomManagePasswordsUIController or new tab
(preferably)?

[vasilii, 2017/08/08 13:57:42]

|web_contents| is a parameter to the constructor. We can clarify it.
CustomManagePasswordsUIController isn't in the header. "new tab" makes sense
when the reader understands the "old tab".
Anyway, a comment on every method using  CustomManagePasswordsUIController is
required.

[kolos1, 2017/08/08 16:20:06]

The comment is back :)

   void WaitForAccountChooser() const;
 
   // Returns once the UI controller is in the management state due to matching
   // credentials autofilled.
-  // |web_contents| must be the custom one returned by
-  // PasswordManagerBrowserTestBase.
   void WaitForManagementState() const;
 
   // Returns once the save prompt pops up or it's already shown.
-  // |web_contents| must be the custom one returned by
-  // PasswordManagerBrowserTestBase.
-  void WaitForSavePrompt() const;
+  void WaitForAutomaticSavePrompt() const;
+
+  // Returns once the fallback for saving becomes available.
+  void WaitForFallbackForSaving() const;
 
  private:
   ManagePasswordsUIController* const passwords_ui_controller_;
@@ -162,6 +168,11 @@ class PasswordManagerBrowserTestBase : public InProcessBrowserTest {
   // Synchronoulsy adds the given host to the list of valid HSTS hosts.
   void AddHSTSHost(const std::string& host);
 
+  // Checks that |password_store| stores only one credential with |username| and
+  // |password|.
+  void CheckThatCredentialsStored(const base::string16& username,
+                                  const base::string16& password);
+
   // Accessors
   // Return the first created tab with a custom ManagePasswordsUIController.
   content::WebContents* WebContents();