Move JavaScriptDialogManager, JavascriptAppModalDialogViews to components/app_modal_dialogs

components/constrained_window/constrained_window_views.h

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef COMPONENTS_CONSTRAINED_WINDOW_CONSTRAINED_WINDOW_VIEWS_H_
 #define COMPONENTS_CONSTRAINED_WINDOW_CONSTRAINED_WINDOW_VIEWS_H_
 
 #include "base/memory/scoped_ptr.h"
 #include "ui/gfx/native_widget_types.h"
 
@@ skipping 30 matching lines @@
 
 // Calls CreateWebModalDialogViews, shows the dialog, and returns its widget.
 views::Widget* ShowWebModalDialogViews(
     views::WidgetDelegate* dialog,
     content::WebContents* initiator_web_contents);
 
 // Create a widget for |dialog| that is modal to |web_contents|.
 views::Widget* CreateWebModalDialogViews(views::WidgetDelegate* dialog,
                                          content::WebContents* web_contents);
 
-// Create a widget for |dialog| that is modal to the browser window |parent|.
-// This places the dialog appropriately if |parent| is a valid browser window.
-views::Widget* CreateBrowserModalDialogViews(views::DialogDelegate* dialog,
-                                             gfx::NativeWindow parent);
+// Create a widget for |dialog| that is modal to the |parent| window.
+// This places the dialog appropriately if |parent| has an associated
+// ModalDialogHost.
+views::Widget* CreateWindowModalDialogViews(views::DialogDelegate* dialog,

[Avi (use Gerrit), 2014/11/04 20:46:41]

You talk about this in the CL description:
CreateBrowserModalDialogViews never had anything to do about the browser. It was
about being "browser-modal", or in other words, "app-modal". Your new name
implies that it's window-modal. Did you change the modality of the dialog? If
not, and if you don't like the old name because it has "browser" in it, pick a
new one that doesn't imply incorrect modality semantics.

(Note the use of "app modal" already in places like
"components/app_modal_dialogs".)

[oshima, 2014/11/04 23:12:18]

I see. I was confused by the comment. I changed to CreateAppModalDialogViews and
update both comment and the description.

App can be confusing too (as we have chrome apps), but I couldn't come up with
better name.

[msw, 2014/11/05 03:53:54]

I wonder if "SystemModal" might be appropriate here (and to rename
[JavaScript|Native]SystemModalDialog), to better match ui::ModalType, although
I'm not entirely opposed to this name (as in "the modality extends to all
application instances in the system" i guess?) Maybe ui::ModalType's
MODAL_TYPE_SYSTEM should change to MODAL_TYPE_APP?
https://code.google.com/p/chromium/codesearch#chromium/src/ui/base/ui_base_ty...

[oshima, 2014/11/05 21:42:17]

Looks like we have inconsistent use of app/system. I actually thought about
"system modal", but I didn't pick it because while this is true on chromeos,
it's not really "system wide" modal on other platforms. ProcessModal is another
candidate, although I sounds ugly to me.

In any case, if this is controversal, I'll do that in a separate CL.

I'm happy to rename whatever you and avi@ agrees on though. avi@, what is your
suggestion?

[Avi (use Gerrit), 2014/11/05 21:50:40]

I made my suggestion for "AppModal". I don't like SystemModal due to the
implication that it's across the entire system and all running apps.

+                                            gfx::NativeWindow parent);
 
 #endif  // COMPONENTS_CONSTRAINED_WINDOW_CONSTRAINED_WINDOW_VIEWS_H_