Refactor AppModalDialogQueue and move JS Alert boxes into a MVC.

Refactor AppModalDialogQueue and move JS Alert boxes into a MVC.

JavascriptMessageBoxHandler (handles alert, confirm, prompt, and onbeforeunload) was a views class.  This change converts it into an MVC so we can port to linux/mac.

AppModalDialog is the model+controller, JavascriptMessageBoxDialog is the windows specific view.

The onbeforeunload dialog (JavascriptBeforeUnloadHandler) was a subclass of JavascriptMessageBoxHandler that had a different title and button text.  I merged this class into JavascriptMessageBoxHandler by passing a bool to handle the custom button text.

[Ben Goodger (Google), 2009-04-07 03:09:08]

OK

http://codereview.chromium.org/63033/diff/1002/70
File chrome/browser/app_modal_dialog.cc (right):

http://codereview.chromium.org/63033/diff/1002/70#newcode38
Line 38: bool web_contents_gone = false;
This variable name is slightly confusing. In one of your cases below the web
contents isn't actually gone, it's just been navigated somewhere else. Maybe
should_close_dialog or source_page_gone or something?

http://codereview.chromium.org/63033/diff/1002/70#newcode87
Line 87: if (web_contents()) {
nit: sometimes you use web_contents_ and sometimes you use web_contents()

http://codereview.chromium.org/63033/diff/1002/82
File chrome/browser/jsmessage_box_handler_win.cc (right):

http://codereview.chromium.org/63033/diff/1002/82#newcode79
Line 79: if (message_box_view_->IsCheckBoxSelected() && web_contents())
This should probably only be done in Accept? Changes to dialogs aren't usually
applied when the dialog is canceled.

[tony, 2009-04-07 18:55:02]

On 2009/04/07 03:09:08, Ben Goodger wrote:
> OK
> 
> http://codereview.chromium.org/63033/diff/1002/70
> File chrome/browser/app_modal_dialog.cc (right):
> 
> http://codereview.chromium.org/63033/diff/1002/70#newcode38
> Line 38: bool web_contents_gone = false;
> This variable name is slightly confusing. In one of your cases below the web
> contents isn't actually gone, it's just been navigated somewhere else. Maybe
> should_close_dialog or source_page_gone or something?

This is ACW code.  I've removed the bool and just set web_contents_ to NULL.

> http://codereview.chromium.org/63033/diff/1002/70#newcode87
> Line 87: if (web_contents()) {
> nit: sometimes you use web_contents_ and sometimes you use web_contents()

Fixed to always use web_contents_

> http://codereview.chromium.org/63033/diff/1002/82
> File chrome/browser/jsmessage_box_handler_win.cc (right):
> 
> http://codereview.chromium.org/63033/diff/1002/82#newcode79
> Line 79: if (message_box_view_->IsCheckBoxSelected() && web_contents())
> This should probably only be done in Accept? Changes to dialogs aren't usually
> applied when the dialog is canceled.

Good point.  I've moved this logic into OnAccept in the controller so we can
share this code on all platforms.

I also renamed JavascriptMessageHandlerWin to JavascriptMessageBoxDialog and
moved it into browser/views/jsmessage_box_dialog.{h,cc}.