It turns out the Cleanup() method in JsModalDialog is not

It turns out the Cleanup() method in JsModalDialog is not
needed. The check in it was reversed, causing it to never
do anything, except in the case where the delegate had been
deleted, in which case it would crash.

The thing it was trying to do is already being done elsewhere
in the case of OnAccept() and OnCancel(). That just leaves
OnClose().

There are other things in here that really need cleanup, but
I will do those separately.

BUG=63732
TEST=See bug

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

[Aaron Boodman, 2010-12-02 01:53:01]

Review is to zelidrag since he was the last person to look at this code
seriously.

[Matt Perry, 2010-12-03 00:39:21]

LGTM, with optional suggestion

http://codereview.chromium.org/5548001/diff/1/chrome/browser/ui/app_modal_dia...
File chrome/browser/ui/app_modal_dialogs/js_modal_dialog.cc (right):

http://codereview.chromium.org/5548001/diff/1/chrome/browser/ui/app_modal_dia...
chrome/browser/ui/app_modal_dialogs/js_modal_dialog.cc:124: if
(!skip_this_dialog_) {
maybe this block is what should go into Cleanup.

[Matt Perry, 2010-12-03 00:58:52]

LGTM

http://codereview.chromium.org/5548001/diff/7001/chrome/browser/ui/app_modal_...
File chrome/browser/ui/app_modal_dialogs/js_modal_dialog.h (right):

http://codereview.chromium.org/5548001/diff/7001/chrome/browser/ui/app_modal_...
chrome/browser/ui/app_modal_dialogs/js_modal_dialog.h:91: void
UpdateDelegate(bool success, const std::wstring& prompt_text,
one more nit: maybe NotifyDelegate would be more accurate

[zel, 2010-12-03 01:29:55]

LGTM