[Passwords] Disable Autofill Popups when Touch To Fill is shown

This change implements suppressing the Autofill popups when the Touch
To Fill sheet is shown. Touch To Fill provides more information than
a password autofill dropdown, which is why it should take precedence.

Bug: 948062, 1015789
Change-Id: I052c0f4d85f79da8818cc3b848d078efed4253f5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1868956Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#707762}

chrome/renderer/autofill/fake_mojo_password_manager_driver.cc

@@ -51,16 +51,6 @@ void FakeMojoPasswordManagerDriver::SameDocumentNavigation(
       submission_indication_event;
 }
 
-void FakeMojoPasswordManagerDriver::ShowPasswordSuggestions(
-    base::i18n::TextDirection text_direction,
-    const base::string16& typed_username,
-    int options,
-    const gfx::RectF& bounds) {
-  called_show_pw_suggestions_ = true;
-  show_pw_suggestions_username_ = typed_username;
-  show_pw_suggestions_options_ = options;
-}
-
 void FakeMojoPasswordManagerDriver::RecordSavePasswordProgress(
     const std::string& log) {
   called_record_save_progress_ = true;

chrome/renderer/autofill/fake_mojo_password_manager_driver.h

@@ -34,23 +34,11 @@ class FakeMojoPasswordManagerDriver
   // TODO(crbug.com/948062): Migrate the other methods to GMock as well.
   MOCK_METHOD0(ShowTouchToFill, void());
 
-  bool called_show_pw_suggestions() const {
-    return called_show_pw_suggestions_;
-  }
-
-  const base::Optional<base::string16>& show_pw_suggestions_username() const {
-    return show_pw_suggestions_username_;
-  }
-
-  int show_pw_suggestions_options() const {
-    return show_pw_suggestions_options_;
-  }
-
-  void reset_show_pw_suggestions() {
-    called_show_pw_suggestions_ = false;
-    show_pw_suggestions_username_ = base::nullopt;
-    show_pw_suggestions_options_ = -1;
-  }
+  MOCK_METHOD4(ShowPasswordSuggestions,
+               void(base::i18n::TextDirection,
+                    const base::string16&,
+                    int,
+                    const gfx::RectF&));
 
   bool called_show_not_secure_warning() const {
     return called_show_not_secure_warning_;
@@ -153,11 +141,6 @@ class FakeMojoPasswordManagerDriver
   void SameDocumentNavigation(autofill::mojom::SubmissionIndicatorEvent
                                   submission_indication_event) override;
 
-  void ShowPasswordSuggestions(base::i18n::TextDirection text_direction,
-                               const base::string16& typed_username,
-                               int options,
-                               const gfx::RectF& bounds) override;
-
   void RecordSavePasswordProgress(const std::string& log) override;
 
   void UserModifiedPasswordField() override;
@@ -176,11 +159,6 @@ class FakeMojoPasswordManagerDriver
   void LogFirstFillingResult(uint32_t form_renderer_id,
                              int32_t result) override {}
 
-  // Records whether ShowPasswordSuggestions() gets called.
-  bool called_show_pw_suggestions_ = false;
-  // Records data received via ShowPasswordSuggestions() call.
-  base::Optional<base::string16> show_pw_suggestions_username_;
-  int show_pw_suggestions_options_ = -1;
   // Records whether ShowNotSecureWarning() gets called.
   bool called_show_not_secure_warning_ = false;
   // Records whether PasswordFormSubmitted() gets called.

chrome/renderer/autofill/password_autofill_agent_browsertest.cc

@@ -44,6 +44,10 @@
 #include "third_party/blink/public/web/web_widget.h"
 #include "ui/events/keycodes/keyboard_codes.h"
 
+#if defined(OS_ANDROID)
+#include "components/autofill/core/common/autofill_features.h"
+#endif
+
 #if defined(OS_WIN)
 #include "third_party/blink/public/web/win/web_font_rendering.h"
 #endif
@@ -64,6 +68,9 @@ using blink::WebInputElement;
 using blink::WebLocalFrame;
 using blink::WebString;
 using testing::_;
+using testing::AtMost;
+using testing::Eq;
+using testing::Truly;
 
 namespace {
 
@@ -409,6 +416,13 @@ class PasswordAutofillAgentTest : public ChromeRenderViewTest {
         password_manager::features::kFillOnAccountSelect);
   }
 
+#if defined(OS_ANDROID)
+  void EnableTouchToFillFeature() {
+    scoped_feature_list_.InitAndEnableFeature(
+        autofill::features::kTouchToFillAndroid);
+  }
+#endif
+
   void EnableShowAutofillSignatures() {
     base::CommandLine::ForCurrentProcess()->AppendSwitch(
         switches::kShowAutofillSignatures);
@@ -600,22 +614,14 @@ class PasswordAutofillAgentTest : public ChromeRenderViewTest {
   // the expected flag for the PasswordAutofillManager, whether to show all
   // suggestions, or only those starting with |username|.
   void CheckSuggestions(const std::string& username, bool show_all) {
-    base::RunLoop().RunUntilIdle();
-
-    ASSERT_TRUE(fake_driver_.called_show_pw_suggestions());
-    ASSERT_TRUE(static_cast<bool>(fake_driver_.show_pw_suggestions_username()));
-    EXPECT_EQ(ASCIIToUTF16(username),
-              *(fake_driver_.show_pw_suggestions_username()));
-    EXPECT_EQ(show_all,
-              static_cast<bool>(fake_driver_.show_pw_suggestions_options() &
-                                autofill::SHOW_ALL));
+    auto show_all_matches = [show_all](int options) {
+      return show_all == !!(options & autofill::SHOW_ALL);
+    };
 
-    fake_driver_.reset_show_pw_suggestions();
-  }
-
-  bool GetCalledShowPasswordSuggestions() {
+    EXPECT_CALL(fake_driver_,
+                ShowPasswordSuggestions(_, Eq(ASCIIToUTF16(username)),
+                                        Truly(show_all_matches), _));
     base::RunLoop().RunUntilIdle();
-    return fake_driver_.called_show_pw_suggestions();
   }
 
   void ExpectFieldPropertiesMasks(
@@ -1737,6 +1743,16 @@ TEST_F(PasswordAutofillAgentTest, TryToShowTouchToFillPassword) {
   base::RunLoop().RunUntilIdle();
 }
 
+#if defined(OS_ANDROID)
+TEST_F(PasswordAutofillAgentTest, TouchToFillSuppressesPopups) {
+  SimulateOnFillPasswordForm(fill_data_);
+  SimulateSuggestionChoice(username_element_);
+  EXPECT_CALL(fake_driver_, ShowTouchToFill);
+  EXPECT_CALL(fake_driver_, ShowPasswordSuggestions).Times(0);
+  base::RunLoop().RunUntilIdle();
+}
+#endif
+
 TEST_F(PasswordAutofillAgentTest, DontTryToShowTouchToFillReadonlyPassword) {
   SetElementReadOnly(password_element_, true);
   SimulateOnFillPasswordForm(fill_data_);
@@ -1911,6 +1927,9 @@ TEST_F(PasswordAutofillAgentTest, ClickAndSelect) {
   ClearUsernameAndPasswordFields();
   SimulateOnFillPasswordForm(fill_data_);
   SimulateElementClick(kUsernameName);
+  EXPECT_CALL(fake_driver_, ShowPasswordSuggestions);
+  base::RunLoop().RunUntilIdle();
+
   SimulateSuggestionChoice(username_element_);
   CheckSuggestions(kAliceUsername, true);
 
@@ -1932,6 +1951,8 @@ TEST_F(PasswordAutofillAgentTest, CredentialsOnClick) {
   // Call SimulateElementClick() to produce a user gesture on the page so
   // autofill will actually fill.
   SimulateElementClick(kUsernameName);
+  EXPECT_CALL(fake_driver_, ShowPasswordSuggestions);
+  base::RunLoop().RunUntilIdle();
 
   // Simulate a user clicking on the username element. This should produce a
   // message with all the usernames.
@@ -1941,7 +1962,9 @@ TEST_F(PasswordAutofillAgentTest, CredentialsOnClick) {
   // Now simulate a user typing in an unrecognized username and then
   // clicking on the username element. This should also produce a message with
   // all the usernames.
+  EXPECT_CALL(fake_driver_, ShowPasswordSuggestions).Times(AtMost(3));
   SimulateUsernameTyping("baz");
+
   autofill_agent_->FormControlElementClicked(username_element_, true);
   CheckSuggestions("baz", true);
   ClearUsernameAndPasswordFields();
@@ -1959,12 +1982,14 @@ TEST_F(PasswordAutofillAgentTest, NoCredentialsOnPasswordClick) {
   // Call SimulateElementClick() to produce a user gesture on the page so
   // autofill will actually fill.
   SimulateElementClick(kUsernameName);
+  EXPECT_CALL(fake_driver_, ShowPasswordSuggestions);
+  base::RunLoop().RunUntilIdle();
 
   // Simulate a user clicking on the password element. This should produce no
   // message.
-  fake_driver_.reset_show_pw_suggestions();
   autofill_agent_->FormControlElementClicked(password_element_, false);
-  EXPECT_TRUE(GetCalledShowPasswordSuggestions());
+  EXPECT_CALL(fake_driver_, ShowPasswordSuggestions);
+  base::RunLoop().RunUntilIdle();
 }
 
 // The user types in a username and a password, but then just before sending
@@ -2381,7 +2406,8 @@ TEST_F(PasswordAutofillAgentTest, NotShowPopupPasswordField) {
 
   SimulateSuggestionChoiceOfUsernameAndPassword(
       password_element_, base::string16(), ASCIIToUTF16(kAlicePassword));
-  ASSERT_FALSE(GetCalledShowPasswordSuggestions());
+  EXPECT_CALL(fake_driver_, ShowPasswordSuggestions).Times(0);
+  base::RunLoop().RunUntilIdle();
 }
 
 // Tests with fill-on-account-select enabled that if the username element is
@@ -2534,7 +2560,9 @@ TEST_F(PasswordAutofillAgentTest, PasswordGenerationSupersedesAutofill) {
   SimulateElementClick("new_password");
   base::RunLoop().RunUntilIdle();
   testing::Mock::VerifyAndClearExpectations(&fake_pw_client_);
-  EXPECT_FALSE(GetCalledShowPasswordSuggestions());
+  EXPECT_CALL(fake_driver_, ShowPasswordSuggestions).Times(0);
+  base::RunLoop().RunUntilIdle();
+
   // On destruction the state is updated.
   EXPECT_CALL(fake_pw_client_, GenerationElementLostFocus())
       .Times(testing::AnyNumber());
@@ -3197,6 +3225,8 @@ TEST_F(PasswordAutofillAgentTest, SuggestPasswordFieldSignInForm) {
   // Call SimulateElementClick() to produce a user gesture on the page so
   // autofill will actually fill.
   SimulateElementClick(kUsernameName);
+  EXPECT_CALL(fake_driver_, ShowPasswordSuggestions);
+  base::RunLoop().RunUntilIdle();
 
   // Simulate a user clicking on the password element. This should produce a
   // dropdown with suggestion of all available usernames.
@@ -3218,6 +3248,8 @@ TEST_F(PasswordAutofillAgentTest, SuggestMultiplePasswordFields) {
   // Call SimulateElementClick() to produce a user gesture on the page so
   // autofill will actually fill.
   SimulateElementClick(kUsernameName);
+  EXPECT_CALL(fake_driver_, ShowPasswordSuggestions);
+  base::RunLoop().RunUntilIdle();
 
   // Simulate a user clicking on the password elements. This should produce
   // dropdowns with suggestion of all available usernames.
@@ -3237,10 +3269,10 @@ TEST_F(PasswordAutofillAgentTest, SuggestMultiplePasswordFields) {
 
   // Simulate a user clicking on not autofilled password fields. This should
   // produce no suggestion dropdowns.
-  fake_driver_.reset_show_pw_suggestions();
   SimulateElementClick("newpassword");
   SimulateElementClick("confirmpassword");
-  EXPECT_FALSE(GetCalledShowPasswordSuggestions());
+  EXPECT_CALL(fake_driver_, ShowPasswordSuggestions).Times(0);
+  base::RunLoop().RunUntilIdle();
 
   // But when the user clicks on the autofilled password field again it should
   // still produce a suggestion dropdown.
@@ -3283,6 +3315,8 @@ TEST_F(PasswordAutofillAgentTest, SuggestWhenJavaScriptUpdatesFieldNames) {
   // Call SimulateElementClick() to produce a user gesture on the page so
   // autofill will actually fill.
   SimulateElementClick(kUsernameName);
+  EXPECT_CALL(fake_driver_, ShowPasswordSuggestions);
+  base::RunLoop().RunUntilIdle();
 
   // Simulate a user clicking on the password element. This should produce a
   // dropdown with suggestion of all available usernames.
@@ -3510,9 +3544,9 @@ TEST_F(PasswordAutofillAgentTest,
 TEST_F(PasswordAutofillAgentTest, SuggestLatestCredentials) {
   password_autofill_agent_->FillPasswordForm(fill_data_);
   SimulateElementClick(kPasswordName);
-  EXPECT_TRUE(GetCalledShowPasswordSuggestions());
+  EXPECT_CALL(fake_driver_, ShowPasswordSuggestions);
+  base::RunLoop().RunUntilIdle();
 
-  fake_driver_.reset_show_pw_suggestions();
   // Change fill data
   fill_data_.username_field.value = ASCIIToUTF16("a-changed-username");
 

components/autofill/content/renderer/password_autofill_agent.cc

@@ -849,6 +849,13 @@ bool PasswordAutofillAgent::ShowSuggestions(const WebInputElement& element,
   if (generation_popup_showing)
     return false;
 
+  // Don't call ShowSuggestionPopup if Touch To Fill is currently showing. Since
+  // Touch To Fill in spirit is very similar to a suggestion pop-up, return true
+  // so that the AutofillAgent does not try to show other autofill suggestions
+  // instead.
+  if (touch_to_fill_state_ == TouchToFillState::kIsShowing)
+    return true;
+
   // Chrome should never show more than one account for a password element since
   // this implies that the username element cannot be modified. Thus even if
   // |show_all| is true, check if the element in question is a password element