[iOS][MF] Do the password filtering in the SQL layer

Moves the filtering responsability to the password store. This causes a
faster and smaller fetch when not all the credentials are needed.
Updates the API of the mediator and the fetcher so only the required
args are passed.
Renames |disableFilter| to something that reflects the actual intent of
that variable.
Updates local test to include the complete spec of the saved password,
so the query works when it is not a domain.

Bug: 932089
Change-Id: Ibcf64953db4e314f4111289e1fff4a248fd8679b
Reviewed-on: https://chromium-review.googlesource.com/c/1491652Reviewed-by: Olivier Robin <olivierrobin@chromium.org>
Commit-Queue: Javier Ernesto Flores Robles <javierrobles@chromium.org>
Cr-Commit-Position: refs/heads/master@{#636829}

ios/chrome/browser/autofill/manual_fill/passwords_fetcher.h

@@ -21,6 +21,8 @@ namespace password_manager {
 class PasswordStore;
 }  // namespace password_manager
 
+class GURL;
+
 // Protocol to receive the passwords fetched asynchronously.
 @protocol PasswordFetcherDelegate
 
@@ -33,12 +35,14 @@ class PasswordStore;
 
 @interface PasswordFetcher : NSObject
 
-// The designated initializer. |browserState| must not be nil.
-- (instancetype)initWithPasswordStore:
-                    (scoped_refptr<password_manager::PasswordStore>)
-                        passwordStore
-                             delegate:(id<PasswordFetcherDelegate>)delegate
-    NS_DESIGNATED_INITIALIZER;
+// The designated initializer. |passwordStore| must not be nil. The passwords
+// will be filtered by the passed |origin|, pass an empty GURL to avoid
+// filtering.
+- (instancetype)
+    initWithPasswordStore:
+        (scoped_refptr<password_manager::PasswordStore>)passwordStore
+                 delegate:(id<PasswordFetcherDelegate>)delegate
+                   origin:(const GURL&)origin NS_DESIGNATED_INITIALIZER;
 
 - (instancetype)init NS_UNAVAILABLE;
 

ios/chrome/browser/autofill/manual_fill/passwords_fetcher.mm

@@ -70,7 +70,8 @@ class PasswordStoreObserverBridge
 - (instancetype)initWithPasswordStore:
                     (scoped_refptr<password_manager::PasswordStore>)
                         passwordStore
-                             delegate:(id<PasswordFetcherDelegate>)delegate {
+                             delegate:(id<PasswordFetcherDelegate>)delegate
+                               origin:(const GURL&)origin {
   DCHECK(passwordStore);
   DCHECK(delegate);
   self = [super init];
@@ -78,9 +79,17 @@ class PasswordStoreObserverBridge
     _delegate = delegate;
     _passwordStore = passwordStore;
     _savedPasswordsConsumer.reset(new ios::SavePasswordsConsumer(self));
-    _passwordStore->GetAutofillableLogins(_savedPasswordsConsumer.get());
     _passwordStoreObserver.reset(new PasswordStoreObserverBridge(self));
     _passwordStore->AddObserver(_passwordStoreObserver.get());
+
+    if (origin.is_empty()) {
+      _passwordStore->GetAutofillableLogins(_savedPasswordsConsumer.get());
+    } else {
+      password_manager::PasswordStore::FormDigest digest = {
+          autofill::PasswordForm::SCHEME_HTML, std::string(), origin};
+      digest.signon_realm = origin.spec();
+      _passwordStore->GetLogins(digest, _savedPasswordsConsumer.get());
+    }
   }
   return self;
 }

ios/chrome/browser/autofill/manual_fill/passwords_fetcher_unittest.mm

@@ -141,7 +141,8 @@ TEST_F(PasswordFetcherTest, Initialization) {
       chrome_browser_state_.get(), ServiceAccessType::EXPLICIT_ACCESS);
   PasswordFetcher* passwordFetcher =
       [[PasswordFetcher alloc] initWithPasswordStore:passwordStore
-                                            delegate:passwordFetcherDelegate];
+                                            delegate:passwordFetcherDelegate
+                                              origin:GURL::EmptyGURL()];
   EXPECT_TRUE(passwordFetcher);
 }
 
