Provide hook to abort dialog cancellation, disable cancellation during install.

Provide hook to abort dialog cancellation, disable cancellation during install.


BUG=148527
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=158644

[groby-ooo-7-16, 2012-09-22 00:51:03]

sky: Please review. 

We needed a way to prevent a dialog from closing. If it has Cancel or OK
buttons, overriding Cancel()/Accept() on the delegate works. I've extended this
to honor Cancel() in the no-button case too. 

The default behavior with non-overridden Cancel() is to always close on
close/pressing ESC, so no change should occur for that.

[sky, 2012-09-24 13:50:42]

http://codereview.chromium.org/10965052/diff/1/ui/views/window/dialog_client_...
File ui/views/window/dialog_client_view.cc (right):

http://codereview.chromium.org/10965052/diff/1/ui/views/window/dialog_client_...
ui/views/window/dialog_client_view.cc:266: else if (buttons ==
ui::DIALOG_BUTTON_NONE)
combine this with 262.
Also, update the description in DialogDelegate::Cancel(). And, did you look at
existing subclasses to make sure this isn't going to cause problems?

[groby-ooo-7-16, 2012-09-25 17:27:11]

http://codereview.chromium.org/10965052/diff/1/ui/views/window/dialog_client_...
File ui/views/window/dialog_client_view.cc (right):

http://codereview.chromium.org/10965052/diff/1/ui/views/window/dialog_client_...
ui/views/window/dialog_client_view.cc:266: else if (buttons ==
ui::DIALOG_BUTTON_NONE)
On 2012/09/24 13:50:42, sky wrote:
> combine this with 262.
> Also, update the description in DialogDelegate::Cancel(). And, did you look at
> existing subclasses to make sure this isn't going to cause problems?

Done.

Checked existing dialogs, and three use DIALOG_BUTTON_NONE (besides
WebIntentPicker):
AboutIPCDialog
TaskManagerView
IdleLogoutDialogView

None of these override Cancel(), so default behavior is observed -> close is set
to true. Same as it was before the change.

[sky, 2012-09-25 18:25:47]

LGTM

[commit-bot: I haz the power, 2012-09-25 18:26:54]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/groby@chromium.org/10965052/6001

[commit-bot: I haz the power, 2012-09-25 20:41:36]

Change committed as 158644