Commit fe2dcf02 authored by siyua's avatar siyua Committed by Commit Bot

[Autofill Auth UI](3) Add WebAuthnOfferDialog error state.

So when fetching authentication challenge fails, update the pending
WebAuthn offer dialog to the error state and show new contents.

Mock: https://docs.google.com/presentation/d/16gfVy4EJ0hPibnOWZ8pqMF09Uy6uCWW2mCd1et9ILDw/edit?ts=5d670e48#slide=id.g33e66b3a17_12_0

Uploaded implementation screenshot in bug comment 11

1) Added function in CCFA to update dialog if error-ed.
2) Added more DialogState enum in model.
3) Changed the way how MVC works. Previously the controller talked to
view directly. Now the controller updates model, and view, as an
observer, observing any state change and updates itself.

Bug: 991037
Change-Id: Ia5159d64f411d51bdcab6d3d1b72638fe014de40
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1785627Reviewed-by: default avatarJared Saul <jsaul@google.com>
Reviewed-by: default avatarEvan Stade <estade@chromium.org>
Commit-Queue: Siyu An <siyua@chromium.org>
Cr-Commit-Position: refs/heads/master@{#695378}
parent 0dc57bde
......@@ -1990,6 +1990,7 @@ jumbo_split_static_library("ui") {
"autofill/payments/webauthn_offer_dialog_controller_impl.h",
"autofill/payments/webauthn_offer_dialog_model.cc",
"autofill/payments/webauthn_offer_dialog_model.h",
"autofill/payments/webauthn_offer_dialog_model_observer.h",
"autofill/payments/webauthn_offer_dialog_view.h",
"frame/window_frame_util.cc",
"frame/window_frame_util.h",
......
......@@ -289,6 +289,16 @@ bool ChromeAutofillClient::CloseWebauthnOfferDialog() {
return false;
}
void ChromeAutofillClient::UpdateWebauthnOfferDialogWithError() {
#if !defined(OS_ANDROID)
WebauthnOfferDialogControllerImpl* controller =
autofill::WebauthnOfferDialogControllerImpl::FromWebContents(
web_contents());
if (controller)
controller->UpdateDialogWithError();
#endif
}
void ChromeAutofillClient::ConfirmSaveAutofillProfile(
const AutofillProfile& profile,
base::OnceClosure callback) {
......
......@@ -84,6 +84,7 @@ class ChromeAutofillClient
MigrationDeleteCardCallback delete_local_card_callback) override;
void ShowWebauthnOfferDialog(WebauthnOfferDialogCallback callback) override;
bool CloseWebauthnOfferDialog() override;
void UpdateWebauthnOfferDialogWithError() override;
void ConfirmSaveAutofillProfile(const AutofillProfile& profile,
base::OnceClosure callback) override;
void ConfirmSaveCreditCardLocally(
......
......@@ -4,6 +4,7 @@
#include "chrome/browser/ui/autofill/payments/webauthn_offer_dialog_controller_impl.h"
#include "chrome/browser/ui/autofill/payments/webauthn_offer_dialog_model.h"
#include "chrome/browser/ui/autofill/payments/webauthn_offer_dialog_view.h"
namespace autofill {
......@@ -17,33 +18,28 @@ WebauthnOfferDialogControllerImpl::~WebauthnOfferDialogControllerImpl() =
void WebauthnOfferDialogControllerImpl::ShowOfferDialog(
AutofillClient::WebauthnOfferDialogCallback callback) {
DCHECK(!dialog_view_);
DCHECK(!dialog_model_);
offer_dialog_callback_ = std::move(callback);
dialog_view_ = WebauthnOfferDialogView::CreateAndShow(this);
dialog_model_ = WebauthnOfferDialogView::CreateAndShow(this);
}
bool WebauthnOfferDialogControllerImpl::CloseDialog() {
if (!dialog_view_)
if (!dialog_model_)
return false;
dialog_view_->Hide();
dialog_model_->SetDialogState(
WebauthnOfferDialogModel::DialogState::kInactive);
return true;
}
void WebauthnOfferDialogControllerImpl::OnOkButtonClicked() {
DCHECK(offer_dialog_callback_);
offer_dialog_callback_.Run(/*did_accept=*/true);
dialog_view_->RefreshContent();
}
void WebauthnOfferDialogControllerImpl::OnCancelButtonClicked() {
DCHECK(offer_dialog_callback_);
offer_dialog_callback_.Run(/*did_accept=*/false);
void WebauthnOfferDialogControllerImpl::UpdateDialogWithError() {
dialog_model_->SetDialogState(WebauthnOfferDialogModel::DialogState::kError);
offer_dialog_callback_.Reset();
}
void WebauthnOfferDialogControllerImpl::OnDialogClosed() {
dialog_view_ = nullptr;
dialog_model_ = nullptr;
offer_dialog_callback_.Reset();
}
......@@ -51,6 +47,18 @@ content::WebContents* WebauthnOfferDialogControllerImpl::GetWebContents() {
return web_contents();
}
void WebauthnOfferDialogControllerImpl::OnOkButtonClicked() {
DCHECK(offer_dialog_callback_);
offer_dialog_callback_.Run(/*did_accept=*/true);
dialog_model_->SetDialogState(
WebauthnOfferDialogModel::DialogState::kPending);
}
void WebauthnOfferDialogControllerImpl::OnCancelButtonClicked() {
DCHECK(offer_dialog_callback_);
offer_dialog_callback_.Run(/*did_accept=*/false);
}
WEB_CONTENTS_USER_DATA_KEY_IMPL(WebauthnOfferDialogControllerImpl)
} // namespace autofill
......@@ -13,7 +13,7 @@
namespace autofill {
class WebauthnOfferDialogView;
class WebauthnOfferDialogModel;
// Implementation of the per-tab controller to control the
// WebauthnOfferDialogView. Lazily initialized when used.
......@@ -26,6 +26,7 @@ class WebauthnOfferDialogControllerImpl
void ShowOfferDialog(AutofillClient::WebauthnOfferDialogCallback callback);
bool CloseDialog();
void UpdateDialogWithError();
// WebauthnOfferDialogController:
void OnOkButtonClicked() override;
......@@ -45,8 +46,7 @@ class WebauthnOfferDialogControllerImpl
// clicked, the dialog stays and the cancel button is still clickable.
AutofillClient::WebauthnOfferDialogCallback offer_dialog_callback_;
// Reference to the dialog, which is owned by the view hierarchy.
WebauthnOfferDialogView* dialog_view_ = nullptr;
WebauthnOfferDialogModel* dialog_model_ = nullptr;
WEB_CONTENTS_USER_DATA_KEY_DECL();
......
......@@ -5,8 +5,10 @@
#include "chrome/browser/ui/autofill/payments/webauthn_offer_dialog_model.h"
#include "chrome/app/vector_icons/vector_icons.h"
#include "chrome/browser/ui/autofill/payments/webauthn_offer_dialog_model_observer.h"
#include "components/strings/grit/components_strings.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/gfx/paint_vector_icon.h"
namespace autofill {
......@@ -16,8 +18,24 @@ WebauthnOfferDialogModel::WebauthnOfferDialogModel() {
WebauthnOfferDialogModel::~WebauthnOfferDialogModel() = default;
void WebauthnOfferDialogModel::SetDialogState(DialogState state) {
state_ = state;
for (WebauthnOfferDialogModelObserver& observer : observers_)
observer.OnDialogStateChanged();
}
void WebauthnOfferDialogModel::AddObserver(
WebauthnOfferDialogModelObserver* observer) {
observers_.AddObserver(observer);
}
void WebauthnOfferDialogModel::RemoveObserver(
WebauthnOfferDialogModelObserver* observer) {
observers_.RemoveObserver(observer);
}
bool WebauthnOfferDialogModel::IsActivityIndicatorVisible() const {
return state_ == kPending;
return state_ == DialogState::kPending;
}
bool WebauthnOfferDialogModel::IsBackButtonVisible() const {
......@@ -29,16 +47,28 @@ bool WebauthnOfferDialogModel::IsCancelButtonVisible() const {
}
base::string16 WebauthnOfferDialogModel::GetCancelButtonLabel() const {
return l10n_util::GetStringUTF16(
IDS_AUTOFILL_WEBAUTHN_OPT_IN_DIALOG_CANCEL_BUTTON_LABEL);
switch (state_) {
case DialogState::kOffer:
case DialogState::kPending:
return l10n_util::GetStringUTF16(
IDS_AUTOFILL_WEBAUTHN_OPT_IN_DIALOG_CANCEL_BUTTON_LABEL);
case DialogState::kError:
return l10n_util::GetStringUTF16(
IDS_AUTOFILL_WEBAUTHN_OPT_IN_DIALOG_CANCEL_BUTTON_LABEL_ERROR);
case DialogState::kInactive:
case DialogState::kUnknown:
break;
}
NOTREACHED();
return base::string16();
}
bool WebauthnOfferDialogModel::IsAcceptButtonVisible() const {
return true;
return state_ == DialogState::kOffer || state_ == DialogState::kPending;
}
bool WebauthnOfferDialogModel::IsAcceptButtonEnabled() const {
return state_ != kPending;
return state_ != DialogState::kPending;
}
base::string16 WebauthnOfferDialogModel::GetAcceptButtonLabel() const {
......@@ -48,18 +78,55 @@ base::string16 WebauthnOfferDialogModel::GetAcceptButtonLabel() const {
const gfx::VectorIcon& WebauthnOfferDialogModel::GetStepIllustration(
ImageColorScheme color_scheme) const {
return color_scheme == ImageColorScheme::kDark
? kWebauthnOfferDialogHeaderDarkIcon
: kWebauthnOfferDialogHeaderIcon;
switch (state_) {
case DialogState::kOffer:
case DialogState::kPending:
return color_scheme == ImageColorScheme::kDark
? kWebauthnOfferDialogHeaderDarkIcon
: kWebauthnOfferDialogHeaderIcon;
case DialogState::kError:
return color_scheme == ImageColorScheme::kDark ? kWebauthnErrorDarkIcon
: kWebauthnErrorIcon;
case DialogState::kInactive:
case DialogState::kUnknown:
break;
}
NOTREACHED();
return gfx::kNoneIcon;
}
base::string16 WebauthnOfferDialogModel::GetStepTitle() const {
return l10n_util::GetStringUTF16(IDS_AUTOFILL_WEBAUTHN_OPT_IN_DIALOG_TITLE);
switch (state_) {
case DialogState::kOffer:
case DialogState::kPending:
return l10n_util::GetStringUTF16(
IDS_AUTOFILL_WEBAUTHN_OPT_IN_DIALOG_TITLE);
case DialogState::kError:
return l10n_util::GetStringUTF16(
IDS_AUTOFILL_WEBAUTHN_OPT_IN_DIALOG_TITLE_ERROR);
case DialogState::kInactive:
case DialogState::kUnknown:
break;
}
NOTREACHED();
return base::string16();
}
base::string16 WebauthnOfferDialogModel::GetStepDescription() const {
return l10n_util::GetStringUTF16(
IDS_AUTOFILL_WEBAUTHN_OPT_IN_DIALOG_INSTRUCTION);
switch (state_) {
case DialogState::kOffer:
case DialogState::kPending:
return l10n_util::GetStringUTF16(
IDS_AUTOFILL_WEBAUTHN_OPT_IN_DIALOG_INSTRUCTION);
case DialogState::kError:
return l10n_util::GetStringUTF16(
IDS_AUTOFILL_WEBAUTHN_OPT_IN_DIALOG_INSTRUCTION_ERROR);
case DialogState::kInactive:
case DialogState::kUnknown:
break;
}
NOTREACHED();
return base::string16();
}
base::Optional<base::string16>
......@@ -71,12 +138,4 @@ ui::MenuModel* WebauthnOfferDialogModel::GetOtherTransportsMenuModel() {
return nullptr;
}
void WebauthnOfferDialogModel::OnBack() {}
void WebauthnOfferDialogModel::OnAccept() {
state_ = kPending;
}
void WebauthnOfferDialogModel::OnCancel() {}
} // namespace autofill
......@@ -6,26 +6,40 @@
#define CHROME_BROWSER_UI_AUTOFILL_PAYMENTS_WEBAUTHN_OFFER_DIALOG_MODEL_H_
#include "base/macros.h"
#include "base/observer_list.h"
#include "chrome/browser/ui/webauthn/authenticator_request_sheet_model.h"
#include "ui/gfx/vector_icon_types.h"
namespace autofill {
class WebauthnOfferDialogModelObserver;
// The model for WebauthnOfferDialogView determining what content is shown.
// Owned by the AuthenticatorRequestSheetView.
class WebauthnOfferDialogModel : public AuthenticatorRequestSheetModel {
public:
enum DialogState {
kUnknown,
// The dialog is about to be closed automatically. This happens only after
// authentication challenge is successfully fetched.
kInactive,
// The option of using platform authenticator is being offered.
kOffer,
// Offer was accepted, fetching authentication challenge.
kPending,
// TODO(crbug.com/991037): Add error state.
// Fetching authentication challenge failed.
kError,
};
WebauthnOfferDialogModel();
~WebauthnOfferDialogModel() override;
void SetDialogState(DialogState state);
DialogState dialog_state() { return state_; }
void AddObserver(WebauthnOfferDialogModelObserver* observer);
void RemoveObserver(WebauthnOfferDialogModelObserver* observer);
// AuthenticatorRequestSheetModel:
bool IsActivityIndicatorVisible() const override;
bool IsBackButtonVisible() const override;
......@@ -40,12 +54,15 @@ class WebauthnOfferDialogModel : public AuthenticatorRequestSheetModel {
base::string16 GetStepDescription() const override;
base::Optional<base::string16> GetAdditionalDescription() const override;
ui::MenuModel* GetOtherTransportsMenuModel() override;
void OnBack() override;
void OnAccept() override;
void OnCancel() override;
// Event handling is handed over to the controller.
void OnBack() override {}
void OnAccept() override {}
void OnCancel() override {}
private:
DialogState state_ = kUnknown;
DialogState state_ = DialogState::kUnknown;
base::ObserverList<WebauthnOfferDialogModelObserver> observers_;
DISALLOW_COPY_AND_ASSIGN(WebauthnOfferDialogModel);
};
......
// Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef CHROME_BROWSER_UI_AUTOFILL_PAYMENTS_WEBAUTHN_OFFER_DIALOG_MODEL_OBSERVER_H_
#define CHROME_BROWSER_UI_AUTOFILL_PAYMENTS_WEBAUTHN_OFFER_DIALOG_MODEL_OBSERVER_H_
#include "base/observer_list_types.h"
namespace autofill {
// The observer for WebauthnOfferDialogModel.
class WebauthnOfferDialogModelObserver : public base::CheckedObserver {
public:
virtual void OnDialogStateChanged() = 0;
};
} // namespace autofill
#endif // CHROME_BROWSER_UI_AUTOFILL_PAYMENTS_WEBAUTHN_OFFER_DIALOG_MODEL_OBSERVER_H_
......@@ -8,18 +8,15 @@
namespace autofill {
class WebauthnOfferDialogController;
class WebauthnOfferDialogModel;
// An interface which displays the dialog to offer the option of using device's
// platform authenticator instead of CVC to verify the card in the future.
// The dialog to offer the option of using device's platform authenticator
// instead of CVC to verify the card in the future. Returns the reference to the
// model of the dialog.
class WebauthnOfferDialogView {
public:
static WebauthnOfferDialogView* CreateAndShow(
static WebauthnOfferDialogModel* CreateAndShow(
WebauthnOfferDialogController* controller);
virtual void Hide() = 0;
// Reinitializes the content in the dialog and resizes.
virtual void RefreshContent() = 0;
};
} // namespace autofill
......
......@@ -10,6 +10,9 @@
#include "chrome/browser/ui/views/webauthn/authenticator_request_sheet_view.h"
#include "chrome/browser/ui/views/webauthn/sheet_view_factory.h"
#include "components/constrained_window/constrained_window_views.h"
#include "components/web_modal/web_contents_modal_dialog_host.h"
#include "components/web_modal/web_contents_modal_dialog_manager.h"
#include "components/web_modal/web_contents_modal_dialog_manager_delegate.h"
#include "ui/views/controls/button/label_button.h"
#include "ui/views/layout/fill_layout.h"
#include "ui/views/window/dialog_client_view.h"
......@@ -23,35 +26,39 @@ WebauthnOfferDialogViewImpl::WebauthnOfferDialogViewImpl(
std::unique_ptr<WebauthnOfferDialogModel> model =
std::make_unique<WebauthnOfferDialogModel>();
model_ = model.get();
model_->AddObserver(this);
sheet_view_ =
AddChildView(CreateSheetViewForAutofillWebAuthn(std::move(model)));
sheet_view_->ReInitChildViews();
}
WebauthnOfferDialogViewImpl::~WebauthnOfferDialogViewImpl() = default;
WebauthnOfferDialogViewImpl::~WebauthnOfferDialogViewImpl() {
model_->RemoveObserver(this);
}
// static
WebauthnOfferDialogView* WebauthnOfferDialogView::CreateAndShow(
WebauthnOfferDialogModel* WebauthnOfferDialogView::CreateAndShow(
WebauthnOfferDialogController* controller) {
WebauthnOfferDialogViewImpl* dialog =
new WebauthnOfferDialogViewImpl(controller);
constrained_window::ShowWebModalDialogViews(dialog,
controller->GetWebContents());
return dialog;
}
void WebauthnOfferDialogViewImpl::Hide() {
GetWidget()->Close();
}
// TODO(crbug.com/991037): Need to resize the dialog after reinitializing the
// dialog to error dialog since the string content in the error dialog is
// different.
void WebauthnOfferDialogViewImpl::RefreshContent() {
sheet_view_->ReInitChildViews();
sheet_view_->InvalidateLayout();
Layout();
DialogModelChanged();
return dialog->model();
}
void WebauthnOfferDialogViewImpl::OnDialogStateChanged() {
switch (model_->dialog_state()) {
case WebauthnOfferDialogModel::DialogState::kInactive:
Hide();
break;
case WebauthnOfferDialogModel::DialogState::kPending:
case WebauthnOfferDialogModel::DialogState::kError:
RefreshContent();
break;
case WebauthnOfferDialogModel::DialogState::kUnknown:
case WebauthnOfferDialogModel::DialogState::kOffer:
NOTREACHED();
}
}
gfx::Size WebauthnOfferDialogViewImpl::CalculatePreferredSize() const {
......@@ -61,14 +68,19 @@ gfx::Size WebauthnOfferDialogViewImpl::CalculatePreferredSize() const {
}
bool WebauthnOfferDialogViewImpl::Accept() {
model_->OnAccept();
DCHECK_EQ(model_->dialog_state(),
WebauthnOfferDialogModel::DialogState::kOffer);
controller_->OnOkButtonClicked();
return false;
}
bool WebauthnOfferDialogViewImpl::Cancel() {
model_->OnCancel();
controller_->OnCancelButtonClicked();
if (model_->dialog_state() == WebauthnOfferDialogModel::DialogState::kOffer ||
model_->dialog_state() ==
WebauthnOfferDialogModel::DialogState::kPending) {
controller_->OnCancelButtonClicked();
}
return true;
}
......@@ -76,12 +88,12 @@ bool WebauthnOfferDialogViewImpl::Close() {
return true;
}
// TODO(crbug.com/991037): Fetching authentication challenge will send a request
// to Payments server, and it can fail sometimes. For the error case, the dialog
// should be updated and the OK button should not be shown.
int WebauthnOfferDialogViewImpl::GetDialogButtons() const {
DCHECK(model_->IsAcceptButtonVisible() && model_->IsCancelButtonVisible());
return ui::DIALOG_BUTTON_OK | ui::DIALOG_BUTTON_CANCEL;
// Cancel button is always visible but OK button depends on dialog state.
DCHECK(model_->IsCancelButtonVisible());
return model_->IsAcceptButtonVisible()
? ui::DIALOG_BUTTON_OK | ui::DIALOG_BUTTON_CANCEL
: ui::DIALOG_BUTTON_CANCEL;
}
base::string16 WebauthnOfferDialogViewImpl::GetDialogButtonLabel(
......@@ -119,4 +131,24 @@ void WebauthnOfferDialogViewImpl::WindowClosing() {
}
}
void WebauthnOfferDialogViewImpl::Hide() {
GetWidget()->Close();
}
void WebauthnOfferDialogViewImpl::RefreshContent() {
sheet_view_->ReInitChildViews();
sheet_view_->InvalidateLayout();
DialogModelChanged();
Layout();
// Update the dialog's size.
if (GetWidget() && controller_->GetWebContents()) {
constrained_window::UpdateWebContentsModalDialogPosition(
GetWidget(), web_modal::WebContentsModalDialogManager::FromWebContents(
controller_->GetWebContents())
->delegate()
->GetWebContentsModalDialogHost());
}
}
} // namespace autofill
......@@ -6,6 +6,7 @@
#define CHROME_BROWSER_UI_VIEWS_AUTOFILL_PAYMENTS_WEBAUTHN_OFFER_DIALOG_VIEW_IMPL_H_
#include "base/macros.h"
#include "chrome/browser/ui/autofill/payments/webauthn_offer_dialog_model_observer.h"
#include "chrome/browser/ui/autofill/payments/webauthn_offer_dialog_view.h"
#include "ui/views/window/dialog_delegate.h"
......@@ -20,15 +21,15 @@ class WebauthnOfferDialogModel;
// authenticator. It is shown automatically after card unmasked details are
// obtained and filled into the form.
class WebauthnOfferDialogViewImpl : public WebauthnOfferDialogView,
public WebauthnOfferDialogModelObserver,
public views::DialogDelegateView {
public:
explicit WebauthnOfferDialogViewImpl(
WebauthnOfferDialogController* controller);
~WebauthnOfferDialogViewImpl() override;
// WebauthnOfferDialogView:
void Hide() override;
void RefreshContent() override;
// WebauthnOfferDialogModelObserver:
void OnDialogStateChanged() override;
// views::DialogDelegateView:
gfx::Size CalculatePreferredSize() const override;
......@@ -44,7 +45,15 @@ class WebauthnOfferDialogViewImpl : public WebauthnOfferDialogView,
bool ShouldShowCloseButton() const override;
void WindowClosing() override;
WebauthnOfferDialogModel* model() { return model_; }
private:
// Closes the dialog.
void Hide();
// Re-inits dialog content and resizes.
void RefreshContent();
WebauthnOfferDialogController* controller_ = nullptr;
AuthenticatorRequestSheetView* sheet_view_ = nullptr;
......
......@@ -315,6 +315,10 @@ class AutofillClient : public RiskDataLoader {
// and has been closed. Implemented only on desktop.
virtual bool CloseWebauthnOfferDialog();
// Will update the WebAuthn offer dialog content to the error state.
// Implemented only on desktop.
virtual void UpdateWebauthnOfferDialogWithError() {}
// Runs |callback| if the |profile| should be imported as personal data.
virtual void ConfirmSaveAutofillProfile(const AutofillProfile& profile,
base::OnceClosure callback) = 0;
......
......@@ -246,6 +246,7 @@ void CreditCardFIDOAuthenticator::OnDidGetOptChangeResult(
// End the flow if the server responded with an error.
if (result != AutofillClient::PaymentsRpcResult::SUCCESS) {
current_flow_ = NONE_FLOW;
autofill_client_->UpdateWebauthnOfferDialogWithError();
return;
}
......
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