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

chrome/browser/password_manager/password_manager_test_base.h

 // Copyright 2015 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.
 
 #ifndef CHROME_BROWSER_PASSWORD_MANAGER_PASSWORD_MANAGER_TEST_BASE_H_
 #define CHROME_BROWSER_PASSWORD_MANAGER_PASSWORD_MANAGER_TEST_BASE_H_
 
 #include <memory>
 
 #include "base/macros.h"
@@ skipping 47 matching lines @@
   DISALLOW_COPY_AND_ASSIGN(NavigationObserver);
 };
 
 // Observes the save password prompt for a specified WebContents, keeps track of
 // whether or not it is currently shown, and allows accepting saving passwords
 // through it.
 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_;
 
   DISALLOW_COPY_AND_ASSIGN(BubbleObserver);
 };
 
 class PasswordManagerBrowserTestBase : public InProcessBrowserTest {
  public:
   PasswordManagerBrowserTestBase();
@@ skipping 44 matching lines @@
   void CheckElementValue(const std::string& element_id,
                          const std::string& expected_value);
   // Same as above except the element |element_id| is in iframe |iframe_id|
   void CheckElementValue(const std::string& iframe_id,
                          const std::string& element_id,
                          const std::string& expected_value);
 
   // 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();
   content::RenderViewHost* RenderViewHost();
   content::RenderFrameHost* RenderFrameHost();
   net::EmbeddedTestServer& https_test_server() { return https_test_server_; }
   net::MockCertVerifier& mock_cert_verifier() { return mock_cert_verifier_; }
 
  private:
   net::EmbeddedTestServer https_test_server_;
   net::MockCertVerifier mock_cert_verifier_;
   // A tab with some hooks injected.
   content::WebContents* web_contents_;
   DISALLOW_COPY_AND_ASSIGN(PasswordManagerBrowserTestBase);
 };
 
 #endif  // CHROME_BROWSER_PASSWORD_MANAGER_PASSWORD_MANAGER_TEST_BASE_H_