Convert MessageBoxFlags class into an enum.

Convert MessageBoxFlags class into an enum.

R=sky@chromium.org

[sky, 2011-11-12 00:11:55]

http://codereview.chromium.org/8536026/diff/1/ui/base/message_box_flags.h
File ui/base/message_box_flags.h (right):

http://codereview.chromium.org/8536026/diff/1/ui/base/message_box_flags.h#new...
ui/base/message_box_flags.h:36: MESSAGE_BOX_IS_CONFIRM_DIALOG =
MESSAGE_BOX_HAS_MESSAGE |
The first 5 are appropriate for an enum, but the remaining ones don't fit an
enum to well. I'm inclined to leave this file alone.

[tfarina, 2011-11-12 11:01:54]

http://codereview.chromium.org/8536026/diff/1/ui/base/message_box_flags.h
File ui/base/message_box_flags.h (right):

http://codereview.chromium.org/8536026/diff/1/ui/base/message_box_flags.h#new...
ui/base/message_box_flags.h:36: MESSAGE_BOX_IS_CONFIRM_DIALOG =
MESSAGE_BOX_HAS_MESSAGE |
On 2011/11/12 00:11:55, sky wrote:
> The first 5 are appropriate for an enum, but the remaining ones don't fit an
> enum to well. I'm inclined to leave this file alone.

I really want to get rid of this class, really! I'll try to come up with another
solution for the remaining ones. In the mean time, do you have any other
solution for them? Thanks!

[tfarina, 2011-11-12 12:40:21]

Some notes below from my analysis.

I think I'll send a separate patch for cleaning up the cases where we should be
using ui::MessageBoxFlags::kIsJavascriptAlert instead.

http://codereview.chromium.org/8536026/diff/1/chrome/browser/ui/views/externa...
File chrome/browser/ui/views/external_protocol_dialog.cc (right):

http://codereview.chromium.org/8536026/diff/1/chrome/browser/ui/views/externa...
chrome/browser/ui/views/external_protocol_dialog.cc:150:
ui::MESSAGE_BOX_IS_CONFIRM_DIALOG,
same comment about kIsConfirmMessageBox from repost_form_warning_view.cc

http://codereview.chromium.org/8536026/diff/1/chrome/browser/ui/views/repost_...
File chrome/browser/ui/views/repost_form_warning_view.cc (right):

http://codereview.chromium.org/8536026/diff/1/chrome/browser/ui/views/repost_...
chrome/browser/ui/views/repost_form_warning_view.cc:39:
ui::MESSAGE_BOX_IS_CONFIRM_DIALOG,
MessageBoxView does not check any of the flags passed in from
kIsConfirmMessageBox. It just cares about kAutoDetectAlignment and
kFlagHasPromptField. I think I'll make an enum into MessageBoxView for these two
flags instead.

With this I can kill kIsConfirmMessageBox, because it's only passed to
MessageBoxView, but actually not used by MessageBoxView.

http://codereview.chromium.org/8536026/diff/1/chrome/browser/ui/views/restart...
File chrome/browser/ui/views/restart_message_box.cc (right):

http://codereview.chromium.org/8536026/diff/1/chrome/browser/ui/views/restart...
chrome/browser/ui/views/restart_message_box.cc:65: ui::MESSAGE_BOX_HAS_MESSAGE |
ui::MESSAGE_BOX_HAS_OK_BUTTON,
this should be ui::MessageBoxFlags::kIsJavascriptAlert,
but again it just ignores the flags passed from it.

http://codereview.chromium.org/8536026/diff/1/chrome/browser/ui/views/simple_...
File chrome/browser/ui/views/simple_message_box_views.cc (right):

http://codereview.chromium.org/8536026/diff/1/chrome/browser/ui/views/simple_...
chrome/browser/ui/views/simple_message_box_views.cc:44: int dialog_flags =
ui::MESSAGE_BOX_HAS_MESSAGE |
this should be ui::MessageBoxFlags::kIsJavascriptAlert

http://codereview.chromium.org/8536026/diff/1/chrome/browser/ui/views/update_...
File chrome/browser/ui/views/update_recommended_message_box.cc (right):

http://codereview.chromium.org/8536026/diff/1/chrome/browser/ui/views/update_...
chrome/browser/ui/views/update_recommended_message_box.cc:100:
ui::MESSAGE_BOX_HAS_MESSAGE | ui::MESSAGE_BOX_HAS_OK_BUTTON,
this should be ui::MessageBoxFlags::kIsJavascriptAlert

http://codereview.chromium.org/8536026/diff/1/chrome/browser/ui/views/user_da...
File chrome/browser/ui/views/user_data_dir_dialog.cc (right):

http://codereview.chromium.org/8536026/diff/1/chrome/browser/ui/views/user_da...
chrome/browser/ui/views/user_data_dir_dialog.cc:33:
ui::MESSAGE_BOX_IS_CONFIRM_DIALOG,
same comment about kIsConfirmMessageBox from repost_form_warning_view.cc

[tfarina, 2011-11-12 12:56:25]

On 2011/11/12 12:40:21, tfarina wrote:
> Some notes below from my analysis.
> 
> I think I'll send a separate patch for cleaning up the cases where we should
be
> using ui::MessageBoxFlags::kIsJavascriptAlert instead.
> 
I splitted out that part into this CL http://codereview.chromium.org/8544004/

[tfarina, 2011-11-12 22:33:56]

http://codereview.chromium.org/8536026/diff/1/chrome/browser/ui/views/repost_...
File chrome/browser/ui/views/repost_form_warning_view.cc (right):

http://codereview.chromium.org/8536026/diff/1/chrome/browser/ui/views/repost_...
chrome/browser/ui/views/repost_form_warning_view.cc:39:
ui::MESSAGE_BOX_IS_CONFIRM_DIALOG,
On 2011/11/12 12:40:22, tfarina wrote:
> MessageBoxView does not check any of the flags passed in from
> kIsConfirmMessageBox. It just cares about kAutoDetectAlignment and
> kFlagHasPromptField. I think I'll make an enum into MessageBoxView for these
two
> flags instead.
> 
> With this I can kill kIsConfirmMessageBox, because it's only passed to
> MessageBoxView, but actually not used by MessageBoxView.

Scott, I think this patch http://codereview.chromium.org/8553001/ should give
you a better idea of what I have in mind.

The changes I think are most important are in:
simple_message_box_views.cc and js_modal_dialog_views.cc