Skip to content

Commit

Permalink
Fix NULL pointer dereferncing in AccountChooserDialogView.
Browse files Browse the repository at this point in the history
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 1fde9e5)

Bug: 792731
Change-Id: Ia177aaa8ace746164ff363ecac94761c3f8f3dc6
Reviewed-on: https://chromium-review.googlesource.com/951002
Commit-Queue: Vasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: Vaclav Brozek <vabr@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#541100}
Reviewed-on: https://chromium-review.googlesource.com/955562
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/branch-heads/3359@{#93}
Cr-Branched-From: 66afc5e-refs/heads/master@{#540276}
  • Loading branch information
Vasilii Sukhanov committed Mar 8, 2018
1 parent 0bbd673 commit 45c89b0
Showing 1 changed file with 14 additions and 5 deletions.
19 changes: 14 additions & 5 deletions chrome/browser/ui/views/passwords/account_chooser_dialog_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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<CredentialsItemView*>(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() {
Expand Down

0 comments on commit 45c89b0

Please sign in to comment.