[Merge 115] Update form managers when credential gets updated or deleted

This CL is to fix the issue if the user is already on a page to sign-in and they go to the Password Manager to update their credential. The modified credential is now updated on the password suggestions list. (Deleted credentials are also removed from the list). (cherry picked from commit b45d8ee) Bug: 1446310 Change-Id: Iebe68654d501010edc09654a4635668fe9ac636a Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4543753 Reviewed-by: Tommy Martino <tmartino@chromium.org> Commit-Queue: Veronique Nguyen <veronguyen@google.com> Cr-Original-Commit-Position: refs/heads/main@{#1148661} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4581761 Cr-Commit-Position: refs/branch-heads/5790@{#355} Cr-Branched-From: 1d71a33-refs/heads/main@{#1148114}
chromium · Jun 5, 2023 · 7ac28e6 · 7ac28e6
1 parent a8fe1de
commit 7ac28e6
Show file tree

Hide file tree

Showing 8 changed files with 258 additions and 1 deletion.
diff --git a/ios/chrome/browser/passwords/ios_chrome_password_manager_client.h b/ios/chrome/browser/passwords/ios_chrome_password_manager_client.h
@@ -139,6 +139,7 @@ class IOSChromePasswordManagerClient
   scoped_refptr<network::SharedURLLoaderFactory> GetURLLoaderFactory() override;
   password_manager::PasswordRequirementsService*
   GetPasswordRequirementsService() override;
+  void UpdateFormManagers() override;
   bool IsIsolationForPasswordSitesEnabled() const override;
   bool IsNewTabPage() const override;
   password_manager::FieldInfoManager* GetFieldInfoManager() const override;

diff --git a/ios/chrome/browser/passwords/ios_chrome_password_manager_client.mm b/ios/chrome/browser/passwords/ios_chrome_password_manager_client.mm
@@ -319,6 +319,10 @@
       bridge_.browserState, ServiceAccessType::EXPLICIT_ACCESS);
 }
 
