Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1)

Issue 339094: Set active_dialog_ before showing it, since showing a dialog can sometimes... (Closed)

Created:
11 years, 1 month ago by Pam (message me for reviews)
Modified:
9 years, 5 months ago
Reviewers:
jcampan
CC:
chromium-reviews_googlegroups.com, ben+cc_chromium.org
Visibility:
Public.

Description

Set active_dialog_ before showing it, since showing a dialog can sometimes actually move to the next one in the queue instead. BUG=26398 TEST=Make sure alert/confirm boxes work properly. Make sure a background tab that shows a (delayed) alert box works. Same with a background browser. Especially make sure http://crbug.com/10699 doesn't recur.

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+3 lines, -1 line) Patch
M chrome/browser/app_modal_dialog_queue.cc View 1 chunk +3 lines, -1 line 0 comments Download

Messages

Total messages: 2 (0 generated)
Pam (message me for reviews)
I wasn't able to tell from the history of this file whether moving active_dialog_ after ...
11 years, 1 month ago (2009-10-31 02:48:19 UTC) #1
jcampan
11 years, 1 month ago (2009-11-02 19:51:15 UTC) #2
I sadly don't recall why I moved the setting of active_dialog_ after the
ShowModalDialog() call.
I guess my assumption was that the case mentioned in the old comment (one modal
dialog showing on top of another one) could not happen, but I can definitely see
that happening with delayed tasks.
Anyway, LGTM.

Powered by Google App Engine
This is Rietveld 408576698