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

[Android] Send accessory password sheet caption upon creation

This CL sends the corrected strings for passwords right upon creation.
That fixes the issue of having plain white sheets when only the
password generation option is available.

(Later, this same call will also provide the manual password generation
and a link to all passwords.)

Bug: 835234
Change-Id: Ie38b76231e371f6d4bb88b2152c04a3f47dd98cc
Reviewed-on: https://chromium-review.googlesource.com/1110117
Commit-Queue: Friedrich Horschig <fhorschig@chromium.org>
Reviewed-by: default avatarVasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569583}
parent afe93f45
...@@ -5289,10 +5289,10 @@ the Bookmarks menu."> ...@@ -5289,10 +5289,10 @@ the Bookmarks menu.">
Password for <ph name="username">$1<ex>Alexander</ex></ph> Password for <ph name="username">$1<ex>Alexander</ex></ph>
</message> </message>
<message name="IDS_PASSWORD_MANAGER_ACCESSORY_PASSWORD_LIST_EMPTY_MESSAGE" desc="Message indicating that the list of saved passwords for this site is empty."> <message name="IDS_PASSWORD_MANAGER_ACCESSORY_PASSWORD_LIST_EMPTY_MESSAGE" desc="Message indicating that the list of saved passwords for this site is empty.">
No saved passwords for this site. No saved passwords for <ph name="domain">$1<ex>example.com</ex></ph>
</message> </message>
<message name="IDS_PASSWORD_MANAGER_ACCESSORY_PASSWORD_LIST_TITLE" desc="The title of a list of saved passwords for the currently visible site."> <message name="IDS_PASSWORD_MANAGER_ACCESSORY_PASSWORD_LIST_TITLE" desc="The title of a list of saved passwords for the currently visible site.">
Passwords Saved passwords for <ph name="domain">$1<ex>example.com</ex></ph>
</message> </message>
</if> </if>
......
...@@ -555,6 +555,13 @@ void ChromePasswordManagerClient::DidFinishNavigation( ...@@ -555,6 +555,13 @@ void ChromePasswordManagerClient::DidFinishNavigation(
web_contents()->GetRenderViewHost()->GetWidget()->RemoveInputEventObserver( web_contents()->GetRenderViewHost()->GetWidget()->RemoveInputEventObserver(
this); this);
web_contents()->GetRenderViewHost()->GetWidget()->AddInputEventObserver(this); web_contents()->GetRenderViewHost()->GetWidget()->AddInputEventObserver(this);
#else
// Ensure that entries from old origins are properly cleaned up.
PasswordAccessoryController* password_accessory =
PasswordAccessoryController::FromWebContents(web_contents());
if (password_accessory) {
password_accessory->DidNavigateMainFrame();
}
#endif #endif
} }
......
...@@ -6,6 +6,7 @@ ...@@ -6,6 +6,7 @@
#include <vector> #include <vector>
#include "base/strings/utf_string_conversions.h"
#include "chrome/browser/password_manager/password_accessory_view_interface.h" #include "chrome/browser/password_manager/password_accessory_view_interface.h"
#include "chrome/browser/ui/passwords/manage_passwords_view_utils.h" #include "chrome/browser/ui/passwords/manage_passwords_view_utils.h"
#include "chrome/grit/generated_resources.h" #include "chrome/grit/generated_resources.h"
...@@ -57,16 +58,14 @@ void PasswordAccessoryController::OnPasswordsAvailable( ...@@ -57,16 +58,14 @@ void PasswordAccessoryController::OnPasswordsAvailable(
} }
DCHECK(view_); DCHECK(view_);
std::vector<Item> items; std::vector<Item> items;
base::string16 passwords_title_str = l10n_util::GetStringUTF16( base::string16 passwords_title_str;
IDS_PASSWORD_MANAGER_ACCESSORY_PASSWORD_LIST_TITLE); passwords_title_str = l10n_util::GetStringFUTF16(
best_matches.empty()
? IDS_PASSWORD_MANAGER_ACCESSORY_PASSWORD_LIST_EMPTY_MESSAGE
: IDS_PASSWORD_MANAGER_ACCESSORY_PASSWORD_LIST_TITLE,
base::ASCIIToUTF16(origin.host()));
items.emplace_back(passwords_title_str, passwords_title_str, items.emplace_back(passwords_title_str, passwords_title_str,
/*is_password=*/false, Item::Type::LABEL); /*is_password=*/false, Item::Type::LABEL);
if (best_matches.empty()) {
base::string16 passwords_empty_str = l10n_util::GetStringUTF16(
IDS_PASSWORD_MANAGER_ACCESSORY_PASSWORD_LIST_EMPTY_MESSAGE);
items.emplace_back(passwords_empty_str, passwords_empty_str,
/*is_password=*/false, Item::Type::LABEL);
}
for (const auto& pair : best_matches) { for (const auto& pair : best_matches) {
const PasswordForm* form = pair.second; const PasswordForm* form = pair.second;
base::string16 username = GetDisplayUsername(*form); base::string16 username = GetDisplayUsername(*form);
...@@ -81,6 +80,13 @@ void PasswordAccessoryController::OnPasswordsAvailable( ...@@ -81,6 +80,13 @@ void PasswordAccessoryController::OnPasswordsAvailable(
view_->OnItemsAvailable(origin, items); view_->OnItemsAvailable(origin, items);
} }
void PasswordAccessoryController::DidNavigateMainFrame() {
// Sending no passwords removes stale stuggestions and sends default options.
OnPasswordsAvailable(
/*best_matches=*/{},
web_contents_->GetMainFrame()->GetLastCommittedOrigin().GetURL());
}
void PasswordAccessoryController::OnFillingTriggered( void PasswordAccessoryController::OnFillingTriggered(
bool is_password, bool is_password,
const base::string16& textToFill) const { const base::string16& textToFill) const {
......
...@@ -46,6 +46,9 @@ class PasswordAccessoryController ...@@ -46,6 +46,9 @@ class PasswordAccessoryController
best_matches, best_matches,
const GURL& origin); const GURL& origin);
// Send empty suggestions and default options to the view.
void DidNavigateMainFrame();
// Called by the UI code to request that |textToFill| is to be filled into the // Called by the UI code to request that |textToFill| is to be filled into the
// currently focused field. // currently focused field.
void OnFillingTriggered(bool is_password, void OnFillingTriggered(bool is_password,
...@@ -75,7 +78,7 @@ class PasswordAccessoryController ...@@ -75,7 +78,7 @@ class PasswordAccessoryController
content::WebContents* web_contents, content::WebContents* web_contents,
std::unique_ptr<PasswordAccessoryViewInterface> view); std::unique_ptr<PasswordAccessoryViewInterface> view);
// The web page view this accessory sheet and the focused field live in. // The tab for which this class is scoped.
content::WebContents* web_contents_; content::WebContents* web_contents_;
// Hold the native instance of the view. Must be last declared and initialized // Hold the native instance of the view. Must be last declared and initialized
......
...@@ -32,6 +32,9 @@ using testing::StrictMock; ...@@ -32,6 +32,9 @@ using testing::StrictMock;
using AccessoryItem = PasswordAccessoryViewInterface::AccessoryItem; using AccessoryItem = PasswordAccessoryViewInterface::AccessoryItem;
using ItemType = AccessoryItem::Type; using ItemType = AccessoryItem::Type;
constexpr char kExampleSite[] = "https://example.com";
constexpr char kExampleDomain[] = "example.com";
// The mock view mocks the platform-specific implementation. That also means // The mock view mocks the platform-specific implementation. That also means
// that we have to care about the lifespan of the Controller because that would // that we have to care about the lifespan of the Controller because that would
// usually be responsibility of the view. // usually be responsibility of the view.
...@@ -62,6 +65,12 @@ std::string PrintItem(const base::string16& text, ...@@ -62,6 +65,12 @@ std::string PrintItem(const base::string16& text,
PrintToString(static_cast<int>(type)); PrintToString(static_cast<int>(type));
} }
// Compares whether a given AccessoryItem is a label with the given text.
MATCHER_P(MatchesLabel, text, PrintItem(text, text, false, ItemType::LABEL)) {
return arg.text == text && arg.is_password == false &&
arg.content_description == text && arg.itemType == ItemType::LABEL;
}
// Compares whether a given AccessoryItem had the given properties. // Compares whether a given AccessoryItem had the given properties.
MATCHER_P4(MatchesItem, MATCHER_P4(MatchesItem,
text, text,
...@@ -98,14 +107,15 @@ base::string16 password_for_str(const std::string& user) { ...@@ -98,14 +107,15 @@ base::string16 password_for_str(const std::string& user) {
return password_for_str(ASCIIToUTF16(user)); return password_for_str(ASCIIToUTF16(user));
} }
base::string16 passwords_empty_str() { base::string16 passwords_empty_str(const std::string& domain) {
return l10n_util::GetStringUTF16( return l10n_util::GetStringFUTF16(
IDS_PASSWORD_MANAGER_ACCESSORY_PASSWORD_LIST_EMPTY_MESSAGE); IDS_PASSWORD_MANAGER_ACCESSORY_PASSWORD_LIST_EMPTY_MESSAGE,
ASCIIToUTF16(domain));
} }
base::string16 passwords_title_str() { base::string16 passwords_title_str(const std::string& domain) {
return l10n_util::GetStringUTF16( return l10n_util::GetStringFUTF16(
IDS_PASSWORD_MANAGER_ACCESSORY_PASSWORD_LIST_TITLE); IDS_PASSWORD_MANAGER_ACCESSORY_PASSWORD_LIST_TITLE, ASCIIToUTF16(domain));
} }
base::string16 no_user_str() { base::string16 no_user_str() {
...@@ -136,6 +146,7 @@ class PasswordAccessoryControllerTest : public ChromeRenderViewHostTestHarness { ...@@ -136,6 +146,7 @@ class PasswordAccessoryControllerTest : public ChromeRenderViewHostTestHarness {
public: public:
void SetUp() override { void SetUp() override {
ChromeRenderViewHostTestHarness::SetUp(); ChromeRenderViewHostTestHarness::SetUp();
NavigateAndCommit(GURL(kExampleSite));
PasswordAccessoryController::CreateForWebContentsForTesting( PasswordAccessoryController::CreateForWebContentsForTesting(
web_contents(), web_contents(),
std::make_unique<StrictMock<MockPasswordAccessoryView>>()); std::make_unique<StrictMock<MockPasswordAccessoryView>>());
...@@ -164,44 +175,40 @@ TEST_F(PasswordAccessoryControllerTest, TransformsMatchesToSuggestions) { ...@@ -164,44 +175,40 @@ TEST_F(PasswordAccessoryControllerTest, TransformsMatchesToSuggestions) {
EXPECT_CALL( EXPECT_CALL(
*view(), *view(),
OnItemsAvailable( OnItemsAvailable(
GURL("https://example.com"), GURL(kExampleSite),
ElementsAre( ElementsAre(
MatchesItem(ASCIIToUTF16("Passwords"), ASCIIToUTF16("Passwords"), MatchesLabel(passwords_title_str(kExampleDomain)),
false, ItemType::LABEL),
MatchesItem(ASCIIToUTF16("Ben"), ASCIIToUTF16("Ben"), false, MatchesItem(ASCIIToUTF16("Ben"), ASCIIToUTF16("Ben"), false,
ItemType::SUGGESTION), ItemType::SUGGESTION),
MatchesItem(ASCIIToUTF16("S3cur3"), password_for_str("Ben"), true, MatchesItem(ASCIIToUTF16("S3cur3"), password_for_str("Ben"), true,
ItemType::SUGGESTION)))); ItemType::SUGGESTION))));
controller()->OnPasswordsAvailable({CreateEntry("Ben", "S3cur3").first}, controller()->OnPasswordsAvailable({CreateEntry("Ben", "S3cur3").first},
GURL("https://example.com")); GURL(kExampleSite));
} }
TEST_F(PasswordAccessoryControllerTest, HintsToEmptyUserNames) { TEST_F(PasswordAccessoryControllerTest, HintsToEmptyUserNames) {
EXPECT_CALL( EXPECT_CALL(*view(),
*view(), OnItemsAvailable(
OnItemsAvailable(GURL("https://example.com"), GURL(kExampleSite),
ElementsAre(MatchesItem(ASCIIToUTF16("Passwords"), ElementsAre(MatchesLabel(passwords_title_str(kExampleDomain)),
ASCIIToUTF16("Passwords"), false, MatchesItem(no_user_str(), no_user_str(), false,
ItemType::LABEL), ItemType::SUGGESTION),
MatchesItem(no_user_str(), no_user_str(), MatchesItem(ASCIIToUTF16("S3cur3"),
false, ItemType::SUGGESTION), password_for_str(no_user_str()), true,
MatchesItem(ASCIIToUTF16("S3cur3"), ItemType::SUGGESTION))));
password_for_str(no_user_str()),
true, ItemType::SUGGESTION))));
controller()->OnPasswordsAvailable({CreateEntry("", "S3cur3").first}, controller()->OnPasswordsAvailable({CreateEntry("", "S3cur3").first},
GURL("https://example.com")); GURL(kExampleSite));
} }
TEST_F(PasswordAccessoryControllerTest, SortsAlphabeticalDuringTransform) { TEST_F(PasswordAccessoryControllerTest, SortsAlphabeticalDuringTransform) {
EXPECT_CALL( EXPECT_CALL(
*view(), *view(),
OnItemsAvailable( OnItemsAvailable(
GURL("https://example.com"), GURL(kExampleSite),
ElementsAre( ElementsAre(
MatchesItem(passwords_title_str(), passwords_title_str(), false, MatchesLabel(passwords_title_str(kExampleDomain)),
ItemType::LABEL),
MatchesItem(ASCIIToUTF16("Alf"), ASCIIToUTF16("Alf"), false, MatchesItem(ASCIIToUTF16("Alf"), ASCIIToUTF16("Alf"), false,
ItemType::SUGGESTION), ItemType::SUGGESTION),
...@@ -226,20 +233,33 @@ TEST_F(PasswordAccessoryControllerTest, SortsAlphabeticalDuringTransform) { ...@@ -226,20 +233,33 @@ TEST_F(PasswordAccessoryControllerTest, SortsAlphabeticalDuringTransform) {
controller()->OnPasswordsAvailable( controller()->OnPasswordsAvailable(
{CreateEntry("Ben", "S3cur3").first, CreateEntry("Zebra", "M3h").first, {CreateEntry("Ben", "S3cur3").first, CreateEntry("Zebra", "M3h").first,
CreateEntry("Alf", "PWD").first, CreateEntry("Cat", "M1@u").first}, CreateEntry("Alf", "PWD").first, CreateEntry("Cat", "M1@u").first},
GURL("https://example.com")); GURL(kExampleSite));
}
TEST_F(PasswordAccessoryControllerTest, ClearsSuggestionsOnFrameNavigation) {
// Set any, non-empty password list.
EXPECT_CALL(*view(), OnItemsAvailable(GURL(kExampleSite), _));
controller()->OnPasswordsAvailable({CreateEntry("Ben", "S3cur3").first},
GURL(kExampleSite));
// Pretend that a navigation happened.
EXPECT_CALL(
*view(),
OnItemsAvailable(
GURL(kExampleSite),
ElementsAre(MatchesLabel(passwords_empty_str(kExampleDomain)))));
controller()->DidNavigateMainFrame();
} }
TEST_F(PasswordAccessoryControllerTest, ProvidesEmptySuggestionsMessage) { TEST_F(PasswordAccessoryControllerTest, ProvidesEmptySuggestionsMessage) {
EXPECT_CALL(*view(), OnItemsAvailable( EXPECT_CALL(
GURL("https://example.com"), *view(),
ElementsAre(MatchesItem(ASCIIToUTF16("Passwords"), OnItemsAvailable(
ASCIIToUTF16("Passwords"), GURL(kExampleSite),
false, ItemType::LABEL), ElementsAre(MatchesLabel(passwords_empty_str(kExampleDomain)))));
MatchesItem(passwords_empty_str(),
passwords_empty_str(), false, controller()->OnPasswordsAvailable({}, GURL(kExampleSite));
ItemType::LABEL))));
controller()->OnPasswordsAvailable({}, GURL("https://example.com"));
} }
TEST_F(PasswordAccessoryControllerTest, IgnoresCrossOriginCalls) { TEST_F(PasswordAccessoryControllerTest, IgnoresCrossOriginCalls) {
......
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