Commit 44d63178 authored by Evan Stade's avatar Evan Stade Committed by Commit Bot

Tweaks to app modal dialog shutdown.

Make sure that an invalidated dialog doesn't call
AppModalDialogQueue::ShowNextDialog() during shutdown. This could
potentially cause issues as it causes ShowNextDialog to be reentrant,
which doesn't seem intentional.

Also remove an unnecessary check: the active dialog is not in the dialog
queue so we don't need to check the queue against the active dialog when
hiding a batch of dialogs.

Bug: 1060986
Change-Id: I962a5cf14c3f5b67d287de61464b604dbe96aaf7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2295822
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: default avatarAvi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#789242}
parent 5e6ed805
...@@ -78,7 +78,6 @@ AppModalDialogController::AppModalDialogController( ...@@ -78,7 +78,6 @@ AppModalDialogController::AppModalDialogController(
bool is_reload, bool is_reload,
content::JavaScriptDialogManager::DialogClosedCallback callback) content::JavaScriptDialogManager::DialogClosedCallback callback)
: title_(title), : title_(title),
completed_(false),
valid_(true), valid_(true),
view_(nullptr), view_(nullptr),
web_contents_(web_contents), web_contents_(web_contents),
...@@ -114,9 +113,13 @@ void AppModalDialogController::CloseModalDialog() { ...@@ -114,9 +113,13 @@ void AppModalDialogController::CloseModalDialog() {
} }
void AppModalDialogController::CompleteDialog() { void AppModalDialogController::CompleteDialog() {
if (!completed_) { // If |view_| is non-null, then |this| is the active dialog and the next one
completed_ = true; // should be shown. Otherwise, |this| was never shown.
if (view_) {
view_ = nullptr;
AppModalDialogQueue::GetInstance()->ShowNextDialog(); AppModalDialogQueue::GetInstance()->ShowNextDialog();
} else {
DCHECK(!valid_);
} }
} }
......
...@@ -107,14 +107,12 @@ class AppModalDialogController { ...@@ -107,14 +107,12 @@ class AppModalDialogController {
// The title of the dialog. // The title of the dialog.
const base::string16 title_; const base::string16 title_;
// // True if CompleteDialog was called.
bool completed_;
// False if the dialog should no longer be shown, e.g. because the underlying // False if the dialog should no longer be shown, e.g. because the underlying
// tab navigated away while the dialog was queued. // tab navigated away while the dialog was queued.
bool valid_; bool valid_;
// // The toolkit-specific implementation of the app modal dialog box. // The toolkit-specific implementation of the app modal dialog box. When
// non-null, |view_| owns |this|.
AppModalDialogView* view_; AppModalDialogView* view_;
// The WebContents that opened this dialog. // The WebContents that opened this dialog.
......
...@@ -278,15 +278,11 @@ bool AppModalDialogManager::HandleJavaScriptDialog( ...@@ -278,15 +278,11 @@ bool AppModalDialogManager::HandleJavaScriptDialog(
void AppModalDialogManager::CancelDialogs(content::WebContents* web_contents, void AppModalDialogManager::CancelDialogs(content::WebContents* web_contents,
bool reset_state) { bool reset_state) {
AppModalDialogQueue* queue = AppModalDialogQueue::GetInstance(); AppModalDialogQueue* queue = AppModalDialogQueue::GetInstance();
AppModalDialogController* active_dialog = queue->active_dialog();
for (auto* dialog : *queue) { for (auto* dialog : *queue) {
// Invalidating the active dialog might trigger showing a not-yet
// invalidated dialog, so invalidate the active dialog last.
if (dialog == active_dialog)
continue;
if (dialog->web_contents() == web_contents) if (dialog->web_contents() == web_contents)
dialog->Invalidate(); dialog->Invalidate();
} }
AppModalDialogController* active_dialog = queue->active_dialog();
if (active_dialog && active_dialog->web_contents() == web_contents) if (active_dialog && active_dialog->web_contents() == web_contents)
active_dialog->Invalidate(); active_dialog->Invalidate();
......
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