From 45c89b0bb3d3be292b5b50ce5dc08f86e1d7c514 Mon Sep 17 00:00:00 2001 From: Vasilii Sukhanov Date: Thu, 8 Mar 2018 13:30:41 +0000 Subject: [PATCH] Fix NULL pointer dereferncing in AccountChooserDialogView. The fix is for 3 different crashes. - AccountChooserDialogView::ControllerGone can trigger some accessibility events. The framework needs the title of the dialog. As a result the controller is dereferenced. - AccountChooserDialogView::ButtonPressed and AccountChooserDialogView::StyledLabelLinkClicked. On Mac those events may be triggered when the dialog already disappeared. Therefore, a NULL-check is required before pinging the controller. TBR=vasilii@chromium.org (cherry picked from commit 1fde9e58756272e39cdfc3eb27d3cee034ef1eaf) Bug: 792731 Change-Id: Ia177aaa8ace746164ff363ecac94761c3f8f3dc6 Reviewed-on: https://chromium-review.googlesource.com/951002 Commit-Queue: Vasilii Sukhanov Reviewed-by: Vaclav Brozek Cr-Original-Commit-Position: refs/heads/master@{#541100} Reviewed-on: https://chromium-review.googlesource.com/955562 Reviewed-by: Vasilii Sukhanov Cr-Commit-Position: refs/branch-heads/3359@{#93} Cr-Branched-From: 66afc5e5d10127546cc4b98b9117aff588b5e66b-refs/heads/master@{#540276} --- .../passwords/account_chooser_dialog_view.cc | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/chrome/browser/ui/views/passwords/account_chooser_dialog_view.cc b/chrome/browser/ui/views/passwords/account_chooser_dialog_view.cc index 7b6120c988e2..f017f2744a21 100644 --- a/chrome/browser/ui/views/passwords/account_chooser_dialog_view.cc +++ b/chrome/browser/ui/views/passwords/account_chooser_dialog_view.cc @@ -109,8 +109,10 @@ void AccountChooserDialogView::ShowAccountChooser() { } void AccountChooserDialogView::ControllerGone() { - controller_ = nullptr; + // During Widget::Close() phase some accessibility event may occur. Thus, + // |controller_| should be kept around. GetWidget()->Close(); + controller_ = nullptr; } ui::ModalType AccountChooserDialogView::GetModalType() const { @@ -170,15 +172,22 @@ base::string16 AccountChooserDialogView::GetDialogButtonLabel( void AccountChooserDialogView::StyledLabelLinkClicked(views::StyledLabel* label, const gfx::Range& range, int event_flags) { - controller_->OnSmartLockLinkClicked(); + // On Mac the button click event may be dispatched after the dialog was + // hidden. Thus, the controller can be NULL. + if (controller_) + controller_->OnSmartLockLinkClicked(); } void AccountChooserDialogView::ButtonPressed(views::Button* sender, const ui::Event& event) { CredentialsItemView* view = static_cast(sender); - controller_->OnChooseCredentials( - *view->form(), - password_manager::CredentialType::CREDENTIAL_TYPE_PASSWORD); + // On Mac the button click event may be dispatched after the dialog was + // hidden. Thus, the controller can be NULL. + if (controller_) { + controller_->OnChooseCredentials( + *view->form(), + password_manager::CredentialType::CREDENTIAL_TYPE_PASSWORD); + } } void AccountChooserDialogView::InitWindow() {