+void IOSChromePasswordManagerClient::UpdateFormManagers() {
+  bridge_.passwordManager->UpdateFormManagers();
+}
+
 bool IOSChromePasswordManagerClient::IsIsolationForPasswordSitesEnabled()
     const {
   return false;

diff --git a/ios/chrome/browser/ui/passwords/bottom_sheet/BUILD.gn b/ios/chrome/browser/ui/passwords/bottom_sheet/BUILD.gn
@@ -81,6 +81,8 @@ source_set("eg2_tests") {
     "//ios/chrome/browser/passwords:eg_test_support+eg2",
     "//ios/chrome/browser/signin:fake_system_identity",
     "//ios/chrome/browser/ui/authentication:eg_test_support+eg2",
+    "//ios/chrome/browser/ui/settings/password:eg_test_support+eg2",
+    "//ios/chrome/browser/ui/settings/password:password_constants",
     "//ios/chrome/test/earl_grey:eg_test_support+eg2",
     "//ios/testing/earl_grey:eg_test_support+eg2",
     "//net",

diff --git a/ios/chrome/browser/ui/passwords/bottom_sheet/password_suggestion_bottom_sheet_egtest.mm b/ios/chrome/browser/ui/passwords/bottom_sheet/password_suggestion_bottom_sheet_egtest.mm
@@ -11,9 +11,12 @@
 #import "ios/chrome/browser/signin/fake_system_identity.h"
 #import "ios/chrome/browser/ui/authentication/signin_earl_grey_ui_test_util.h"
 #import "ios/chrome/browser/ui/passwords/bottom_sheet/password_suggestion_bottom_sheet_app_interface.h"
+#import "ios/chrome/browser/ui/settings/password/password_settings_app_interface.h"
+#import "ios/chrome/browser/ui/settings/password/passwords_table_view_constants.h"
 #import "ios/chrome/grit/ios_strings.h"
 #import "ios/chrome/test/earl_grey/chrome_actions.h"
 #import "ios/chrome/test/earl_grey/chrome_earl_grey.h"
+#import "ios/chrome/test/earl_grey/chrome_earl_grey_ui.h"
 #import "ios/chrome/test/earl_grey/chrome_matchers.h"
 #import "ios/chrome/test/earl_grey/chrome_test_case.h"
 #import "ios/testing/earl_grey/earl_grey_test.h"
@@ -41,6 +44,11 @@ BOOL WaitForKeyboardToAppear() {
   return [waitForKeyboard waitWithTimeout:kWaitForActionTimeout.InSecondsF()];
 }
 
+id<GREYMatcher> ButtonWithAccessibilityID(NSString* id) {
+  return grey_allOf(grey_accessibilityID(id),
+                    grey_accessibilityTrait(UIAccessibilityTraitButton), nil);
+}
+
 }  // namespace
 
 @interface PasswordSuggestionBottomSheetEGTest : ChromeTestCase
@@ -73,6 +81,9 @@ - (AppLaunchConfiguration)appConfigurationForTestCase {
   AppLaunchConfiguration config;
   config.features_enabled.push_back(
       password_manager::features::kIOSPasswordBottomSheet);
+  config.features_enabled.push_back(
+      password_manager::features::kPasswordsGrouping);
+  config.relaunch_policy = ForceRelaunchByKilling;
   return config;
 }
 
@@ -85,6 +96,32 @@ - (void)loadLoginPage {
   [ChromeEarlGrey waitForWebStateContainingText:"Login form."];
 }
 
+// Return the edit button from the navigation bar.
+id<GREYMatcher> NavigationBarEditButton() {
+  return grey_allOf(chrome_test_util::ButtonWithAccessibilityLabelId(
+                        IDS_IOS_NAVIGATION_BAR_EDIT_BUTTON),
+                    grey_not(chrome_test_util::TabGridEditButton()),
+                    grey_userInteractionEnabled(), nil);
+}
+
+// Matcher for the Delete button at with accessibility identifier containing
+// `username` and `password` in Password Details view.
+id<GREYMatcher> DeleteButtonForUsernameAndPassword(NSString* username,
+                                                   NSString* password) {
+  return grey_allOf(
+      grey_accessibilityID([NSString
+          stringWithFormat:@"%@%@%@", kDeleteButtonForPasswordDetailsId,
+                           username, password]),
+      grey_interactable(), nullptr);
+}
+
+// Matcher for the Delete button in Confirmation Alert for password deletion.
+id<GREYMatcher> DeleteConfirmationButton() {
+  return grey_allOf(chrome_test_util::ButtonWithAccessibilityLabel(
+                        l10n_util::GetNSString(IDS_IOS_DELETE_ACTION_TITLE)),
+                    grey_interactable(), nullptr);
+}
+
 #pragma mark - Tests
 
 - (void)testOpenPasswordBottomSheetUsePassword {
@@ -137,4 +174,193 @@ - (void)testOpenPasswordBottomSheetTapNoThanksShowKeyboard {
   WaitForKeyboardToAppear();
 }
 
+- (void)testOpenPasswordBottomSheetOpenPasswordManager {
+  [SigninEarlGreyUI signinWithFakeIdentity:[FakeSystemIdentity fakeIdentity1]
+                                enableSync:NO];
+  NSURL* URL =
+      net::NSURLWithGURL(self.testServer->GetURL("/simple_login_form.html"));
+  [PasswordSuggestionBottomSheetAppInterface setUpMockReauthenticationModule];
+  [PasswordSuggestionBottomSheetAppInterface
+      mockReauthenticationModuleExpectedResult:ReauthenticationResult::
+                                                   kSuccess];
+  [PasswordManagerAppInterface storeCredentialWithUsername:@"user"
+                                                  password:@"password"
+                                                       URL:URL];
+  [PasswordManagerAppInterface storeCredentialWithUsername:@"user2"
+                                                  password:@"password2"
+                                                       URL:URL];
+  int credentialsCount = [PasswordManagerAppInterface storedCredentialsCount];
+  GREYAssertEqual(2, credentialsCount, @"Wrong number of stored credentials.");
+
+  [self loadLoginPage];
+
+  [[EarlGrey selectElementWithMatcher:chrome_test_util::WebViewMatcher()]
+      performAction:chrome_test_util::TapWebElementWithId(kFormPassword)];
+
+  [ChromeEarlGrey
+      waitForUIElementToAppearWithMatcher:grey_accessibilityID(@"user")];
+
+  [[EarlGrey selectElementWithMatcher:grey_accessibilityID(@"user")]
+      performAction:grey_tap()];
+
+  [ChromeEarlGrey
+      waitForUIElementToAppearWithMatcher:grey_accessibilityID(@"user2")];
+
+  // Long press to open context menu.
+  [[EarlGrey selectElementWithMatcher:grey_accessibilityID(@"user2")]
+      performAction:grey_longPress()];
+
+  [ChromeEarlGreyUI waitForAppToIdle];
+
+  [[EarlGrey
+      selectElementWithMatcher:
+          grey_allOf(chrome_test_util::ButtonWithAccessibilityLabel(
+                         l10n_util::GetNSString(
+                             IDS_IOS_PASSWORD_BOTTOM_SHEET_PASSWORD_MANAGER)),
+                     grey_interactable(), nullptr)] performAction:grey_tap()];
+
+  [ChromeEarlGreyUI waitForAppToIdle];
+
+  NSString* origin =
+      [NSString stringWithFormat:@"http://%@:%@", [URL host], [URL port]];
+  [[EarlGrey
+      selectElementWithMatcher:grey_allOf(
+                                   ButtonWithAccessibilityID([NSString
+                                       stringWithFormat:@"%@, 2 accounts",
+                                                        origin]),
+                                   grey_sufficientlyVisible(), nil)]
+      assertWithMatcher:grey_notNil()];
+}
+
+- (void)testOpenPasswordBottomSheetOpenPasswordDetails {
+  [SigninEarlGreyUI signinWithFakeIdentity:[FakeSystemIdentity fakeIdentity1]
+                                enableSync:NO];
+  NSURL* URL =
+      net::NSURLWithGURL(self.testServer->GetURL("/simple_login_form.html"));
+  [PasswordSuggestionBottomSheetAppInterface setUpMockReauthenticationModule];
+  [PasswordSuggestionBottomSheetAppInterface
+      mockReauthenticationModuleExpectedResult:ReauthenticationResult::
+                                                   kSuccess];
+  [PasswordManagerAppInterface storeCredentialWithUsername:@"user"
+                                                  password:@"password"
+                                                       URL:URL];
+  [PasswordManagerAppInterface storeCredentialWithUsername:@"user2"
+                                                  password:@"password2"
+                                                       URL:URL];
+  int credentialsCount = [PasswordManagerAppInterface storedCredentialsCount];
+  GREYAssertEqual(2, credentialsCount, @"Wrong number of stored credentials.");
+
+  [self loadLoginPage];
+
+  [[EarlGrey selectElementWithMatcher:chrome_test_util::WebViewMatcher()]
+      performAction:chrome_test_util::TapWebElementWithId(kFormPassword)];
+
+  [ChromeEarlGrey
+      waitForUIElementToAppearWithMatcher:grey_accessibilityID(@"user")];
+
+  [[EarlGrey selectElementWithMatcher:grey_accessibilityID(@"user")]
+      performAction:grey_tap()];
+
+  [ChromeEarlGrey
+      waitForUIElementToAppearWithMatcher:grey_accessibilityID(@"user2")];
+
+  // Long press to open context menu.
+  [[EarlGrey selectElementWithMatcher:grey_accessibilityID(@"user2")]
+      performAction:grey_longPress()];
+
+  [ChromeEarlGreyUI waitForAppToIdle];
+
+  [[EarlGrey
+      selectElementWithMatcher:
+          grey_allOf(chrome_test_util::ButtonWithAccessibilityLabel(
+                         l10n_util::GetNSString(
+                             IDS_IOS_PASSWORD_BOTTOM_SHEET_SHOW_DETAILS)),
+                     grey_interactable(), nullptr)] performAction:grey_tap()];
+
+  [ChromeEarlGreyUI waitForAppToIdle];
+
+  [[EarlGrey
+      selectElementWithMatcher:chrome_test_util::TextFieldForCellWithLabelId(
+                                   IDS_IOS_SHOW_PASSWORD_VIEW_USERNAME)]
+      assertWithMatcher:grey_textFieldValue(@"user2")];
+}
+
+- (void)testOpenPasswordBottomSheetDeletePassword {
+  [SigninEarlGreyUI signinWithFakeIdentity:[FakeSystemIdentity fakeIdentity1]
+                                enableSync:NO];
+  NSURL* URL =
+      net::NSURLWithGURL(self.testServer->GetURL("/simple_login_form.html"));
+  [PasswordSuggestionBottomSheetAppInterface setUpMockReauthenticationModule];
+  [PasswordSuggestionBottomSheetAppInterface
+      mockReauthenticationModuleExpectedResult:ReauthenticationResult::
+                                                   kSuccess];
+  [PasswordManagerAppInterface storeCredentialWithUsername:@"user"
+                                                  password:@"password"
+                                                       URL:URL];
+  [PasswordManagerAppInterface storeCredentialWithUsername:@"user2"
+                                                  password:@"password2"
+                                                       URL:URL];
+  int credentialsCount = [PasswordManagerAppInterface storedCredentialsCount];
+  GREYAssertEqual(2, credentialsCount, @"Wrong number of stored credentials.");
+
+  [self loadLoginPage];
+
+  [[EarlGrey selectElementWithMatcher:chrome_test_util::WebViewMatcher()]
+      performAction:chrome_test_util::TapWebElementWithId(kFormPassword)];
+
+  [ChromeEarlGrey
+      waitForUIElementToAppearWithMatcher:grey_accessibilityID(@"user")];
+
+  [[EarlGrey selectElementWithMatcher:grey_accessibilityID(@"user")]
+      performAction:grey_tap()];
+
+  [ChromeEarlGrey
+      waitForUIElementToAppearWithMatcher:grey_accessibilityID(@"user2")];
+
+  // Long press to open context menu.
+  [[EarlGrey selectElementWithMatcher:grey_accessibilityID(@"user2")]
+      performAction:grey_longPress()];
+
+  [ChromeEarlGreyUI waitForAppToIdle];
+
+  [[EarlGrey
+      selectElementWithMatcher:
+          grey_allOf(chrome_test_util::ButtonWithAccessibilityLabel(
+                         l10n_util::GetNSString(
+                             IDS_IOS_PASSWORD_BOTTOM_SHEET_SHOW_DETAILS)),
+                     grey_interactable(), nullptr)] performAction:grey_tap()];
+
+  [ChromeEarlGreyUI waitForAppToIdle];
+
+  [PasswordSettingsAppInterface setUpMockReauthenticationModule];
+  [PasswordSettingsAppInterface mockReauthenticationModuleExpectedResult:
+                                    ReauthenticationResult::kSuccess];
+
+  [[EarlGrey selectElementWithMatcher:NavigationBarEditButton()]
+      performAction:grey_tap()];
+
+  [[EarlGrey selectElementWithMatcher:DeleteButtonForUsernameAndPassword(
+                                          @"user2", @"password2")]
+      performAction:grey_tap()];
+
+  [[EarlGrey selectElementWithMatcher:DeleteConfirmationButton()]
+      performAction:grey_tap()];
+
+  // Wait until the alert and the detail view are dismissed.
+  [ChromeEarlGreyUI waitForAppToIdle];
+
+  // Verify that user2 is not available anymore.
+  [[EarlGrey selectElementWithMatcher:chrome_test_util::WebViewMatcher()]
+      performAction:chrome_test_util::TapWebElementWithId(kFormPassword)];
+
+  [ChromeEarlGrey
+      waitForUIElementToAppearWithMatcher:grey_accessibilityID(@"user")];
+
+  [[EarlGrey selectElementWithMatcher:grey_accessibilityID(@"user")]
+      performAction:grey_tap()];
+
+  [[EarlGrey selectElementWithMatcher:grey_accessibilityID(@"user2")]
+      assertWithMatcher:grey_nil()];
+}
+
 @end
diff --git a/ios/chrome/browser/ui/passwords/bottom_sheet/password_suggestion_bottom_sheet_mediator.mm b/ios/chrome/browser/ui/passwords/bottom_sheet/password_suggestion_bottom_sheet_mediator.mm
@@ -154,7 +154,7 @@ - (instancetype)initWithWebStateList:(WebStateList*)webStateList
           initWithProfilePasswordStore:_profilePasswordStore
                   accountPasswordStore:_accountPasswordStore
                               delegate:self
-                                   URL:_URL];
+                                   URL:url::Origin::Create(_URL).GetURL()];
     }
   }
   return self;

