Skip to content

Commit

Permalink
[ios] Password Messages uses IOSChromeSavePasswordInfoBarDelegate only.
Browse files Browse the repository at this point in the history
InfobarPasswordCoordinator now only owns a IOSChromeSavePasswordInfoBarDelegate
instead of the parent class.

IOSChromeSavePasswordInfoBarDelegate will be in charge to both Save/Update
credentials.

This CL leaves IOSChromeUpdatePasswordInfoBarDelegate as it was before the
messages implementation. And removes the virtual methods from the parent
class since these can now be part of IOSChromeSavePasswordInfoBarDelegate
public interface.

Bug: 945478
Change-Id: I8968643cda4e872317fd658f5ca42af7c6616035
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1581830
Commit-Queue: Sergio Collazos <sczs@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: Chris Lu <thegreenfrog@chromium.org>
Reviewed-by: Peter Lee <pkl@chromium.org>
Cr-Commit-Position: refs/heads/master@{#655605}
  • Loading branch information
sczs authored and Commit Bot committed May 1, 2019
1 parent 7b1d5c3 commit b5d52c1
Show file tree
Hide file tree
Showing 13 changed files with 108 additions and 87 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,6 @@ class IOSChromePasswordManagerInfoBarDelegate : public ConfirmInfoBarDelegate {
public:
~IOSChromePasswordManagerInfoBarDelegate() override;

// Updates the credentials being saved with |username| and |password|.
void UpdateCredentials(NSString* username, NSString* password);

// Getter for the message displayed in addition to the title. If no message
// was set, this returns an empty string.
NSString* GetDetailsMessageText() const;
Expand All @@ -40,9 +37,6 @@ class IOSChromePasswordManagerInfoBarDelegate : public ConfirmInfoBarDelegate {
// The URL host for which the credentials are being saved for.
NSString* GetURLHostText() const;

// The title for the InfobarModal being presented.
virtual NSString* GetInfobarModalTitleText() const;

// Sets the dispatcher for this delegate.
void set_dispatcher(id<ApplicationCommands> dispatcher);

Expand All @@ -67,13 +61,13 @@ class IOSChromePasswordManagerInfoBarDelegate : public ConfirmInfoBarDelegate {
}

private:
// ConfirmInfoBarDelegate implementation.
int GetIconId() const override;

// The password_manager::PasswordFormManager managing the form we're asking
// the user about, and should save as per their decision.
std::unique_ptr<password_manager::PasswordFormManagerForUI> form_to_save_;

// ConfirmInfoBarDelegate implementation.
int GetIconId() const override;

// Used to track the results we get from the info bar.
password_manager::metrics_util::UIDismissalReason infobar_response_;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
#include "base/strings/string16.h"
#include "base/strings/sys_string_conversions.h"
#include "components/password_manager/core/browser/password_form_manager_for_ui.h"
#include "components/password_manager/core/browser/password_ui_utils.h"
#include "ios/chrome/grit/ios_strings.h"
#include "ios/chrome/grit/ios_theme_resources.h"
#include "ui/base/l10n/l10n_util.h"
Expand All @@ -31,16 +30,6 @@
infobar_response_(password_manager::metrics_util::NO_DIRECT_INTERACTION),
is_sync_user_(is_sync_user) {}

void IOSChromePasswordManagerInfoBarDelegate::UpdateCredentials(
NSString* username,
NSString* password) {
const base::string16 username_string = base::SysNSStringToUTF16(username);
const base::string16 password_string = base::SysNSStringToUTF16(password);
UpdatePasswordFormUsernameAndPassword(username_string, password_string,
form_to_save_.get());
form_to_save()->Save();
}

NSString* IOSChromePasswordManagerInfoBarDelegate::GetDetailsMessageText()
const {
return is_sync_user_ ? l10n_util::GetNSString(IDS_SAVE_PASSWORD_FOOTER) : @"";
Expand All @@ -60,12 +49,6 @@
return base::SysUTF8ToNSString(form_to_save_->GetOrigin().host());
}

NSString* IOSChromePasswordManagerInfoBarDelegate::GetInfobarModalTitleText()
const {
NOTREACHED() << "Subclass must implement.";
return @"";
}

void IOSChromePasswordManagerInfoBarDelegate::set_dispatcher(
id<ApplicationCommands> dispatcher) {
dispatcher_ = dispatcher;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,27 +20,44 @@ class PasswordFormManagerForUI;
// password_manager::PasswordFormManager and move it to a
// IOSChromeSavePasswordInfoBarDelegate while the user makes up their mind
// with the "save password" infobar.
// If |password_update| is true the delegate will use "Update" related strings,
// and should Update the credentials instead of Saving new ones.
class IOSChromeSavePasswordInfoBarDelegate
: public IOSChromePasswordManagerInfoBarDelegate {
public:
IOSChromeSavePasswordInfoBarDelegate(
bool is_sync_user,
bool password_update,
std::unique_ptr<password_manager::PasswordFormManagerForUI> form_to_save);

~IOSChromeSavePasswordInfoBarDelegate() override;

// InfoBarDelegate implementation
bool ShouldExpire(const NavigationDetails& details) const override;

private:
// ConfirmInfoBarDelegate implementation.
infobars::InfoBarDelegate::InfoBarIdentifier GetIdentifier() const override;
base::string16 GetMessageText() const override;
base::string16 GetButtonLabel(InfoBarButton button) const override;
bool Accept() override;
bool Cancel() override;

// IOSChromePasswordManagerInfoBarDelegate implementation.
NSString* GetInfobarModalTitleText() const override;
// Updates the credentials being saved with |username| and |password|.
void UpdateCredentials(NSString* username, NSString* password);

// true if password is being updated at the moment the InfobarModal is
// created.
bool IsPasswordUpdate() const;

// The title for the InfobarModal being presented.
NSString* GetInfobarModalTitleText() const;

private:
// ConfirmInfoBarDelegate implementation.
infobars::InfoBarDelegate::InfoBarIdentifier GetIdentifier() const override;

// true if password is being updated at the moment the InfobarModal is
// created.
bool password_update_ = false;

DISALLOW_COPY_AND_ASSIGN(IOSChromeSavePasswordInfoBarDelegate);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,13 @@
#include <utility>

#include "base/memory/ptr_util.h"
#include "base/strings/sys_string_conversions.h"
#include "components/infobars/core/infobar.h"
#include "components/infobars/core/infobar_manager.h"
#include "components/password_manager/core/browser/password_form_manager_for_ui.h"
#include "components/password_manager/core/browser/password_form_metrics_recorder.h"
#include "components/password_manager/core/browser/password_manager_constants.h"
#include "components/password_manager/core/browser/password_ui_utils.h"
#include "components/strings/grit/components_strings.h"
#import "ios/chrome/browser/ui/infobars/infobar_feature.h"
#include "ios/chrome/grit/ios_chromium_strings.h"
Expand All @@ -26,9 +28,11 @@

IOSChromeSavePasswordInfoBarDelegate::IOSChromeSavePasswordInfoBarDelegate(
bool is_sync_user,
bool password_update,
std::unique_ptr<PasswordFormManagerForUI> form_manager)
: IOSChromePasswordManagerInfoBarDelegate(is_sync_user,
std::move(form_manager)) {
std::move(form_manager)),
password_update_(password_update) {
form_to_save()->GetMetricsRecorder()->RecordPasswordBubbleShown(
form_to_save()->GetCredentialSource(),
password_manager::metrics_util::AUTOMATIC_WITH_PASSWORD_PENDING);
Expand All @@ -46,23 +50,39 @@
}

base::string16 IOSChromeSavePasswordInfoBarDelegate::GetMessageText() const {
if (IsInfobarUIRebootEnabled() && IsPasswordUpdate()) {
return l10n_util::GetStringUTF16(IDS_IOS_PASSWORD_MANAGER_UPDATE_PASSWORD);
}
return l10n_util::GetStringUTF16(
IDS_IOS_PASSWORD_MANAGER_SAVE_PASSWORD_PROMPT);
}

NSString* IOSChromeSavePasswordInfoBarDelegate::GetInfobarModalTitleText()
const {
DCHECK(IsInfobarUIRebootEnabled());
return l10n_util::GetNSString(IDS_IOS_PASSWORD_MANAGER_SAVE_PASSWORD_TITLE);
return l10n_util::GetNSString(
IsPasswordUpdate() ? IDS_IOS_PASSWORD_MANAGER_UPDATE_PASSWORD_TITLE
: IDS_IOS_PASSWORD_MANAGER_SAVE_PASSWORD_TITLE);
}

base::string16 IOSChromeSavePasswordInfoBarDelegate::GetButtonLabel(
InfoBarButton button) const {
if (IsInfobarUIRebootEnabled()) {
return l10n_util::GetStringUTF16(
(button == BUTTON_OK)
? IDS_IOS_PASSWORD_MANAGER_SAVE_BUTTON
: IDS_IOS_PASSWORD_MANAGER_MODAL_BLACKLIST_BUTTON);
switch (button) {
case BUTTON_OK:
return l10n_util::GetStringUTF16(
IsPasswordUpdate() ? IDS_IOS_PASSWORD_MANAGER_UPDATE_BUTTON
: IDS_IOS_PASSWORD_MANAGER_SAVE_BUTTON);
case BUTTON_CANCEL: {
return IsPasswordUpdate()
? base::string16()
: l10n_util::GetStringUTF16(
IDS_IOS_PASSWORD_MANAGER_MODAL_BLACKLIST_BUTTON);
}
case BUTTON_NONE:
NOTREACHED();
return base::string16();
}
} else {
return l10n_util::GetStringUTF16(
(button == BUTTON_OK) ? IDS_IOS_PASSWORD_MANAGER_SAVE_BUTTON
Expand All @@ -74,11 +94,13 @@
DCHECK(form_to_save());
form_to_save()->Save();
set_infobar_response(password_manager::metrics_util::CLICKED_SAVE);
password_update_ = true;
return true;
}

bool IOSChromeSavePasswordInfoBarDelegate::Cancel() {
DCHECK(form_to_save());
DCHECK(!password_update_);
form_to_save()->PermanentlyBlacklist();
set_infobar_response(password_manager::metrics_util::CLICKED_NEVER);
return true;
Expand All @@ -88,3 +110,18 @@
const NavigationDetails& details) const {
return !details.is_redirect && ConfirmInfoBarDelegate::ShouldExpire(details);
}

void IOSChromeSavePasswordInfoBarDelegate::UpdateCredentials(
NSString* username,
NSString* password) {
DCHECK(IsInfobarUIRebootEnabled());
const base::string16 username_string = base::SysNSStringToUTF16(username);
const base::string16 password_string = base::SysNSStringToUTF16(password);
UpdatePasswordFormUsernameAndPassword(username_string, password_string,
form_to_save());
}

bool IOSChromeSavePasswordInfoBarDelegate::IsPasswordUpdate() const {
DCHECK(IsInfobarUIRebootEnabled());
return password_update_;
}
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,6 @@ class IOSChromeUpdatePasswordInfoBarDelegate
bool Cancel() override;
base::string16 GetLinkText() const override;

// IOSChromePasswordManagerInfoBarDelegate implementation.
NSString* GetInfobarModalTitleText() const override;

// The credential that should be displayed in the infobar, and for which the
// password will be updated.
base::string16 selected_account_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@
#include "components/strings/grit/components_strings.h"
#include "ios/chrome/browser/infobars/infobar.h"
#import "ios/chrome/browser/passwords/update_password_infobar_controller.h"
#import "ios/chrome/browser/ui/infobars/coordinators/infobar_password_coordinator.h"
#import "ios/chrome/browser/ui/infobars/infobar_feature.h"
#include "ios/chrome/grit/ios_chromium_strings.h"
#include "ios/chrome/grit/ios_strings.h"
#include "ui/base/l10n/l10n_util.h"
Expand All @@ -41,20 +39,12 @@
is_sync_user, std::move(form_manager)));
delegate->set_dispatcher(dispatcher);

if (IsInfobarUIRebootEnabled()) {
InfobarPasswordCoordinator* coordinator =
[[InfobarPasswordCoordinator alloc]
initWithInfoBarDelegate:delegate.get()];
infobar_manager->AddInfoBar(
std::make_unique<InfoBarIOS>(coordinator, std::move(delegate)));
} else {
UpdatePasswordInfoBarController* controller =
[[UpdatePasswordInfoBarController alloc]
initWithBaseViewController:baseViewController
infoBarDelegate:delegate.get()];
infobar_manager->AddInfoBar(
std::make_unique<InfoBarIOS>(controller, std::move(delegate)));
}
UpdatePasswordInfoBarController* controller =
[[UpdatePasswordInfoBarController alloc]
initWithBaseViewController:baseViewController
infoBarDelegate:delegate.get()];
infobar_manager->AddInfoBar(
std::make_unique<InfoBarIOS>(controller, std::move(delegate)));
}

IOSChromeUpdatePasswordInfoBarDelegate::
Expand Down Expand Up @@ -102,26 +92,14 @@
IDS_IOS_PASSWORD_MANAGER_UPDATE_PASSWORD);
}

NSString* IOSChromeUpdatePasswordInfoBarDelegate::GetInfobarModalTitleText()
const {
DCHECK(IsInfobarUIRebootEnabled());
return l10n_util::GetNSString(IDS_IOS_PASSWORD_MANAGER_UPDATE_PASSWORD_TITLE);
}

int IOSChromeUpdatePasswordInfoBarDelegate::GetButtons() const {
return BUTTON_OK;
}

base::string16 IOSChromeUpdatePasswordInfoBarDelegate::GetButtonLabel(
InfoBarButton button) const {
if (IsInfobarUIRebootEnabled()) {
return (button == BUTTON_OK) ? l10n_util::GetStringUTF16(
IDS_IOS_PASSWORD_MANAGER_UPDATE_BUTTON)
: base::string16();
} else {
DCHECK_EQ(BUTTON_OK, button);
return l10n_util::GetStringUTF16(IDS_IOS_PASSWORD_MANAGER_UPDATE_BUTTON);
}
DCHECK_EQ(BUTTON_OK, button);
return l10n_util::GetStringUTF16(IDS_IOS_PASSWORD_MANAGER_UPDATE_BUTTON);
}

bool IOSChromeUpdatePasswordInfoBarDelegate::Accept() {
Expand Down
23 changes: 18 additions & 5 deletions ios/chrome/browser/passwords/password_controller.mm
Original file line number Diff line number Diff line change
Expand Up @@ -680,7 +680,7 @@ - (void)showInfoBarForForm:(std::unique_ptr<PasswordFormManagerForUI>)form
switch (type) {
case PasswordInfoBarType::SAVE: {
auto delegate = std::make_unique<IOSChromeSavePasswordInfoBarDelegate>(
isSyncUser, std::move(form));
isSyncUser, /*password_update*/ false, std::move(form));
delegate->set_dispatcher(self.dispatcher);

if (IsInfobarUIRebootEnabled()) {
Expand All @@ -698,11 +698,24 @@ - (void)showInfoBarForForm:(std::unique_ptr<PasswordFormManagerForUI>)form
}
break;
}
case PasswordInfoBarType::UPDATE:
IOSChromeUpdatePasswordInfoBarDelegate::Create(
isSyncUser, infoBarManager, std::move(form), self.baseViewController,
self.dispatcher);
case PasswordInfoBarType::UPDATE: {
if (IsInfobarUIRebootEnabled()) {
auto delegate = std::make_unique<IOSChromeSavePasswordInfoBarDelegate>(
isSyncUser, /*password_update*/ true, std::move(form));
delegate->set_dispatcher(self.dispatcher);
InfobarPasswordCoordinator* coordinator =
[[InfobarPasswordCoordinator alloc]
initWithInfoBarDelegate:delegate.get()];
infoBarManager->AddInfoBar(
std::make_unique<InfoBarIOS>(coordinator, std::move(delegate)));

} else {
IOSChromeUpdatePasswordInfoBarDelegate::Create(
isSyncUser, infoBarManager, std::move(form),
self.baseViewController, self.dispatcher);
}
break;
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ - (UIView*)bannerView {

#pragma mark InfobarModalDelegate

- (void)modalInfobarButtonWasPressed:(UIButton*)sender {
- (void)modalInfobarButtonWasAccepted:(id)sender {
[self performInfobarAction];
[self.badgeDelegate infobarWasAccepted];
[self dismissInfobarModal:sender animated:YES completion:nil];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@

#import "ios/chrome/browser/ui/infobars/coordinators/infobar_coordinator.h"

class IOSChromePasswordManagerInfoBarDelegate;
class IOSChromeSavePasswordInfoBarDelegate;

// Coordinator that creates and manages the PasswordInfobar.
@interface InfobarPasswordCoordinator : InfobarCoordinator

- (instancetype)initWithInfoBarDelegate:
(IOSChromePasswordManagerInfoBarDelegate*)passwordInfoBarDelegate
(IOSChromeSavePasswordInfoBarDelegate*)passwordInfoBarDelegate
NS_DESIGNATED_INITIALIZER;

@end
Expand Down
Loading

0 comments on commit b5d52c1

Please sign in to comment.