Commit a4c5849f authored by Elly Fong-Jones's avatar Elly Fong-Jones Committed by Commit Bot

autofill: migrate more GetDialogButtonLabel -> set_button_label

This change migrates all the other overrides in
//chrome/browser/ui/views/autofill of GetDialogButtonLabel over to
set_button_label. Many of these dialogs dynamically compute their label text
based on the state of their models/controllers. That causes some difficulty with
the "push-based" (client sets properties) design DialogDelegate is migrating to
from its existing "pull-based" (DialogDelegate asks client for properties)
design.

This change migrates:
1) CardUnmaskPromptViews, by adding an UpdateButtonLabels private method to this
   class that pushes updated label text to DialogDelegate when needed and
   calling it at every point that DialogModelChanged is called.
2) LocalCardMigrationBubbleViews, which uses a static ok button text.
3) SaveCardBubbleViews, which uses controller-generated text but never updates
   it in use (no calls to DialogModelChanged)
4) WebAuthnOfferDialogViewImpl, which already has a RefreshContent method, by
   pushing new button text in that method.

Bug: 1011446
Change-Id: Iaf497223e3c44c3a6227e264e40e441f2fbe389f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1854689Reviewed-by: default avatarSiyu An <siyua@chromium.org>
Reviewed-by: default avatarVasilii Sukhanov <vasilii@chromium.org>
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#706515}
parent 3a1ce1c3
...@@ -378,7 +378,6 @@ base::string16 SaveCardBubbleControllerImpl::GetAcceptButtonText() const { ...@@ -378,7 +378,6 @@ base::string16 SaveCardBubbleControllerImpl::GetAcceptButtonText() const {
case BubbleType::SIGN_IN_PROMO: case BubbleType::SIGN_IN_PROMO:
case BubbleType::FAILURE: case BubbleType::FAILURE:
case BubbleType::INACTIVE: case BubbleType::INACTIVE:
NOTREACHED();
return base::string16(); return base::string16();
} }
} }
...@@ -418,7 +417,6 @@ base::string16 SaveCardBubbleControllerImpl::GetDeclineButtonText() const { ...@@ -418,7 +417,6 @@ base::string16 SaveCardBubbleControllerImpl::GetDeclineButtonText() const {
case BubbleType::SIGN_IN_PROMO: case BubbleType::SIGN_IN_PROMO:
case BubbleType::FAILURE: case BubbleType::FAILURE:
case BubbleType::INACTIVE: case BubbleType::INACTIVE:
NOTREACHED();
return base::string16(); return base::string16();
} }
} }
......
...@@ -71,6 +71,7 @@ CardUnmaskPromptViews::CardUnmaskPromptViews( ...@@ -71,6 +71,7 @@ CardUnmaskPromptViews::CardUnmaskPromptViews(
content::WebContents* web_contents) content::WebContents* web_contents)
: controller_(controller), web_contents_(web_contents) { : controller_(controller), web_contents_(web_contents) {
chrome::RecordDialogCreation(chrome::DialogIdentifier::CARD_UNMASK); chrome::RecordDialogCreation(chrome::DialogIdentifier::CARD_UNMASK);
UpdateButtonLabels();
} }
CardUnmaskPromptViews::~CardUnmaskPromptViews() { CardUnmaskPromptViews::~CardUnmaskPromptViews() {
...@@ -92,6 +93,7 @@ void CardUnmaskPromptViews::DisableAndWaitForVerification() { ...@@ -92,6 +93,7 @@ void CardUnmaskPromptViews::DisableAndWaitForVerification() {
controls_container_->SetVisible(false); controls_container_->SetVisible(false);
overlay_->SetVisible(true); overlay_->SetVisible(true);
progress_throbber_->Start(); progress_throbber_->Start();
UpdateButtonLabels();
DialogModelChanged(); DialogModelChanged();
Layout(); Layout();
} }
...@@ -153,6 +155,7 @@ void CardUnmaskPromptViews::GotVerificationResult( ...@@ -153,6 +155,7 @@ void CardUnmaskPromptViews::GotVerificationResult(
layout->AddView(std::move(error_icon)); layout->AddView(std::move(error_icon));
layout->AddView(std::move(error_label)); layout->AddView(std::move(error_label));
} }
UpdateButtonLabels();
DialogModelChanged(); DialogModelChanged();
} }
...@@ -172,6 +175,7 @@ void CardUnmaskPromptViews::LinkClicked(views::Link* source, int event_flags) { ...@@ -172,6 +175,7 @@ void CardUnmaskPromptViews::LinkClicked(views::Link* source, int event_flags) {
input_row_->InvalidateLayout(); input_row_->InvalidateLayout();
cvc_input_->SetInvalid(false); cvc_input_->SetInvalid(false);
cvc_input_->SetText(base::string16()); cvc_input_->SetText(base::string16());
UpdateButtonLabels();
DialogModelChanged(); DialogModelChanged();
GetWidget()->UpdateWindowTitle(); GetWidget()->UpdateWindowTitle();
instructions_->SetText(controller_->GetInstructionsMessage()); instructions_->SetText(controller_->GetInstructionsMessage());
...@@ -284,14 +288,6 @@ int CardUnmaskPromptViews::GetDialogButtons() const { ...@@ -284,14 +288,6 @@ int CardUnmaskPromptViews::GetDialogButtons() const {
return ui::DIALOG_BUTTON_OK | ui::DIALOG_BUTTON_CANCEL; return ui::DIALOG_BUTTON_OK | ui::DIALOG_BUTTON_CANCEL;
} }
base::string16 CardUnmaskPromptViews::GetDialogButtonLabel(
ui::DialogButton button) const {
if (button == ui::DIALOG_BUTTON_OK)
return controller_->GetOkButtonLabel();
return DialogDelegateView::GetDialogButtonLabel(button);
}
bool CardUnmaskPromptViews::IsDialogButtonEnabled( bool CardUnmaskPromptViews::IsDialogButtonEnabled(
ui::DialogButton button) const { ui::DialogButton button) const {
if (button == ui::DIALOG_BUTTON_CANCEL) if (button == ui::DIALOG_BUTTON_CANCEL)
...@@ -339,6 +335,7 @@ void CardUnmaskPromptViews::ContentsChanged( ...@@ -339,6 +335,7 @@ void CardUnmaskPromptViews::ContentsChanged(
if (controller_->InputCvcIsValid(new_contents)) if (controller_->InputCvcIsValid(new_contents))
cvc_input_->SetInvalid(false); cvc_input_->SetInvalid(false);
UpdateButtonLabels();
DialogModelChanged(); DialogModelChanged();
} }
...@@ -359,6 +356,7 @@ void CardUnmaskPromptViews::OnPerformAction(views::Combobox* combobox) { ...@@ -359,6 +356,7 @@ void CardUnmaskPromptViews::OnPerformAction(views::Combobox* combobox) {
IDS_AUTOFILL_CARD_UNMASK_INVALID_EXPIRATION_DATE)); IDS_AUTOFILL_CARD_UNMASK_INVALID_EXPIRATION_DATE));
} }
UpdateButtonLabels();
DialogModelChanged(); DialogModelChanged();
} }
...@@ -491,6 +489,11 @@ void CardUnmaskPromptViews::ClosePrompt() { ...@@ -491,6 +489,11 @@ void CardUnmaskPromptViews::ClosePrompt() {
GetWidget()->Close(); GetWidget()->Close();
} }
void CardUnmaskPromptViews::UpdateButtonLabels() {
DialogDelegate::set_button_label(ui::DIALOG_BUTTON_OK,
controller_->GetOkButtonLabel());
}
CardUnmaskPromptView* CreateCardUnmaskPromptView( CardUnmaskPromptView* CreateCardUnmaskPromptView(
CardUnmaskPromptController* controller, CardUnmaskPromptController* controller,
content::WebContents* web_contents) { content::WebContents* web_contents) {
......
...@@ -61,7 +61,6 @@ class CardUnmaskPromptViews : public CardUnmaskPromptView, ...@@ -61,7 +61,6 @@ class CardUnmaskPromptViews : public CardUnmaskPromptView,
base::string16 GetWindowTitle() const override; base::string16 GetWindowTitle() const override;
void DeleteDelegate() override; void DeleteDelegate() override;
int GetDialogButtons() const override; int GetDialogButtons() const override;
base::string16 GetDialogButtonLabel(ui::DialogButton button) const override;
bool IsDialogButtonEnabled(ui::DialogButton button) const override; bool IsDialogButtonEnabled(ui::DialogButton button) const override;
View* GetInitiallyFocusedView() override; View* GetInitiallyFocusedView() override;
bool ShouldShowCloseButton() const override; bool ShouldShowCloseButton() const override;
...@@ -88,6 +87,8 @@ class CardUnmaskPromptViews : public CardUnmaskPromptView, ...@@ -88,6 +87,8 @@ class CardUnmaskPromptViews : public CardUnmaskPromptView,
void ShowNewCardLink(); void ShowNewCardLink();
void ClosePrompt(); void ClosePrompt();
void UpdateButtonLabels();
CardUnmaskPromptController* controller_; CardUnmaskPromptController* controller_;
content::WebContents* web_contents_; content::WebContents* web_contents_;
......
...@@ -50,6 +50,10 @@ LocalCardMigrationBubbleViews::LocalCardMigrationBubbleViews( ...@@ -50,6 +50,10 @@ LocalCardMigrationBubbleViews::LocalCardMigrationBubbleViews(
: LocationBarBubbleDelegateView(anchor_view, web_contents), : LocationBarBubbleDelegateView(anchor_view, web_contents),
controller_(controller) { controller_(controller) {
DCHECK(controller); DCHECK(controller);
DialogDelegate::set_button_label(
ui::DIALOG_BUTTON_OK,
l10n_util::GetStringUTF16(
IDS_AUTOFILL_LOCAL_CARD_MIGRATION_BUBBLE_BUTTON_LABEL));
} }
void LocalCardMigrationBubbleViews::Show(DisplayReason reason) { void LocalCardMigrationBubbleViews::Show(DisplayReason reason) {
...@@ -87,13 +91,6 @@ int LocalCardMigrationBubbleViews::GetDialogButtons() const { ...@@ -87,13 +91,6 @@ int LocalCardMigrationBubbleViews::GetDialogButtons() const {
return ui::DIALOG_BUTTON_OK; return ui::DIALOG_BUTTON_OK;
} }
base::string16 LocalCardMigrationBubbleViews::GetDialogButtonLabel(
ui::DialogButton button) const {
DCHECK_EQ(button, ui::DIALOG_BUTTON_OK);
return l10n_util::GetStringUTF16(
IDS_AUTOFILL_LOCAL_CARD_MIGRATION_BUBBLE_BUTTON_LABEL);
}
gfx::Size LocalCardMigrationBubbleViews::CalculatePreferredSize() const { gfx::Size LocalCardMigrationBubbleViews::CalculatePreferredSize() const {
const int width = ChromeLayoutProvider::Get()->GetDistanceMetric( const int width = ChromeLayoutProvider::Get()->GetDistanceMetric(
DISTANCE_BUBBLE_PREFERRED_WIDTH) - DISTANCE_BUBBLE_PREFERRED_WIDTH) -
......
...@@ -38,7 +38,6 @@ class LocalCardMigrationBubbleViews : public LocalCardMigrationBubble, ...@@ -38,7 +38,6 @@ class LocalCardMigrationBubbleViews : public LocalCardMigrationBubble,
bool Cancel() override; bool Cancel() override;
bool Close() override; bool Close() override;
int GetDialogButtons() const override; int GetDialogButtons() const override;
base::string16 GetDialogButtonLabel(ui::DialogButton button) const override;
gfx::Size CalculatePreferredSize() const override; gfx::Size CalculatePreferredSize() const override;
void AddedToWidget() override; void AddedToWidget() override;
bool ShouldShowCloseButton() const override; bool ShouldShowCloseButton() const override;
......
...@@ -55,6 +55,10 @@ SaveCardBubbleViews::SaveCardBubbleViews(views::View* anchor_view, ...@@ -55,6 +55,10 @@ SaveCardBubbleViews::SaveCardBubbleViews(views::View* anchor_view,
SaveCardBubbleController* controller) SaveCardBubbleController* controller)
: LocationBarBubbleDelegateView(anchor_view, web_contents), : LocationBarBubbleDelegateView(anchor_view, web_contents),
controller_(controller) { controller_(controller) {
DialogDelegate::set_button_label(ui::DIALOG_BUTTON_OK,
controller->GetAcceptButtonText());
DialogDelegate::set_button_label(ui::DIALOG_BUTTON_CANCEL,
controller->GetDeclineButtonText());
DCHECK(controller); DCHECK(controller);
chrome::RecordDialogCreation(chrome::DialogIdentifier::SAVE_CARD); chrome::RecordDialogCreation(chrome::DialogIdentifier::SAVE_CARD);
} }
...@@ -75,12 +79,6 @@ void SaveCardBubbleViews::Hide() { ...@@ -75,12 +79,6 @@ void SaveCardBubbleViews::Hide() {
CloseBubble(); CloseBubble();
} }
base::string16 SaveCardBubbleViews::GetDialogButtonLabel(
ui::DialogButton button) const {
return button == ui::DIALOG_BUTTON_OK ? controller()->GetAcceptButtonText()
: controller()->GetDeclineButtonText();
}
std::unique_ptr<views::View> SaveCardBubbleViews::CreateFootnoteView() { std::unique_ptr<views::View> SaveCardBubbleViews::CreateFootnoteView() {
return nullptr; return nullptr;
} }
......
...@@ -39,7 +39,6 @@ class SaveCardBubbleViews : public SaveCardBubbleView, ...@@ -39,7 +39,6 @@ class SaveCardBubbleViews : public SaveCardBubbleView,
void Hide() override; void Hide() override;
// views::BubbleDialogDelegateView: // views::BubbleDialogDelegateView:
base::string16 GetDialogButtonLabel(ui::DialogButton button) const override;
std::unique_ptr<views::View> CreateFootnoteView() override; std::unique_ptr<views::View> CreateFootnoteView() override;
bool Accept() override; bool Accept() override;
bool Cancel() override; bool Cancel() override;
......
...@@ -30,6 +30,11 @@ WebauthnOfferDialogViewImpl::WebauthnOfferDialogViewImpl( ...@@ -30,6 +30,11 @@ WebauthnOfferDialogViewImpl::WebauthnOfferDialogViewImpl(
sheet_view_ = sheet_view_ =
AddChildView(CreateSheetViewForAutofillWebAuthn(std::move(model))); AddChildView(CreateSheetViewForAutofillWebAuthn(std::move(model)));
sheet_view_->ReInitChildViews(); sheet_view_->ReInitChildViews();
DialogDelegate::set_button_label(ui::DIALOG_BUTTON_OK,
model_->GetAcceptButtonLabel());
DialogDelegate::set_button_label(ui::DIALOG_BUTTON_CANCEL,
model_->GetCancelButtonLabel());
} }
WebauthnOfferDialogViewImpl::~WebauthnOfferDialogViewImpl() { WebauthnOfferDialogViewImpl::~WebauthnOfferDialogViewImpl() {
...@@ -104,12 +109,6 @@ int WebauthnOfferDialogViewImpl::GetDialogButtons() const { ...@@ -104,12 +109,6 @@ int WebauthnOfferDialogViewImpl::GetDialogButtons() const {
: ui::DIALOG_BUTTON_CANCEL; : ui::DIALOG_BUTTON_CANCEL;
} }
base::string16 WebauthnOfferDialogViewImpl::GetDialogButtonLabel(
ui::DialogButton button) const {
return button == ui::DIALOG_BUTTON_OK ? model_->GetAcceptButtonLabel()
: model_->GetCancelButtonLabel();
}
bool WebauthnOfferDialogViewImpl::IsDialogButtonEnabled( bool WebauthnOfferDialogViewImpl::IsDialogButtonEnabled(
ui::DialogButton button) const { ui::DialogButton button) const {
return button == ui::DIALOG_BUTTON_OK ? model_->IsAcceptButtonEnabled() return button == ui::DIALOG_BUTTON_OK ? model_->IsAcceptButtonEnabled()
...@@ -146,6 +145,10 @@ void WebauthnOfferDialogViewImpl::Hide() { ...@@ -146,6 +145,10 @@ void WebauthnOfferDialogViewImpl::Hide() {
void WebauthnOfferDialogViewImpl::RefreshContent() { void WebauthnOfferDialogViewImpl::RefreshContent() {
sheet_view_->ReInitChildViews(); sheet_view_->ReInitChildViews();
sheet_view_->InvalidateLayout(); sheet_view_->InvalidateLayout();
DialogDelegate::set_button_label(ui::DIALOG_BUTTON_OK,
model_->GetAcceptButtonLabel());
DialogDelegate::set_button_label(ui::DIALOG_BUTTON_CANCEL,
model_->GetCancelButtonLabel());
DialogModelChanged(); DialogModelChanged();
Layout(); Layout();
......
...@@ -40,7 +40,6 @@ class WebauthnOfferDialogViewImpl : public WebauthnOfferDialogView, ...@@ -40,7 +40,6 @@ class WebauthnOfferDialogViewImpl : public WebauthnOfferDialogView,
bool Cancel() override; bool Cancel() override;
bool Close() override; bool Close() override;
int GetDialogButtons() const override; int GetDialogButtons() const override;
base::string16 GetDialogButtonLabel(ui::DialogButton button) const override;
bool IsDialogButtonEnabled(ui::DialogButton button) const override; bool IsDialogButtonEnabled(ui::DialogButton button) const override;
ui::ModalType GetModalType() const override; ui::ModalType GetModalType() const override;
base::string16 GetWindowTitle() const override; base::string16 GetWindowTitle() const override;
......
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