diff --git a/ios/chrome/browser/ui/settings/password/password_details/password_details_coordinator.mm b/ios/chrome/browser/ui/settings/password/password_details/password_details_coordinator.mm
@@ -10,6 +10,7 @@
 #import "base/mac/foundation_util.h"
 #import "base/memory/scoped_refptr.h"
 #import "base/strings/sys_string_conversions.h"
+#import "components/password_manager/core/browser/password_manager_client.h"
 #import "components/password_manager/core/browser/ui/affiliated_group.h"
 #import "components/password_manager/core/browser/ui/credential_ui_entry.h"
 #import "components/password_manager/core/common/password_manager_features.h"
@@ -40,6 +41,7 @@
 #import "ios/chrome/browser/ui/settings/utils/password_utils.h"
 #import "ios/chrome/common/ui/reauthentication/reauthentication_module.h"
 #import "ios/chrome/grit/ios_strings.h"
+#import "ios/web/public/web_state.h"
 #import "ui/base/l10n/l10n_util.h"
 
 #if !defined(__has_feature) || !__has_feature(objc_arc)
@@ -396,4 +398,14 @@ - (void)showDismissWarningDialogWithPasswordDetails:(PasswordDetails*)password {
   [self.alertCoordinator start];
 }
 
+- (void)updateFormManagers {
+  web::WebState* activeWebState =
+      self.browser->GetWebStateList()->GetActiveWebState();
+  DCHECK(activeWebState);
+  password_manager::PasswordManagerClient* passwordManagerClient =
+      PasswordTabHelper::FromWebState(activeWebState)
+          ->GetPasswordManagerClient();
+  passwordManagerClient->UpdateFormManagers();
+}
+
 @end
