Commit 5a6f540e authored by Friedrich Horschig's avatar Friedrich Horschig Committed by Commit Bot

[Passwords] Show empty state after unlocking account storage

This CL adds an empty state preventing the user from seeing the loading
state forever if no passwords are available after unlocking the account
storage.
An empty state is preferable over closing the dropdown since it provides
the feedback that the requested action (look in account storage) turned
up empty and provides the user with a path to settings.

Thus, the empty state completes the unlock flow:
1. Focus a password field
2. Click on the unlock button
3. Reauth
4. Wait for store to push an update (either new data or delete data)
 a. Use the newly available suggestions.  <-- Already implemented.
 b. See the empty state; go to settings.  <-- This CL


Screenshot for with empty state string:
https://storage.cloud.google.com/chromium-translation-screenshots/5a36eba8302b1aca1911d2a2873ecbc09363d290

See linked bug for a short video.

Bug: 1068555
Change-Id: I419d508e1faa2c230f3e4c85de71a9e08da1df54
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2142113Reviewed-by: default avatarVasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: default avatarChristos Froussios <cfroussios@chromium.org>
Commit-Queue: Friedrich [CET] <fhorschig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#758831}
parent 2d12f816
......@@ -31,6 +31,7 @@
#include "ui/accessibility/ax_node_data.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/base/resource/resource_bundle.h"
#include "ui/gfx/color_palette.h"
#include "ui/gfx/font.h"
#include "ui/gfx/geometry/rect_conversions.h"
#include "ui/gfx/paint_vector_icon.h"
......@@ -78,6 +79,7 @@ constexpr int kIconSize = 16;
constexpr autofill::PopupItemId kItemTypesUsingLeadingIcons[] = {
autofill::PopupItemId::POPUP_ITEM_ID_SHOW_ACCOUNT_CARDS,
autofill::PopupItemId::POPUP_ITEM_ID_ALL_SAVED_PASSWORDS_ENTRY,
autofill::PopupItemId::POPUP_ITEM_ID_PASSWORD_ACCOUNT_STORAGE_EMPTY,
autofill::PopupItemId::POPUP_ITEM_ID_PASSWORD_ACCOUNT_STORAGE_OPT_IN,
autofill::PopupItemId::POPUP_ITEM_ID_PASSWORD_ACCOUNT_STORAGE_RE_SIGNIN,
autofill::PopupItemId::
......@@ -132,6 +134,10 @@ gfx::ImageSkia GetIconImageByName(const std::string& icon_str) {
return gfx::CreateVectorIcon(vector_icons::kSettingsIcon, kIconSize,
gfx::kChromeIconGrey);
}
if (icon_str == "empty") {
return gfx::CreateVectorIcon(omnibox::kHttpIcon, kIconSize,
gfx::kChromeIconGrey);
}
if (icon_str == "google") {
#if BUILDFLAG(GOOGLE_CHROME_BRANDING)
return gfx::CreateVectorIcon(kGoogleGLogoIcon, kIconSize,
......@@ -1097,6 +1103,7 @@ void AutofillPopupViewNativeViews::CreateChildViews() {
case autofill::PopupItemId::POPUP_ITEM_ID_SCAN_CREDIT_CARD:
case autofill::PopupItemId::POPUP_ITEM_ID_CREDIT_CARD_SIGNIN_PROMO:
case autofill::PopupItemId::POPUP_ITEM_ID_ALL_SAVED_PASSWORDS_ENTRY:
case autofill::PopupItemId::POPUP_ITEM_ID_PASSWORD_ACCOUNT_STORAGE_EMPTY:
case autofill::PopupItemId::POPUP_ITEM_ID_HIDE_AUTOFILL_SUGGESTIONS:
case autofill::PopupItemId::POPUP_ITEM_ID_PASSWORD_ACCOUNT_STORAGE_OPT_IN:
case autofill::PopupItemId::
......
......@@ -47,6 +47,7 @@ const struct TypeClicks kClickTestCase[] = {
{autofill::POPUP_ITEM_ID_PASSWORD_ACCOUNT_STORAGE_OPT_IN, 1},
{autofill::POPUP_ITEM_ID_PASSWORD_ACCOUNT_STORAGE_RE_SIGNIN, 1},
{autofill::POPUP_ITEM_ID_PASSWORD_ACCOUNT_STORAGE_OPT_IN_AND_GENERATE, 1},
{autofill::POPUP_ITEM_ID_PASSWORD_ACCOUNT_STORAGE_EMPTY, 1},
};
class TestAXEventObserver : public views::AXEventObserver {
......
......@@ -34,6 +34,7 @@ enum PopupItemId {
POPUP_ITEM_ID_ACCOUNT_STORAGE_PASSWORD_ENTRY = -22,
POPUP_ITEM_ID_ACCOUNT_STORAGE_USERNAME_ENTRY = -23,
POPUP_ITEM_ID_PASSWORD_ACCOUNT_STORAGE_RE_SIGNIN = -24,
POPUP_ITEM_ID_PASSWORD_ACCOUNT_STORAGE_EMPTY = -25,
};
} // namespace autofill
......
......@@ -112,6 +112,7 @@ void ContentPasswordManagerDriver::FillPasswordForm(
}
void ContentPasswordManagerDriver::InformNoSavedCredentials() {
GetPasswordAutofillManager()->OnNoCredentialsFound();
GetPasswordAutofillAgent()->InformNoSavedCredentials();
}
......
......@@ -260,6 +260,16 @@ autofill::Suggestion CreateEntryToReSignin() {
return suggestion;
}
// Entry showing the empty state (i.e. no passwords found in account-storage).
autofill::Suggestion CreateAccountStorageEmptyEntry() {
autofill::Suggestion suggestion(
l10n_util::GetStringUTF16(IDS_PASSWORD_MANAGER_NO_ACCOUNT_STORE_MATCHES));
suggestion.frontend_id =
autofill::POPUP_ITEM_ID_PASSWORD_ACCOUNT_STORAGE_EMPTY;
suggestion.icon = "empty";
return suggestion;
}
bool ContainsOtherThanManagePasswords(
const std::vector<autofill::Suggestion> suggestions) {
return std::any_of(suggestions.begin(), suggestions.end(),
......@@ -278,6 +288,15 @@ bool AreSuggestionForPasswordField(
});
}
bool HasLoadingSuggestion(base::span<const autofill::Suggestion> suggestions,
autofill::PopupItemId item_id) {
return std::any_of(suggestions.begin(), suggestions.end(),
[&item_id](const autofill::Suggestion& suggestion) {
return suggestion.frontend_id == item_id &&
suggestion.is_loading;
});
}
std::vector<autofill::Suggestion> SetUnlockLoadingState(
base::span<const autofill::Suggestion> suggestions,
autofill::PopupItemId unlock_item,
......@@ -329,6 +348,7 @@ void PasswordAutofillManager::DidSelectSuggestion(const base::string16& value,
int identifier) {
ClearPreviewedForm();
if (identifier == autofill::POPUP_ITEM_ID_ALL_SAVED_PASSWORDS_ENTRY ||
identifier == autofill::POPUP_ITEM_ID_PASSWORD_ACCOUNT_STORAGE_EMPTY ||
identifier == autofill::POPUP_ITEM_ID_GENERATE_PASSWORD_ENTRY ||
identifier == autofill::POPUP_ITEM_ID_PASSWORD_ACCOUNT_STORAGE_OPT_IN ||
identifier ==
......@@ -375,7 +395,9 @@ void PasswordAutofillManager::DidAcceptSuggestion(const base::string16& value,
metrics_util::LogPasswordDropdownItemSelected(
PasswordDropdownSelectedOption::kGenerate,
password_client_->IsIncognito());
} else if (identifier == autofill::POPUP_ITEM_ID_ALL_SAVED_PASSWORDS_ENTRY) {
} else if (identifier == autofill::POPUP_ITEM_ID_ALL_SAVED_PASSWORDS_ENTRY ||
identifier ==
autofill::POPUP_ITEM_ID_PASSWORD_ACCOUNT_STORAGE_EMPTY) {
password_client_->NavigateToManagePasswordsPage(
ManagePasswordsReferrer::kPasswordDropdown);
metrics_util::LogContextOfShowAllSavedPasswordsAccepted(
......@@ -461,7 +483,6 @@ void PasswordAutofillManager::OnAddPasswordFillData(
if (!autofill_client_ || autofill_client_->GetPopupSuggestions().empty())
return;
// TODO(https://crbug.com/1043963): Add empty state.
UpdatePopup(BuildSuggestions(base::string16(),
ForPasswordField(AreSuggestionForPasswordField(
autofill_client_->GetPopupSuggestions())),
......@@ -469,6 +490,15 @@ void PasswordAutofillManager::OnAddPasswordFillData(
ShowPasswordSuggestions(true)));
}
void PasswordAutofillManager::OnNoCredentialsFound() {
if (!autofill_client_ ||
!HasLoadingSuggestion(
autofill_client_->GetPopupSuggestions(),
autofill::POPUP_ITEM_ID_PASSWORD_ACCOUNT_STORAGE_OPT_IN))
return;
UpdatePopup({CreateAccountStorageEmptyEntry()});
}
void PasswordAutofillManager::DeleteFillData() {
fill_data_.reset();
if (autofill_client_) {
......@@ -586,6 +616,7 @@ void PasswordAutofillManager::LogMetricsForSuggestions(
for (const auto& suggestion : suggestions) {
switch (suggestion.frontend_id) {
case autofill::POPUP_ITEM_ID_ALL_SAVED_PASSWORDS_ENTRY:
case autofill::PopupItemId::POPUP_ITEM_ID_PASSWORD_ACCOUNT_STORAGE_EMPTY:
metrics_util::LogContextOfShowAllSavedPasswordsShown(
metrics_util::ShowAllSavedPasswordsContext::kPassword);
continue;
......@@ -720,6 +751,8 @@ void PasswordAutofillManager::OnUnlockReauthCompleted(
if (unlock_item ==
autofill::POPUP_ITEM_ID_PASSWORD_ACCOUNT_STORAGE_OPT_IN_AND_GENERATE) {
password_client_->GeneratePassword();
autofill_client_->HideAutofillPopup(
autofill::PopupHidingReason::kAcceptSuggestion);
}
return;
}
......
......@@ -94,6 +94,10 @@ class PasswordAutofillManager : public autofill::AutofillPopupDelegate {
// Called when main frame navigates. Not called for in-page navigations.
void DidNavigateMainFrame();
// Called if no suggestions were found. Assumed to be mutually exclusive with
// |OnAddPasswordFillData|.
void OnNoCredentialsFound();
// A public version of FillSuggestion(), only for use in tests.
bool FillSuggestionForTest(const base::string16& username);
......
......@@ -789,6 +789,9 @@ TEST_F(PasswordAutofillManagerTest,
});
EXPECT_CALL(*client.GetPasswordFeatureManager(),
SetAccountStorageOptIn(true));
EXPECT_CALL(
autofill_client,
HideAutofillPopup(autofill::PopupHidingReason::kAcceptSuggestion));
EXPECT_CALL(client, GeneratePassword());
password_autofill_manager_->DidAcceptSuggestion(
......@@ -796,6 +799,39 @@ TEST_F(PasswordAutofillManagerTest,
autofill::POPUP_ITEM_ID_PASSWORD_ACCOUNT_STORAGE_OPT_IN_AND_GENERATE, 1);
}
// Test that the popup shows an empty state if opted-into an empty store.
TEST_F(PasswordAutofillManagerTest, SuccessfullOptInMayShowEmptyState) {
TestPasswordManagerClient client;
NiceMock<MockAutofillClient> autofill_client;
InitializePasswordAutofillManager(&client, &autofill_client);
client.SetAccountStorageOptIn(true);
const CoreAccountId kAliceId = client.identity_test_env()
.SetUnconsentedPrimaryAccount(kAliceEmail)
.account_id;
testing::Mock::VerifyAndClearExpectations(&autofill_client);
// Only the unlock button was available. After being clicked, it's in a
// loading state which the DeleteFillData() call will end.
Suggestion unlock_suggestion(
/*label=*/"Unlock passwords and fill", /*value=*/"", /*icon=*/"",
/*fronend_id=*/
autofill::POPUP_ITEM_ID_PASSWORD_ACCOUNT_STORAGE_OPT_IN);
unlock_suggestion.is_loading = Suggestion::IsLoading(true);
EXPECT_CALL(autofill_client, GetPopupSuggestions)
.WillRepeatedly(Return(std::vector<Suggestion>{unlock_suggestion}));
EXPECT_CALL(autofill_client,
HideAutofillPopup(autofill::PopupHidingReason::kStaleData));
EXPECT_CALL(
autofill_client,
UpdatePopup(
SuggestionVectorIdsAre(ElementsAreArray(
{autofill::POPUP_ITEM_ID_PASSWORD_ACCOUNT_STORAGE_EMPTY})),
PopupType::kPasswords));
password_autofill_manager_->DeleteFillData();
password_autofill_manager_->OnNoCredentialsFound();
}
// Test that the popup is updated once "opt in and fill" is clicked".
TEST_F(PasswordAutofillManagerTest,
AddOnFillDataAfterOptInAndFillPopulatesPopup) {
......@@ -813,10 +849,11 @@ TEST_F(PasswordAutofillManagerTest,
base::string16 additional_username(base::ASCIIToUTF16("bar.foo@example.com"));
new_data.additional_logins[additional_username] = additional;
EXPECT_CALL(autofill_client, GetPopupSuggestions())
.Times(2)
.WillRepeatedly(Return(CreateTestSuggestions(
/*has_opt_in_and_fill=*/false, /*has_opt_in_and_generate*/ false,
/*has_re_signin=*/false)));
EXPECT_CALL(autofill_client,
HideAutofillPopup(autofill::PopupHidingReason::kStaleData));
EXPECT_CALL(
autofill_client,
UpdatePopup(
......@@ -825,6 +862,8 @@ TEST_F(PasswordAutofillManagerTest,
autofill::POPUP_ITEM_ID_PASSWORD_ENTRY,
autofill::POPUP_ITEM_ID_ALL_SAVED_PASSWORDS_ENTRY}))),
PopupType::kPasswords));
password_autofill_manager_->DeleteFillData();
password_autofill_manager_->OnAddPasswordFillData(new_data);
}
......
......@@ -43,6 +43,9 @@
<message name="IDS_PASSWORD_MANAGER_RE_SIGNIN_ACCOUNT_STORE" desc="A menu item informing signed-out users to sign-in again to use account-stored passwords.">
Sign in to use passwords stored in your Google account
</message>
<message name="IDS_PASSWORD_MANAGER_NO_ACCOUNT_STORE_MATCHES" desc="The menu item indicating that the account store doesn't contain passwords for the current site.">
No password match. Manage all saved passwords.
</message>
<if expr="not use_titlecase">
<message name="IDS_PASSWORD_MANAGER_MANAGE_PASSWORDS" desc="The menu item in the password field drop down that opens the list of saved passwords.">
Manage passwords…
......
5a36eba8302b1aca1911d2a2873ecbc09363d290
\ No newline at end of file
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