Commit 5d21adc8 authored by Javier Ernesto Flores Robles's avatar Javier Ernesto Flores Robles Committed by Commit Bot

[iOS][MF] Hide and show password icon

Hide password icon when no passwords are available and show it again
if passwords become available.

Updates SavePasswordsConsumer to post empty results and adds an early
return where this was expected to keep the same behavior.

Bug: 878388
Change-Id: Ie4948d3b68e1ce00fa34f37f9207d64fca174ceb
Reviewed-on: https://chromium-review.googlesource.com/c/1333427
Commit-Queue: Javier Ernesto Flores Robles <javierrobles@chromium.org>
Reviewed-by: default avatarMark Cogan <marq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608094}
parent dce02bb5
...@@ -15,14 +15,44 @@ ...@@ -15,14 +15,44 @@
#error "This file requires ARC support." #error "This file requires ARC support."
#endif #endif
@interface PasswordFetcher ()<SavePasswordsConsumerDelegate> { // Protocol to observe changes on the Password Store.
@protocol PasswordStoreObserver<NSObject>
// The logins in the Password Store changed.
- (void)loginsDidChange;
@end
namespace {
// Objective-C bridge to observe changes in the Password Store.
class PasswordStoreObserverBridge
: public password_manager::PasswordStore::Observer {
public:
explicit PasswordStoreObserverBridge(id<PasswordStoreObserver> observer)
: observer_(observer) {}
PasswordStoreObserverBridge() {}
private:
void OnLoginsChanged(
const password_manager::PasswordStoreChangeList& changes) override {
[observer_ loginsDidChange];
}
__weak id<PasswordStoreObserver> observer_ = nil;
};
} // namespace
@interface PasswordFetcher ()<SavePasswordsConsumerDelegate,
PasswordStoreObserver> {
// The interface for getting and manipulating a user's saved passwords. // The interface for getting and manipulating a user's saved passwords.
scoped_refptr<password_manager::PasswordStore> _passwordStore; scoped_refptr<password_manager::PasswordStore> _passwordStore;
// A helper object for passing data about saved passwords from a finished // A helper object for passing data about saved passwords from a finished
// password store request to the SavePasswordsCollectionViewController. // password store request to the SavePasswordsCollectionViewController.
std::unique_ptr<ios::SavePasswordsConsumer> _savedPasswordsConsumer; std::unique_ptr<ios::SavePasswordsConsumer> _savedPasswordsConsumer;
// The list of the user's saved passwords. // The object to observe changes in the Password Store.
std::vector<std::unique_ptr<autofill::PasswordForm>> _savedForms; std::unique_ptr<PasswordStoreObserverBridge> _passwordStoreObserver;
} }
// Delegate to send the fetchted passwords. // Delegate to send the fetchted passwords.
...@@ -48,23 +78,37 @@ ...@@ -48,23 +78,37 @@
_passwordStore = passwordStore; _passwordStore = passwordStore;
_savedPasswordsConsumer.reset(new ios::SavePasswordsConsumer(self)); _savedPasswordsConsumer.reset(new ios::SavePasswordsConsumer(self));
_passwordStore->GetAutofillableLogins(_savedPasswordsConsumer.get()); _passwordStore->GetAutofillableLogins(_savedPasswordsConsumer.get());
_passwordStoreObserver.reset(new PasswordStoreObserverBridge(self));
_passwordStore->AddObserver(_passwordStoreObserver.get());
} }
return self; return self;
} }
- (void)dealloc {
_passwordStore->RemoveObserver(_passwordStoreObserver.get());
}
#pragma mark - SavePasswordsConsumerDelegate #pragma mark - SavePasswordsConsumerDelegate
- (void)onGetPasswordStoreResults: - (void)onGetPasswordStoreResults:
(std::vector<std::unique_ptr<autofill::PasswordForm>>&)result { (std::vector<std::unique_ptr<autofill::PasswordForm>>&)result {
for (auto it = result.begin(); it != result.end(); ++it) { result.erase(
if (!(*it)->blacklisted_by_user) std::remove_if(result.begin(), result.end(),
_savedForms.push_back(std::move(*it)); [](std::unique_ptr<autofill::PasswordForm>& form) {
} return form->blacklisted_by_user;
}),
result.end());
password_manager::DuplicatesMap savedPasswordDuplicates; password_manager::DuplicatesMap savedPasswordDuplicates;
password_manager::SortEntriesAndHideDuplicates(&_savedForms, password_manager::SortEntriesAndHideDuplicates(&result,
&savedPasswordDuplicates); &savedPasswordDuplicates);
[self.delegate passwordFetcher:self didFetchPasswords:_savedForms]; [self.delegate passwordFetcher:self didFetchPasswords:result];
}
#pragma mark - PasswordStoreObserver
- (void)loginsDidChange {
_passwordStore->GetAutofillableLogins(_savedPasswordsConsumer.get());
} }
@end @end
...@@ -74,23 +74,25 @@ class PasswordFetcherTest : public PlatformTest { ...@@ -74,23 +74,25 @@ class PasswordFetcherTest : public PlatformTest {
.get(); .get();
} }
// Creates and adds a saved password form. autofill::PasswordForm Form1() {
void AddSavedForm1() { autofill::PasswordForm form;
auto form = std::make_unique<autofill::PasswordForm>(); form.origin = GURL("http://www.example.com/accounts/LoginAuth");
form->origin = GURL("http://www.example.com/accounts/LoginAuth"); form.action = GURL("http://www.example.com/accounts/Login");
form->action = GURL("http://www.example.com/accounts/Login"); form.username_element = base::ASCIIToUTF16("Email");
form->username_element = base::ASCIIToUTF16("Email"); form.username_value = base::ASCIIToUTF16("test@egmail.com");
form->username_value = base::ASCIIToUTF16("test@egmail.com"); form.password_element = base::ASCIIToUTF16("Passwd");
form->password_element = base::ASCIIToUTF16("Passwd"); form.password_value = base::ASCIIToUTF16("test");
form->password_value = base::ASCIIToUTF16("test"); form.submit_element = base::ASCIIToUTF16("signIn");
form->submit_element = base::ASCIIToUTF16("signIn"); form.signon_realm = "http://www.example.com/";
form->signon_realm = "http://www.example.com/"; form.preferred = false;
form->preferred = false; form.scheme = autofill::PasswordForm::SCHEME_HTML;
form->scheme = autofill::PasswordForm::SCHEME_HTML; form.blacklisted_by_user = false;
form->blacklisted_by_user = false; return form;
GetPasswordStore()->AddLogin(*std::move(form));
} }
// Creates and adds a saved password form.
void AddSavedForm1() { GetPasswordStore()->AddLogin(Form1()); }
// Creates and adds a saved password form. // Creates and adds a saved password form.
void AddSavedForm2() { void AddSavedForm2() {
auto form = std::make_unique<autofill::PasswordForm>(); auto form = std::make_unique<autofill::PasswordForm>();
...@@ -227,4 +229,32 @@ TEST_F(PasswordFetcherTest, IgnoresDuplicated) { ...@@ -227,4 +229,32 @@ TEST_F(PasswordFetcherTest, IgnoresDuplicated) {
EXPECT_TRUE(passwordFetcher); EXPECT_TRUE(passwordFetcher);
} }
// Tests PasswordFetcher receives 0 passwords.
TEST_F(PasswordFetcherTest, ReceivesZeroPasswords) {
AddSavedForm1();
TestPasswordFetcherDelegate* passwordFetcherDelegate =
[[TestPasswordFetcherDelegate alloc] init];
auto passwordStore = IOSChromePasswordStoreFactory::GetForBrowserState(
chrome_browser_state_.get(), ServiceAccessType::EXPLICIT_ACCESS);
PasswordFetcher* passwordFetcher =
[[PasswordFetcher alloc] initWithPasswordStore:passwordStore
delegate:passwordFetcherDelegate];
WaitUntilCondition(
^bool {
return passwordFetcherDelegate.passwordNumber > 0;
},
true, base::TimeDelta::FromSeconds(1000));
EXPECT_EQ(passwordFetcherDelegate.passwordNumber, 1u);
GetPasswordStore()->RemoveLogin(Form1());
WaitUntilCondition(
^bool {
return passwordFetcherDelegate.passwordNumber == 0;
},
true, base::TimeDelta::FromSeconds(1000));
EXPECT_EQ(passwordFetcherDelegate.passwordNumber, 0u);
EXPECT_TRUE(passwordFetcher);
}
} // namespace } // namespace
...@@ -18,8 +18,7 @@ SavePasswordsConsumer::~SavePasswordsConsumer() = default; ...@@ -18,8 +18,7 @@ SavePasswordsConsumer::~SavePasswordsConsumer() = default;
void SavePasswordsConsumer::OnGetPasswordStoreResults( void SavePasswordsConsumer::OnGetPasswordStoreResults(
std::vector<std::unique_ptr<autofill::PasswordForm>> results) { std::vector<std::unique_ptr<autofill::PasswordForm>> results) {
if (!results.empty()) [delegate_ onGetPasswordStoreResults:results];
[delegate_ onGetPasswordStoreResults:results];
} }
} // namespace ios } // namespace ios
...@@ -33,9 +33,11 @@ source_set("autofill") { ...@@ -33,9 +33,11 @@ source_set("autofill") {
"//ios/chrome/browser", "//ios/chrome/browser",
"//ios/chrome/browser/autofill", "//ios/chrome/browser/autofill",
"//ios/chrome/browser/autofill:autofill_shared", "//ios/chrome/browser/autofill:autofill_shared",
"//ios/chrome/browser/autofill/manual_fill",
"//ios/chrome/browser/browser_state", "//ios/chrome/browser/browser_state",
"//ios/chrome/browser/infobars", "//ios/chrome/browser/infobars",
"//ios/chrome/browser/metrics", "//ios/chrome/browser/metrics",
"//ios/chrome/browser/passwords",
"//ios/chrome/browser/passwords:passwords_generation_utils", "//ios/chrome/browser/passwords:passwords_generation_utils",
"//ios/chrome/browser/signin", "//ios/chrome/browser/signin",
"//ios/chrome/browser/ssl", "//ios/chrome/browser/ssl",
......
...@@ -7,7 +7,11 @@ ...@@ -7,7 +7,11 @@
#include "base/mac/foundation_util.h" #include "base/mac/foundation_util.h"
#include "components/autofill/core/common/autofill_features.h" #include "components/autofill/core/common/autofill_features.h"
#import "components/autofill/ios/browser/js_suggestion_manager.h" #import "components/autofill/ios/browser/js_suggestion_manager.h"
#include "components/keyed_service/core/service_access_type.h"
#include "components/password_manager/core/browser/password_store.h"
#import "ios/chrome/browser/autofill/form_input_accessory_view_controller.h" #import "ios/chrome/browser/autofill/form_input_accessory_view_controller.h"
#import "ios/chrome/browser/autofill/manual_fill/passwords_fetcher.h"
#include "ios/chrome/browser/passwords/ios_chrome_password_store_factory.h"
#import "ios/chrome/browser/ui/autofill/form_input_accessory_mediator.h" #import "ios/chrome/browser/ui/autofill/form_input_accessory_mediator.h"
#import "ios/chrome/browser/ui/autofill/manual_fill/address_coordinator.h" #import "ios/chrome/browser/ui/autofill/manual_fill/address_coordinator.h"
#import "ios/chrome/browser/ui/autofill/manual_fill/card_coordinator.h" #import "ios/chrome/browser/ui/autofill/manual_fill/card_coordinator.h"
...@@ -24,7 +28,8 @@ ...@@ -24,7 +28,8 @@
ManualFillAccessoryViewControllerDelegate, ManualFillAccessoryViewControllerDelegate,
AddressCoordinatorDelegate, AddressCoordinatorDelegate,
CardCoordinatorDelegate, CardCoordinatorDelegate,
PasswordCoordinatorDelegate> PasswordCoordinatorDelegate,
PasswordFetcherDelegate>
// The Mediator for the input accessory view controller. // The Mediator for the input accessory view controller.
@property(nonatomic, strong) @property(nonatomic, strong)
...@@ -43,6 +48,9 @@ ...@@ -43,6 +48,9 @@
@property(nonatomic, strong) @property(nonatomic, strong)
ManualFillInjectionHandler* manualFillInjectionHandler; ManualFillInjectionHandler* manualFillInjectionHandler;
// The password fetcher used to inform if passwords are available.
@property(nonatomic, strong) PasswordFetcher* passwordFetcher;
// The WebStateList for this instance. Used to instantiate the child // The WebStateList for this instance. Used to instantiate the child
// coordinators lazily. // coordinators lazily.
@property(nonatomic, assign) WebStateList* webStateList; @property(nonatomic, assign) WebStateList* webStateList;
...@@ -78,6 +86,17 @@ ...@@ -78,6 +86,17 @@
_formInputAccessoryMediator = [[FormInputAccessoryMediator alloc] _formInputAccessoryMediator = [[FormInputAccessoryMediator alloc]
initWithConsumer:self.formInputAccessoryViewController initWithConsumer:self.formInputAccessoryViewController
webStateList:webStateList]; webStateList:webStateList];
auto passwordStore = IOSChromePasswordStoreFactory::GetForBrowserState(
browserState, ServiceAccessType::EXPLICIT_ACCESS);
// In BVC unit tests the password store doesn't exist. Skip creating the
// fetcher.
// TODO:(crbug.com/878388) Remove this workaround.
if (passwordStore) {
_passwordFetcher =
[[PasswordFetcher alloc] initWithPasswordStore:passwordStore
delegate:self];
}
} }
return self; return self;
} }
...@@ -195,4 +214,13 @@ ...@@ -195,4 +214,13 @@
[self.delegate openAddressSettings]; [self.delegate openAddressSettings];
} }
#pragma mark - PasswordFetcherDelegate
- (void)passwordFetcher:(PasswordFetcher*)passwordFetcher
didFetchPasswords:
(std::vector<std::unique_ptr<autofill::PasswordForm>>&)passwords {
self.manualFillAccessoryViewController.passwordButtonHidden =
passwords.empty();
}
@end @end
...@@ -41,6 +41,10 @@ extern NSString* const AccessoryCreditCardAccessibilityIdentifier; ...@@ -41,6 +41,10 @@ extern NSString* const AccessoryCreditCardAccessibilityIdentifier;
// shown above the keyboard on iPhone and above the manual fill view. // shown above the keyboard on iPhone and above the manual fill view.
@interface ManualFillAccessoryViewController : UIViewController @interface ManualFillAccessoryViewController : UIViewController
// Changing this property hides and shows the password button.
@property(nonatomic, assign, getter=isPasswordButtonHidden)
BOOL passwordButtonHidden;
// Instances an object with the desired delegate. // Instances an object with the desired delegate.
// //
// @param delegate The delegate for this object. // @param delegate The delegate for this object.
......
...@@ -41,24 +41,28 @@ static NSTimeInterval MFAnimationDuration = 0.20; ...@@ -41,24 +41,28 @@ static NSTimeInterval MFAnimationDuration = 0.20;
@interface ManualFillAccessoryViewController () @interface ManualFillAccessoryViewController ()
// Delegate to handle interactions.
@property(nonatomic, readonly, weak) @property(nonatomic, readonly, weak)
id<ManualFillAccessoryViewControllerDelegate> id<ManualFillAccessoryViewControllerDelegate>
delegate; delegate;
// The button to close manual fallback.
@property(nonatomic, strong) UIButton* keyboardButton; @property(nonatomic, strong) UIButton* keyboardButton;
// The button to open the passwords section.
@property(nonatomic, strong) UIButton* passwordButton; @property(nonatomic, strong) UIButton* passwordButton;
// The button to open the credit cards section.
@property(nonatomic, strong) UIButton* cardsButton; @property(nonatomic, strong) UIButton* cardsButton;
// The button to open the profiles section.
@property(nonatomic, strong) UIButton* accountButton; @property(nonatomic, strong) UIButton* accountButton;
@end @end
@implementation ManualFillAccessoryViewController @implementation ManualFillAccessoryViewController
@synthesize delegate = _delegate; #pragma mark - Public
@synthesize keyboardButton = _keyboardButton;
@synthesize passwordButton = _passwordButton;
@synthesize cardsButton = _cardsButton;
@synthesize accountButton = _accountButton;
- (instancetype)initWithDelegate: - (instancetype)initWithDelegate:
(id<ManualFillAccessoryViewControllerDelegate>)delegate { (id<ManualFillAccessoryViewControllerDelegate>)delegate {
...@@ -69,6 +73,24 @@ static NSTimeInterval MFAnimationDuration = 0.20; ...@@ -69,6 +73,24 @@ static NSTimeInterval MFAnimationDuration = 0.20;
return self; return self;
} }
- (void)reset {
[self resetTintColors];
self.keyboardButton.hidden = YES;
self.keyboardButton.alpha = 0.0;
}
#pragma mark - Setters
- (void)setPasswordButtonHidden:(BOOL)passwordButtonHidden {
if (passwordButtonHidden == _passwordButtonHidden) {
return;
}
_passwordButton.hidden = passwordButtonHidden;
_passwordButtonHidden = passwordButtonHidden;
}
#pragma mark - Private
- (void)loadView { - (void)loadView {
self.view = [[UIView alloc] init]; self.view = [[UIView alloc] init];
self.view.translatesAutoresizingMaskIntoConstraints = NO; self.view.translatesAutoresizingMaskIntoConstraints = NO;
...@@ -100,6 +122,7 @@ static NSTimeInterval MFAnimationDuration = 0.20; ...@@ -100,6 +122,7 @@ static NSTimeInterval MFAnimationDuration = 0.20;
forControlEvents:UIControlEventTouchUpInside]; forControlEvents:UIControlEventTouchUpInside];
self.passwordButton.accessibilityIdentifier = self.passwordButton.accessibilityIdentifier =
manual_fill::AccessoryPasswordAccessibilityIdentifier; manual_fill::AccessoryPasswordAccessibilityIdentifier;
self.passwordButton.hidden = self.isPasswordButtonHidden;
[icons addObject:self.passwordButton]; [icons addObject:self.passwordButton];
if (autofill::features::IsAutofillManualFallbackEnabled()) { if (autofill::features::IsAutofillManualFallbackEnabled()) {
...@@ -151,12 +174,6 @@ static NSTimeInterval MFAnimationDuration = 0.20; ...@@ -151,12 +174,6 @@ static NSTimeInterval MFAnimationDuration = 0.20;
self.keyboardButton.alpha = 0.0; self.keyboardButton.alpha = 0.0;
} }
- (void)reset {
[self resetTintColors];
self.keyboardButton.hidden = YES;
self.keyboardButton.alpha = 0.0;
}
// Resets the colors of all the icons to the active color. // Resets the colors of all the icons to the active color.
- (void)resetTintColors { - (void)resetTintColors {
UIColor* activeTintColor = [self activeTintColor]; UIColor* activeTintColor = [self activeTintColor];
......
...@@ -249,6 +249,7 @@ BOOL WaitForJavaScriptCondition(NSString* java_script_condition) { ...@@ -249,6 +249,7 @@ BOOL WaitForJavaScriptCondition(NSString* java_script_condition) {
const GURL URL = self.testServer->GetURL(kFormHTMLFile); const GURL URL = self.testServer->GetURL(kFormHTMLFile);
[ChromeEarlGrey loadURL:URL]; [ChromeEarlGrey loadURL:URL];
[ChromeEarlGrey waitForWebViewContainingText:"hello!"]; [ChromeEarlGrey waitForWebViewContainingText:"hello!"];
SaveExamplePasswordForm();
} }
- (void)tearDown { - (void)tearDown {
...@@ -645,4 +646,31 @@ BOOL WaitForJavaScriptCondition(NSString* java_script_condition) { ...@@ -645,4 +646,31 @@ BOOL WaitForJavaScriptCondition(NSString* java_script_condition) {
XCTAssertTrue(WaitForJavaScriptCondition(javaScriptCondition)); XCTAssertTrue(WaitForJavaScriptCondition(javaScriptCondition));
} }
// Tests that the password icon is hidden when no passwords are available.
- (void)testPasswordIconIsNotVisibleWhenPasswordStoreEmpty {
ClearPasswordStore();
// Bring up the keyboard.
[[EarlGrey selectElementWithMatcher:chrome_test_util::WebViewMatcher()]
performAction:chrome_test_util::TapWebElement(kFormElementUsername)];
// Wait for the keyboard to appear.
[GREYKeyboard waitForKeyboardToAppear];
// Assert the password icon is not visible.
[[EarlGrey selectElementWithMatcher:PasswordIconMatcher()]
assertWithMatcher:grey_notVisible()];
// Store one password.
SaveExamplePasswordForm();
// Tap another field to trigger form activity.
[[EarlGrey selectElementWithMatcher:chrome_test_util::WebViewMatcher()]
performAction:chrome_test_util::TapWebElement(kFormElementPassword)];
// Assert the password icon is visible now.
[[EarlGrey selectElementWithMatcher:PasswordIconMatcher()]
assertWithMatcher:grey_sufficientlyVisible()];
}
@end @end
...@@ -508,6 +508,9 @@ blacklistedFormItemWithText:(NSString*)text ...@@ -508,6 +508,9 @@ blacklistedFormItemWithText:(NSString*)text
- (void)onGetPasswordStoreResults: - (void)onGetPasswordStoreResults:
(std::vector<std::unique_ptr<autofill::PasswordForm>>&)result { (std::vector<std::unique_ptr<autofill::PasswordForm>>&)result {
if (result.empty()) {
return;
}
for (auto it = result.begin(); it != result.end(); ++it) { for (auto it = result.begin(); it != result.end(); ++it) {
// PasswordForm is needed when user wants to delete the site/password. // PasswordForm is needed when user wants to delete the site/password.
auto form = std::make_unique<autofill::PasswordForm>(**it); auto form = std::make_unique<autofill::PasswordForm>(**it);
......
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