Commit f8b0c3b3 authored by Friedrich Horschig's avatar Friedrich Horschig Committed by Commit Bot

[Android][Mfill] Cache subsqeuent accessory sheet updates

Currently, the keyboard accessory may show stale suggestions. This is
because sheets are cached by adding them with `emplace` to a flat_map.
`emplace` only adds an item if it isn't present, so the oldest,
non-empty sheet (for a given tab) will be shown forever. This isn't
immediately obvious since sheets are also pushed directly when they are
updated but with indirect updates (e.g. updating another sheet), the
stale value will override the new one.

By using `insert_or_assign`, the newer value replaces the old one.

Bug: 1140358, 1134119
Change-Id: I933ec47a1da596f58fa99d5b9bc481954122c2a3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2489899Reviewed-by: default avatarVasilii Sukhanov <vasilii@chromium.org>
Commit-Queue: Friedrich [CET] <fhorschig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#820165}
parent 8a1b573d
......@@ -110,8 +110,8 @@ void ManualFillingControllerImpl::OnAutomaticGenerationStatusChanged(
void ManualFillingControllerImpl::RefreshSuggestions(
const AccessorySheetData& accessory_sheet_data) {
view_->OnItemsAvailable(accessory_sheet_data);
available_sheets_.emplace(GetSourceForTab(accessory_sheet_data),
accessory_sheet_data);
available_sheets_.insert_or_assign(GetSourceForTab(accessory_sheet_data),
accessory_sheet_data);
UpdateSourceAvailability(GetSourceForTab(accessory_sheet_data),
!accessory_sheet_data.user_info_list().empty());
}
......
......@@ -23,6 +23,7 @@
#include "chrome/browser/autofill/mock_password_accessory_controller.h"
#include "chrome/browser/password_manager/android/password_accessory_controller.h"
#include "chrome/test/base/testing_profile.h"
#include "components/autofill/core/browser/ui/accessory_sheet_data.h"
#include "components/autofill/core/browser/ui/accessory_sheet_enums.h"
#include "components/autofill/core/common/autofill_features.h"
#include "components/password_manager/core/common/password_manager_features.h"
......@@ -36,9 +37,11 @@ using autofill::AccessoryAction;
using autofill::AccessorySheetData;
using autofill::AccessoryTabType;
using autofill::mojom::FocusedFieldType;
using base::ASCIIToUTF16;
using testing::_;
using testing::AnyNumber;
using testing::NiceMock;
using testing::SaveArg;
using testing::StrictMock;
using testing::WithArgs;
using FillingSource = ManualFillingController::FillingSource;
......@@ -49,6 +52,16 @@ AccessorySheetData empty_passwords_sheet() {
base::ASCIIToUTF16(kTitle));
}
AccessorySheetData filled_passwords_sheet() {
return AccessorySheetData::Builder(AccessoryTabType::PASSWORDS,
ASCIIToUTF16("Pwds"))
.AddUserInfo("example.com", autofill::UserInfo::IsPslMatch(false))
.AppendField(ASCIIToUTF16("Ben"), ASCIIToUTF16("Ben"), false, true)
.AppendField(ASCIIToUTF16("S3cur3"), ASCIIToUTF16("Ben's PW"), true,
false)
.Build();
}
AccessorySheetData populate_sheet(AccessoryTabType type) {
constexpr char kTitle[] = "Suggestions available!";
return AccessorySheetData::Builder(type, base::ASCIIToUTF16(kTitle))
......@@ -288,6 +301,22 @@ TEST_F(ManualFillingControllerTest, OnFillingTriggeredFillsAndClosesSheet) {
controller()->OnFillingTriggered(AccessoryTabType::PASSWORDS, field);
}
TEST_F(ManualFillingControllerTest, RefreshingUpdatesCache) {
// First, focus a field for which suggestions exist.
FocusFieldAndClearExpectations(FocusedFieldType::kFillableUsernameField);
controller()->RefreshSuggestions(populate_sheet(AccessoryTabType::PASSWORDS));
// Now, focus a field (e.g. in an iframe) with different suggestions.
FocusFieldAndClearExpectations(FocusedFieldType::kFillablePasswordField);
controller()->RefreshSuggestions(filled_passwords_sheet());
// Triggering a subsequent (independent) update must reuse the latest sheet.
AccessorySheetData cached(AccessoryTabType::PASSWORDS, base::string16());
EXPECT_CALL(*view(), OnItemsAvailable).WillOnce(SaveArg<0>(&cached));
FocusFieldAndClearExpectations(FocusedFieldType::kFillablePasswordField);
EXPECT_EQ(cached, filled_passwords_sheet());
}
TEST_F(ManualFillingControllerTest, ForwardsPasswordManagingToController) {
EXPECT_CALL(mock_pwd_controller_,
OnOptionSelected(AccessoryAction::MANAGE_PASSWORDS));
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment