[Android][Mfill] Hide "Use other password" without saved credentials

This CL defers adding the "Use other [password|username]" button
until the newly introduced helper confirmed that there is at
least one credential to show.

To do that, the helper fetches the passwords when it's created.
If it finishes before suggestions are to be refreshed, the
stored boolean is used, otherwise, the callback is queued and
will trigger an update.

Smaller clean-ups:
* New test fixture for filling (mostly moved to save setup code
  and to prevent ScopedFeatureList issues)
* Moved accessory sheet state (focused field) into helper since
  frame state shouldn't be used by the accessory controller
  (if possible but required by all pwds sheet)

Bug: 1104132
Change-Id: I4fa5757744dda5c2091fec24daf0654f92b2be46
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2489914
Commit-Queue: Friedrich [CET] <fhorschig@chromium.org>
Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#825468}

chrome/browser/BUILD.gn

@@ -3005,6 +3005,8 @@ static_library("browser") {
       "password_manager/android/account_chooser_dialog_android.h",
       "password_manager/android/all_passwords_bottom_sheet_controller.cc",
       "password_manager/android/all_passwords_bottom_sheet_controller.h",
+      "password_manager/android/all_passwords_bottom_sheet_helper.cc",
+      "password_manager/android/all_passwords_bottom_sheet_helper.h",
       "password_manager/android/auto_signin_first_run_dialog_android.cc",
       "password_manager/android/auto_signin_first_run_dialog_android.h",
       "password_manager/android/auto_signin_prompt_controller.cc",

chrome/browser/password_manager/android/all_passwords_bottom_sheet_helper.cc

+// Copyright 2020 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.
+
+#include "chrome/browser/password_manager/android/all_passwords_bottom_sheet_helper.h"
+
+#include "components/password_manager/core/browser/password_store.h"
+
+AllPasswordsBottomSheetHelper::AllPasswordsBottomSheetHelper(
+    password_manager::PasswordStore* store) {
+  DCHECK(store);
+  store->GetAllLoginsWithAffiliationAndBrandingInformation(this);
+}
+
+AllPasswordsBottomSheetHelper::~AllPasswordsBottomSheetHelper() = default;
+
+void AllPasswordsBottomSheetHelper::SetLastFocusedFieldType(
+    autofill::mojom::FocusedFieldType focused_field_type) {
+  last_focused_field_type_ = focused_field_type;
+}
+
+void AllPasswordsBottomSheetHelper::SetUpdateCallback(
+    base::OnceClosure update_callback) {
+  DCHECK(update_callback);
+  update_callback_ = std::move(update_callback);
+}
+
+void AllPasswordsBottomSheetHelper::ClearUpdateCallback() {
+  update_callback_.Reset();
+}
+
+void AllPasswordsBottomSheetHelper::OnGetPasswordStoreResults(
+    std::vector<std::unique_ptr<password_manager::PasswordForm>> results) {
+  available_credentials_ = results.size();
+  if (results.empty())
+    return;  // Don't update if sheet still wouldn't be available.
+  if (update_callback_.is_null())
+    return;  // No update if cannot be triggered right now.
+  if (last_focused_field_type_ == autofill::mojom::FocusedFieldType::kUnknown)
+    return;  // Don't update if no valid field was focused.
+  std::move(update_callback_).Run();
+}

chrome/browser/password_manager/android/all_passwords_bottom_sheet_helper.h

+// Copyright 2020 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_ANDROID_ALL_PASSWORDS_BOTTOM_SHEET_HELPER_H_
+#define CHROME_BROWSER_PASSWORD_MANAGER_ANDROID_ALL_PASSWORDS_BOTTOM_SHEET_HELPER_H_
+
+#include "base/callback_forward.h"
+#include "base/optional.h"
+#include "components/autofill/core/common/mojom/autofill_types.mojom-shared.h"
+#include "components/password_manager/core/browser/password_store_consumer.h"
+
+// This class helps to determine the visibility of the "All Passwords Sheet"
+// button by requesting whether there are any passwords stored at all.
+// When created, it will immediately query the store for passwords and store the
+// result. If provided with an eligible focused field and a callback to update,
+// the helper will trigger an update right away if passwords are found.
+class AllPasswordsBottomSheetHelper
+    : public password_manager::PasswordStoreConsumer {
+ public:
+  explicit AllPasswordsBottomSheetHelper(
+      password_manager::PasswordStore* store);
+  AllPasswordsBottomSheetHelper(const AllPasswordsBottomSheetHelper&) = delete;
+  AllPasswordsBottomSheetHelper& operator=(
+      const AllPasswordsBottomSheetHelper&) = delete;
+  ~AllPasswordsBottomSheetHelper() override;
+
+  // Returns the number of found credentials only if the helper already finished
+  // querying the password store.
+  base::Optional<size_t> available_credentials() const {
+    return available_credentials_;
+  }
+  autofill::mojom::FocusedFieldType last_focused_field_type() const {
+    return last_focused_field_type_;
+  }
+
+  void SetLastFocusedFieldType(
+      autofill::mojom::FocusedFieldType focused_field_type);
+  void SetUpdateCallback(base::OnceClosure update_callback);
+  void ClearUpdateCallback();
+
+ private:
+  // PasswordStoreConsumer:
+  void OnGetPasswordStoreResults(
+      std::vector<std::unique_ptr<password_manager::PasswordForm>> results)
+      override;
+
+  // A callback used to update the suggestions if the password store provides
+  // credentials and the focused field might still profit from them.
+  base::OnceClosure update_callback_;
+
+  // Stores whether the store returned credentials the sheet can show.
+  base::Optional<size_t> available_credentials_ = base::nullopt;
+
+  // Records the last focused field type to infer whether an update should be
+  // triggered if the store returns suggestions.
+  autofill::mojom::FocusedFieldType last_focused_field_type_ =
+      autofill::mojom::FocusedFieldType::kUnknown;
+};
+
+#endif  // CHROME_BROWSER_PASSWORD_MANAGER_ANDROID_ALL_PASSWORDS_BOTTOM_SHEET_HELPER_H_

chrome/browser/password_manager/android/all_passwords_bottom_sheet_helper_unittest.cc

+// Copyright 2020 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.
+
+#include "chrome/browser/password_manager/android/all_passwords_bottom_sheet_helper.h"
+
+#include "base/strings/string_piece_forward.h"
+#include "base/strings/utf_string_conversions.h"
+#include "base/test/mock_callback.h"
+#include "chrome/browser/password_manager/password_manager_test_util.h"
+#include "chrome/test/base/testing_profile.h"
+#include "components/autofill/core/common/mojom/autofill_types.mojom-shared.h"
+#include "components/password_manager/core/browser/password_form.h"
+#include "components/password_manager/core/browser/test_password_store.h"
+#include "content/public/test/browser_task_environment.h"
+
+using password_manager::PasswordForm;
+using password_manager::TestPasswordStore;
+
+constexpr char kExampleCom[] = "https://example.com";
+constexpr char kUsername[] = "alice";
+constexpr char kPassword[] = "password123";
+
+namespace {
+PasswordForm MakeSavedPassword(base::StringPiece signon_realm,
+                               base::StringPiece username) {
+  PasswordForm form;
+  form.signon_realm = std::string(signon_realm);
+  form.url = GURL(signon_realm);
+  form.username_value = base::ASCIIToUTF16(username);
+  form.password_value = base::ASCIIToUTF16(kPassword);
+  form.in_store = PasswordForm::Store::kProfileStore;
+  return form;
+}
+}  // namespace
+
+class AllPasswordsBottomSheetHelperTest : public testing::Test {
+ protected:
+  TestPasswordStore& store() { return *store_; }
+
+  void RunUntilIdle() { task_env_.RunUntilIdle(); }
+
+ private:
+  content::BrowserTaskEnvironment task_env_;
+  TestingProfile profile_;
+  scoped_refptr<TestPasswordStore> store_ =
+      CreateAndUseTestPasswordStore(&profile_);
+};
+
+TEST_F(AllPasswordsBottomSheetHelperTest, CallbackIsCalledAfterFetch) {
+  store().AddLogin(MakeSavedPassword(kExampleCom, kUsername));
+
+  base::MockOnceClosure callback;
+  EXPECT_CALL(callback, Run);
+
+  AllPasswordsBottomSheetHelper all_passwords_helper(&store());
+  all_passwords_helper.SetUpdateCallback(callback.Get());
+  all_passwords_helper.SetLastFocusedFieldType(
+      autofill::mojom::FocusedFieldType::kFillableUsernameField);
+
+  RunUntilIdle();
+}
+
+TEST_F(AllPasswordsBottomSheetHelperTest, CallbackIsNotCalledForEmptyStore) {
+  base::MockOnceClosure callback;
+  EXPECT_CALL(callback, Run).Times(0);
+
+  AllPasswordsBottomSheetHelper all_passwords_helper(&store());
+  all_passwords_helper.SetUpdateCallback(callback.Get());
+  all_passwords_helper.SetLastFocusedFieldType(
+      autofill::mojom::FocusedFieldType::kFillableUsernameField);
+
+  RunUntilIdle();
+}
+
+TEST_F(AllPasswordsBottomSheetHelperTest, CallbackIsNotCalledIfUnset) {
+  store().AddLogin(MakeSavedPassword(kExampleCom, kUsername));
+
+  base::MockOnceClosure callback;
+  EXPECT_CALL(callback, Run).Times(0);
+
+  AllPasswordsBottomSheetHelper all_passwords_helper(&store());
+  all_passwords_helper.SetLastFocusedFieldType(
+      autofill::mojom::FocusedFieldType::kFillableUsernameField);
+
+  RunUntilIdle();
+}
+TEST_F(AllPasswordsBottomSheetHelperTest, CallbackIsNotCalledForUnknownFields) {
+  store().AddLogin(MakeSavedPassword(kExampleCom, kUsername));
+
+  base::MockOnceClosure callback;
+  EXPECT_CALL(callback, Run).Times(0);
+
+  AllPasswordsBottomSheetHelper all_passwords_helper(&store());
+  all_passwords_helper.SetUpdateCallback(callback.Get());
+  all_passwords_helper.SetLastFocusedFieldType(
+      autofill::mojom::FocusedFieldType::kUnknown);
+
+  RunUntilIdle();
+}

chrome/browser/password_manager/android/password_accessory_controller_impl.cc

@@ -242,7 +242,8 @@ void PasswordAccessoryControllerImpl::OnToggleChanged(
 void PasswordAccessoryControllerImpl::RefreshSuggestionsForField(
     FocusedFieldType focused_field_type,
     bool is_manual_generation_available) {
-  last_focused_field_type_ = focused_field_type;
+  all_passwords_helper_.SetLastFocusedFieldType(focused_field_type);
+
   // Prevent crashing by not acting at all if frame became unfocused at any
   // point. The next time a focus event happens, this will be called again and
   // ensure we show correct data.
@@ -272,9 +273,18 @@ void PasswordAccessoryControllerImpl::RefreshSuggestionsForField(
     }
   }
 
-  if (IsSecureSite() && origin.GetURL().SchemeIsCryptographic() &&
-      base::FeatureList::IsEnabled(
-          password_manager::features::kFillingPasswordsFromAnyOrigin)) {
+  all_passwords_helper_.ClearUpdateCallback();
+  if (!all_passwords_helper_.available_credentials().has_value()) {
+    // Don't add the button yet but wait for helper to determine whether there
+    // are credentials at all.
+    all_passwords_helper_.SetUpdateCallback(base::BindOnce(
+        &PasswordAccessoryControllerImpl::RefreshSuggestionsForField,
+        base::Unretained(this), focused_field_type,
+        is_manual_generation_available));
+  } else if (IsSecureSite() && origin.GetURL().SchemeIsCryptographic() &&
+             all_passwords_helper_.available_credentials().value() > 0 &&
+             base::FeatureList::IsEnabled(
+                 password_manager::features::kFillingPasswordsFromAnyOrigin)) {
     base::string16 button_title =
         is_password_field
             ? l10n_util::GetStringUTF16(
@@ -432,7 +442,7 @@ void PasswordAccessoryControllerImpl::ShowAllPasswords() {
           base::BindOnce(
               &PasswordAccessoryControllerImpl::AllPasswordsSheetDismissed,
               base::Unretained(this)),
-          last_focused_field_type_);
+          all_passwords_helper_.last_focused_field_type());
 
   all_passords_bottom_sheet_controller_->Show();
 }

chrome/browser/password_manager/android/password_accessory_controller_impl.h

@@ -14,6 +14,7 @@
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/task/cancelable_task_tracker.h"
+#include "chrome/browser/password_manager/android/all_passwords_bottom_sheet_helper.h"
 #include "chrome/browser/password_manager/android/password_accessory_controller.h"
 #include "components/autofill/core/browser/ui/accessory_sheet_data.h"
 #include "components/autofill/core/common/mojom/autofill_types.mojom-forward.h"
@@ -153,9 +154,9 @@ class PasswordAccessoryControllerImpl
   std::unique_ptr<AllPasswordsBottomSheetController>
       all_passords_bottom_sheet_controller_;
 
-  // Records the last focused field type that `RefreshSuggestionsForField()` was
-  // called with.
-  autofill::mojom::FocusedFieldType last_focused_field_type_;
+  // Helper for determining whether a bottom sheet showing passwords is useful.
+  AllPasswordsBottomSheetHelper all_passwords_helper_{
+      password_client_->GetProfilePasswordStore()};
 
   // Security level used for testing only.
   security_state::SecurityLevel security_level_for_testing_ =

chrome/browser/password_manager/chrome_password_manager_client_unittest.cc

@@ -671,24 +671,24 @@ TEST_F(ChromePasswordManagerClientTest, CanShowBubbleOnURL) {
     const char* scheme;
     bool can_show_bubble;
   } kTestCases[] = {
-      {url::kHttpScheme, true},
-      {url::kHttpsScheme, true},
-      {url::kFtpScheme, true},
-      {url::kDataScheme, true},
-      {"feed", true},
-      {url::kBlobScheme, true},
-      {url::kFileSystemScheme, true},
-
-      {"invalid-scheme-i-just-made-up", false},
+    {url::kHttpScheme, true},
+    {url::kHttpsScheme, true},
+    {url::kFtpScheme, true},
+    {url::kDataScheme, true},
+    {"feed", true},
+    {url::kBlobScheme, true},
+    {url::kFileSystemScheme, true},
+
+    {"invalid-scheme-i-just-made-up", false},
 #if BUILDFLAG(ENABLE_EXTENSIONS)
-      {extensions::kExtensionScheme, false},
+    {extensions::kExtensionScheme, false},
 #endif
-      {url::kAboutScheme, false},
-      {content::kChromeDevToolsScheme, false},
-      {content::kChromeUIScheme, false},
-      {url::kJavaScriptScheme, false},
-      {url::kMailToScheme, false},
-      {content::kViewSourceScheme, false},
+    {url::kAboutScheme, false},
+    {content::kChromeDevToolsScheme, false},
+    {content::kChromeUIScheme, false},
+    {url::kJavaScriptScheme, false},
+    {url::kMailToScheme, false},
+    {content::kViewSourceScheme, false},
   };
 
   for (const TestCase& test_case : kTestCases) {
@@ -806,6 +806,8 @@ class ChromePasswordManagerClientAndroidTest
   std::unique_ptr<password_manager::ContentPasswordManagerDriver>
   CreateContentPasswordManagerDriver(content::RenderFrameHost* rfh);
 
+  void SetUp() override;
+
   void CreateManualFillingController(content::WebContents* web_contents);
 
   ManualFillingControllerImpl* controller() {
@@ -823,6 +825,15 @@ class ChromePasswordManagerClientAndroidTest
   NiceMock<MockCreditCardAccessoryController> mock_cc_controller_;
 };
 
+void ChromePasswordManagerClientAndroidTest::SetUp() {
+  ChromePasswordManagerClientTest::SetUp();
+  PasswordStoreFactory::GetInstance()->SetTestingFactory(
+      GetBrowserContext(),
+      base::BindRepeating(
+          &password_manager::BuildPasswordStore<
+              content::BrowserContext, password_manager::MockPasswordStore>));
+}
+
 std::unique_ptr<password_manager::ContentPasswordManagerDriver>
 ChromePasswordManagerClientAndroidTest::CreateContentPasswordManagerDriver(
     content::RenderFrameHost* rfh) {

chrome/test/BUILD.gn

@@ -4313,6 +4313,7 @@ test("unit_tests") {
       "../browser/page_load_metrics/observers/android_page_load_metrics_observer_unittest.cc",
       "../browser/password_manager/android/account_chooser_dialog_android_unittest.cc",
       "../browser/password_manager/android/all_passwords_bottom_sheet_controller_unittest.cc",
+      "../browser/password_manager/android/all_passwords_bottom_sheet_helper_unittest.cc",
       "../browser/password_manager/android/auto_signin_first_run_dialog_android_unittest.cc",
       "../browser/password_manager/android/credential_leak_controller_android_unittest.cc",
       "../browser/password_manager/android/password_accessory_controller_impl_unittest.cc",

components/password_manager/core/browser/mock_password_store.h

@@ -97,6 +97,8 @@ class MockPasswordStore : public PasswordStore {
   MOCK_METHOD0(GetAllFieldInfoImpl, std::vector<FieldInfo>());
   MOCK_METHOD2(RemoveFieldInfoByTimeImpl, void(base::Time, base::Time));
   MOCK_METHOD0(IsEmpty, bool());
+  MOCK_METHOD1(GetAllLoginsWithAffiliationAndBrandingInformation,
+               void(PasswordStoreConsumer*));
 
   MOCK_CONST_METHOD0(IsAbleToSavePasswords, bool());