@@ -154,7 +155,9 @@ TEST_F(PasswordFetcherTest, ReturnsPassword) {
       chrome_browser_state_.get(), ServiceAccessType::EXPLICIT_ACCESS);
   PasswordFetcher* passwordFetcher =
       [[PasswordFetcher alloc] initWithPasswordStore:passwordStore
-                                            delegate:passwordFetcherDelegate];
+                                            delegate:passwordFetcherDelegate
+                                              origin:GURL::EmptyGURL()];
+
   WaitUntilCondition(
       ^bool {
         return passwordFetcherDelegate.passwordNumber > 0;
@@ -175,7 +178,8 @@ TEST_F(PasswordFetcherTest, ReturnsTwoPasswords) {
       chrome_browser_state_.get(), ServiceAccessType::EXPLICIT_ACCESS);
   PasswordFetcher* passwordFetcher =
       [[PasswordFetcher alloc] initWithPasswordStore:passwordStore
-                                            delegate:passwordFetcherDelegate];
+                                            delegate:passwordFetcherDelegate
+                                              origin:GURL::EmptyGURL()];
   WaitUntilCondition(
       ^bool {
         return passwordFetcherDelegate.passwordNumber > 0;
@@ -196,7 +200,8 @@ TEST_F(PasswordFetcherTest, IgnoresBlacklisted) {
       chrome_browser_state_.get(), ServiceAccessType::EXPLICIT_ACCESS);
   PasswordFetcher* passwordFetcher =
       [[PasswordFetcher alloc] initWithPasswordStore:passwordStore
-                                            delegate:passwordFetcherDelegate];
+                                            delegate:passwordFetcherDelegate
+                                              origin:GURL::EmptyGURL()];
   WaitUntilCondition(
       ^bool {
         return passwordFetcherDelegate.passwordNumber > 0;
@@ -219,7 +224,8 @@ TEST_F(PasswordFetcherTest, IgnoresDuplicated) {
       chrome_browser_state_.get(), ServiceAccessType::EXPLICIT_ACCESS);
   PasswordFetcher* passwordFetcher =
       [[PasswordFetcher alloc] initWithPasswordStore:passwordStore
-                                            delegate:passwordFetcherDelegate];
+                                            delegate:passwordFetcherDelegate
+                                              origin:GURL::EmptyGURL()];
   WaitUntilCondition(
       ^bool {
         return passwordFetcherDelegate.passwordNumber > 0;
@@ -239,7 +245,8 @@ TEST_F(PasswordFetcherTest, ReceivesZeroPasswords) {
       chrome_browser_state_.get(), ServiceAccessType::EXPLICIT_ACCESS);
   PasswordFetcher* passwordFetcher =
       [[PasswordFetcher alloc] initWithPasswordStore:passwordStore
-                                            delegate:passwordFetcherDelegate];
+                                            delegate:passwordFetcherDelegate
+                                              origin:GURL::EmptyGURL()];
   WaitUntilCondition(
       ^bool {
         return passwordFetcherDelegate.passwordNumber > 0;
@@ -258,4 +265,26 @@ TEST_F(PasswordFetcherTest, ReceivesZeroPasswords) {
   EXPECT_TRUE(passwordFetcher);
 }
 
+// Tests PasswordFetcher filters 1 passwords.
+TEST_F(PasswordFetcherTest, FilterPassword) {
+  AddSavedForm1();
+  AddSavedForm2();
+  TestPasswordFetcherDelegate* passwordFetcherDelegate =
+      [[TestPasswordFetcherDelegate alloc] init];
+  auto passwordStore = IOSChromePasswordStoreFactory::GetForBrowserState(
+      chrome_browser_state_.get(), ServiceAccessType::EXPLICIT_ACCESS);
+  PasswordFetcher* passwordFetcher = [[PasswordFetcher alloc]
+      initWithPasswordStore:passwordStore
+                   delegate:passwordFetcherDelegate
+                     origin:GURL("http://www.secret.com/")];
+  WaitUntilCondition(
+      ^bool {
+        return passwordFetcherDelegate.passwordNumber > 0;
+      },
+      true, base::TimeDelta::FromSeconds(1000));
+
+  EXPECT_EQ(passwordFetcherDelegate.passwordNumber, 2u);
+  EXPECT_TRUE(passwordFetcher);
+}
+
 }  // namespace

ios/chrome/browser/ui/autofill/form_input_accessory_mediator.mm

@@ -178,7 +178,8 @@
   if (passwordStore) {
     _passwordFetcher =
         [[PasswordFetcher alloc] initWithPasswordStore:passwordStore
-                                              delegate:self];
+                                              delegate:self
+                                                origin:GURL::EmptyGURL()];
   }
   if (personalDataManager) {
     _personalDataManager = personalDataManager;

ios/chrome/browser/ui/autofill/manual_fill/password_coordinator.mm

@@ -15,6 +15,8 @@
 #import "ios/chrome/browser/ui/table_view/table_view_navigation_controller.h"
 #import "ios/chrome/browser/ui/table_view/table_view_presentation_controller.h"
 #include "ios/chrome/browser/ui/util/ui_util.h"
+#import "ios/chrome/browser/web_state_list/web_state_list.h"
+#import "ios/web/public/web_state/web_state.h"
 
 #if !defined(__has_feature) || !__has_feature(objc_arc)
 #error "This file requires ARC support."
@@ -46,6 +48,9 @@ NSString* const PasswordDoneButtonAccessibilityIdentifier =
 // dismissing any presented view controller. iPad only.
 @property(nonatomic, weak) UIButton* presentingButton;
 
+// The origin for this mediator.
+@property(nonatomic) GURL origin;
+
 @end
 
 @implementation ManualFillPasswordCoordinator
@@ -70,9 +75,14 @@ initWithBaseViewController:(UIViewController*)viewController
 
     auto passwordStore = IOSChromePasswordStoreFactory::GetForBrowserState(
         browserState, ServiceAccessType::EXPLICIT_ACCESS);
-    _passwordMediator =
-        [[ManualFillPasswordMediator alloc] initWithWebStateList:webStateList
-                                                   passwordStore:passwordStore];
+    _passwordMediator = [[ManualFillPasswordMediator alloc]
+        initWithPasswordStore:passwordStore];
+    // TODO(crbug.com/936413): This class doesn't need the WebState anymore,
+    // remove it from the init and only pass the origin.
+    DCHECK(webStateList->GetActiveWebState());
+    _origin = webStateList->GetActiveWebState()->GetLastCommittedURL();
+    [_passwordMediator fetchPasswordsForOrigin:_origin];
+    _passwordMediator.actionSectionEnabled = YES;
     _passwordMediator.consumer = _passwordViewController;
     _passwordMediator.navigationDelegate = self;
     _passwordMediator.contentDelegate = injectionHandler;
@@ -159,7 +169,8 @@ animationControllerForDismissedController:(UIViewController*)dismissed {
 
   PasswordViewController* allPasswordsViewController = [
       [PasswordViewController alloc] initWithSearchController:searchController];
-  self.passwordMediator.disableFilter = YES;
+  self.passwordMediator.actionSectionEnabled = NO;
+  [self.passwordMediator fetchPasswordsForOrigin:GURL::EmptyGURL()];
   self.passwordMediator.consumer = allPasswordsViewController;
   UIBarButtonItem* doneButton = [[UIBarButtonItem alloc]
       initWithBarButtonSystemItem:UIBarButtonSystemItemDone
@@ -187,7 +198,9 @@ animationControllerForDismissedController:(UIViewController*)dismissed {
   [self.allPasswordsViewController.presentingViewController
       dismissViewControllerAnimated:YES
                          completion:^{
-                           weakSelf.passwordMediator.disableFilter = NO;
+                           weakSelf.passwordMediator.actionSectionEnabled = YES;
+                           [weakSelf.passwordMediator
+                               fetchPasswordsForOrigin:self.origin];
                            weakSelf.passwordMediator.consumer =
                                weakSelf.passwordViewController;
                            if (weakSelf.presentingButton) {

ios/chrome/browser/ui/autofill/manual_fill/password_mediator.h

@@ -17,6 +17,8 @@ namespace password_manager {
 class PasswordStore;
 }  // namespace password_manager
 
+class GURL;
+
 class WebStateList;
 
 namespace manual_fill {
@@ -38,22 +40,24 @@ extern NSString* const SuggestPasswordAccessibilityIdentifier;
 @property(nonatomic, weak) id<ManualFillContentDelegate> contentDelegate;
 // The delegate in charge of navigation.
 @property(nonatomic, weak) id<PasswordListDelegate> navigationDelegate;
-// If YES disables filtering the fetched passwords with the active web state
-// url. Also activates an "All passwords" action if NO. Set this value before
+// If YES  actions will be post to the consumer. Set this value before
 // setting the consumer, since just setting this won't trigger the consumer
-// methods.
-@property(nonatomic, assign) BOOL disableFilter;
+// callbacks. Defaults to NO.
+@property(nonatomic, assign, getter=isActionSectionEnabled)
+    BOOL actionSectionEnabled;
 
-// The designated initializer. |passwordStore| and |webStateList| must not be
-// nil.
-- (instancetype)initWithWebStateList:(WebStateList*)webStateList
-                       passwordStore:
-                           (scoped_refptr<password_manager::PasswordStore>)
-                               passwordStore NS_DESIGNATED_INITIALIZER;
+// The designated initializer. |passwordStore| must not be nil.
+- (instancetype)initWithPasswordStore:
+    (scoped_refptr<password_manager::PasswordStore>)passwordStore
+    NS_DESIGNATED_INITIALIZER;
 
 // Unavailable. Use |initWithWebStateList:passwordStore:|.
 - (instancetype)init NS_UNAVAILABLE;
 
+// Fetches passwords using |origin| as the filter. If origin is empty (invalid)
+// it will fetch all the passwords.
+- (void)fetchPasswordsForOrigin:(const GURL&)origin;
+
 @end
 
 #endif  // IOS_CHROME_BROWSER_UI_AUTOFILL_MANUAL_FILL_PASSWORD_MEDIATOR_H_

ios/chrome/browser/ui/autofill/manual_fill/password_mediator.mm

@@ -20,10 +20,7 @@
 #import "ios/chrome/browser/ui/autofill/manual_fill/password_list_delegate.h"
 #import "ios/chrome/browser/ui/list_model/list_model.h"
 #import "ios/chrome/browser/ui/table_view/table_view_model.h"
-#import "ios/chrome/browser/web_state_list/web_state_list.h"
 #include "ios/chrome/grit/ios_strings.h"
-#import "ios/web/public/web_state/web_state.h"
-#include "net/base/registry_controlled_domains/registry_controlled_domain.h"
 #include "ui/base/l10n/l10n_util_mac.h"
 #include "url/gurl.h"
 
@@ -55,8 +52,11 @@ BOOL AreCredentialsAtIndexesConnected(
       isEqualToString:credentials[secondIndex].host];
 }
 
-@interface ManualFillPasswordMediator ()<ManualFillContentDelegate,
-                                         PasswordFetcherDelegate>
+@interface ManualFillPasswordMediator () <ManualFillContentDelegate,
+                                          PasswordFetcherDelegate> {
+  // The interface for getting and manipulating a user's saved passwords.
+  scoped_refptr<password_manager::PasswordStore> _passwordStore;
+}
 
 // The |WebStateList| containing the active web state. Used to filter the list
 // of credentials based on the active web state.
@@ -69,10 +69,6 @@ BOOL AreCredentialsAtIndexesConnected(
 // reuse the mediator.
 @property(nonatomic, strong) NSArray<ManualFillCredential*>* credentials;
 
-// If the filter is disabled, the "Show All Passwords" button is not included
-// in the model.
-@property(nonatomic, assign, readonly) BOOL isAllPasswordButtonEnabled;
-
 // YES if the password fetcher has completed at least one fetch.
 @property(nonatomic, assign) BOOL passwordFetcherDidFetch;
 
@@ -80,21 +76,24 @@ BOOL AreCredentialsAtIndexesConnected(
 
 @implementation ManualFillPasswordMediator
 
-- (instancetype)initWithWebStateList:(WebStateList*)webStateList
-                       passwordStore:
-                           (scoped_refptr<password_manager::PasswordStore>)
-                               passwordStore {
+- (instancetype)initWithPasswordStore:
+    (scoped_refptr<password_manager::PasswordStore>)passwordStore {
   self = [super init];
   if (self) {
     _credentials = @[];
-    _webStateList = webStateList;
-    _passwordFetcher =
-        [[PasswordFetcher alloc] initWithPasswordStore:passwordStore
-                                              delegate:self];
+    _passwordStore = passwordStore;
   }
   return self;
 }
 
+- (void)fetchPasswordsForOrigin:(const GURL&)origin {
+  self.credentials = @[];
+  self.passwordFetcher =
+      [[PasswordFetcher alloc] initWithPasswordStore:_passwordStore
+                                            delegate:self
+                                              origin:origin];
+}
+
 #pragma mark - PasswordFetcherDelegate
 
 - (void)passwordFetcher:(PasswordFetcher*)passwordFetcher
@@ -152,31 +151,7 @@ BOOL AreCredentialsAtIndexesConnected(
   if (!self.consumer) {
     return;
   }
-  if (self.disableFilter) {
-    auto credentials = [self createItemsForCredentials:self.credentials];
-    [self.consumer presentCredentials:credentials];
-    return;
-  }
-  web::WebState* currentWebState = self.webStateList->GetActiveWebState();
-  if (!currentWebState) {
-    return;
-  }
-  GURL visibleURL = currentWebState->GetVisibleURL();
-  std::string site_name =
-      net::registry_controlled_domains::GetDomainAndRegistry(
-          visibleURL.host(),
-          net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES);
-  // Sometimes the site_name can be empty. i.e. if the host is an IP address.
-  if (site_name.empty()) {
-    site_name = visibleURL.host();
-  }
-  NSString* siteName = base::SysUTF8ToNSString(site_name);
-
-  NSPredicate* predicate =
-      [NSPredicate predicateWithFormat:@"siteName = %@", siteName];
-  NSArray* filteredCredentials =
-      [self.credentials filteredArrayUsingPredicate:predicate];
-  auto credentials = [self createItemsForCredentials:filteredCredentials];
+  auto credentials = [self createItemsForCredentials:self.credentials];
   [self.consumer presentCredentials:credentials];
 }
 
@@ -205,7 +180,7 @@ BOOL AreCredentialsAtIndexesConnected(
   if (!self.consumer) {
     return;
   }
-  if (self.isAllPasswordButtonEnabled) {
+  if (self.isActionSectionEnabled) {
     NSMutableArray<ManualFillActionItem*>* actions =
         [[NSMutableArray alloc] init];
     __weak __typeof(self) weakSelf = self;
@@ -262,12 +237,6 @@ BOOL AreCredentialsAtIndexesConnected(
   }
 }
 
-#pragma mark - Getters
-
-- (BOOL)isAllPasswordButtonEnabled {
-  return !self.disableFilter;
-}
-
 #pragma mark - Setters
 
 - (void)setConsumer:(id<ManualFillPasswordConsumer>)consumer {

ios/chrome/browser/ui/autofill/manual_fill/password_view_controller_egtest.mm

@@ -200,14 +200,14 @@ void SaveExamplePasswordForm() {
   SaveToPasswordStore(example);
 }
 
-// Saves an example form in the store.
-void SaveLocalPasswordForm() {
-  autofill::PasswordForm example;
-  example.username_value = base::ASCIIToUTF16(kExampleUsername);
-  example.password_value = base::ASCIIToUTF16(kExamplePassword);
-  example.origin = GURL("http://127.0.0.1:55264");
-  example.signon_realm = example.origin.spec();
-  SaveToPasswordStore(example);
+// Saves an example form in the storefor the passed URL.
+void SaveLocalPasswordForm(const GURL& url) {
+  autofill::PasswordForm localForm;
+  localForm.username_value = base::ASCIIToUTF16(kExampleUsername);
+  localForm.password_value = base::ASCIIToUTF16(kExamplePassword);
+  localForm.origin = url;
+  localForm.signon_realm = localForm.origin.spec();
+  SaveToPasswordStore(localForm);
 }
 
 // Removes all credentials stored.
@@ -585,8 +585,7 @@ BOOL WaitForJavaScriptCondition(NSString* java_script_condition) {
   const GURL URL = self.testServer->GetURL(kIFrameHTMLFile);
   [ChromeEarlGrey loadURL:URL];
   [ChromeEarlGrey waitForWebViewContainingText:"iFrame"];
-
-  SaveLocalPasswordForm();
+  SaveLocalPasswordForm(URL);
 
   // Bring up the keyboard.
   [[EarlGrey selectElementWithMatcher:chrome_test_util::WebViewMatcher()]