diff --git a/ios/chrome/browser/ui/settings/password/password_details/password_details_mediator.mm b/ios/chrome/browser/ui/settings/password/password_details/password_details_mediator.mm
@@ -262,6 +262,10 @@ - (void)removeCredential:(PasswordDetails*)password {
   // be multiple credentials; nor username/password since the values changed).
   base::Erase(_credentials, *it);
   [self providePasswordsToConsumer];
+
+  // Update form managers so the list of password suggestions shown to the user
+  // is the correct one.
+  [_delegate updateFormManagers];
 }
 
 - (void)moveCredentialToAccountStore:(PasswordDetails*)password {
@@ -371,6 +375,10 @@ - (void)passwordDetailsViewController:
 
       // Update the credential in the credentials vector.
       *it = std::move(updated_credential);
+
+      // Update form managers so the list of password suggestions shown to the
+      // user is the correct one.
+      [_delegate updateFormManagers];
       return;
     }
   }

diff --git a/...chrome/browser/ui/settings/password/password_details/password_details_mediator_delegate.h b/...chrome/browser/ui/settings/password/password_details/password_details_mediator_delegate.h
@@ -11,6 +11,10 @@
 // Called when the user wants to dismiss a compromised credential warning.
 - (void)showDismissWarningDialogWithPasswordDetails:(PasswordDetails*)password;
 
+// Called when a credential has been updated or deleted. This will refresh the
+// password suggestions list.
+- (void)updateFormManagers;
+
 @end
 
 #endif  // IOS_CHROME_BROWSER_UI_SETTINGS_PASSWORD_PASSWORD_DETAILS_PASSWORD_DETAILS_MEDIATOR_DELEGATE_H_