Place browser-modal dialogs like web-contents-modal dialogs.

Place browser-modal dialogs like web-contents-modal dialogs.

Add CreateBrowserModalDialogViews for browser-modal dialogs.
Places dialogs with browser window parents dialog like wcmds.
(somewhat parallel to CreateWebContentsModalDialogViews)
See example before/after pics at: http://crbug.com/246777#c8

Update all applicable dialogs with the new call.
Remove now extraneous JS dialog placement code.

TODO(followup): Similar for c/b/chromeos dialogs (violates DEPS):
-chrome/browser/chromeos/enrollment_dialog_view.cc
-chrome/browser/chromeos/external_protocol_dialog.cc
-chrome/browser/chromeos/options/network_config_view.cc
-chrome/browser/chromeos/ui/echo_dialog_view.cc

BUG=166075, 246777
TEST=Browser-modal dialogs are placed top-aligned to their browser.
R=wittman@chromium.org,sky@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=205718

[msw, 2013-06-06 23:10:07]

Hey Mike and Scott, please take a look; thanks!

Can you advise me on how to resolve the DEPS violations?
1) Move files from c/b/chromeos[/options] to c/b/chromeos/ui
   (and potentially allow that as a new dependency?)
2) Move files from c/b/chromeos[/options] c/b/ui/views/ash
3) Ignore the DEPS violation or something else???

[Mike Wittman, 2013-06-07 02:38:03]

On the DEPS violation, also consider moving CreateWebContentsModalDialogViews
and CreateBrowserModalDialogViews and their dependencies within
constrained_window_views.cc to somewhere else.  This code should only depend on
ui/views + Browser, and that file needs to go away anyway once the new dialog
style flag flip sticks.

https://codereview.chromium.org/16374006/diff/12001/chrome/browser/ui/views/c...
File chrome/browser/ui/views/constrained_window_views.cc (right):

https://codereview.chromium.org/16374006/diff/12001/chrome/browser/ui/views/c...
chrome/browser/ui/views/constrained_window_views.cc:698: if (browser) {
How about DCHECK(browser)?  Wouldn't invoking this function with a non-browser
parent window be a coding error?

[sky, 2013-06-07 17:03:47]

One question, what happens if the browser window is too small?

Also, it feels to me like this modality should be possible at the views level
and not browser specific.

[msw, 2013-06-07 17:51:12]

Mike wrote:
> ... consider moving ... constrained_window_views.cc to somewhere else.
> ... This code should only depend on ui/views + Browser...
The problem is that c/b/chromeos[/ui] can't depend on c/b/ui/views.
Maybe I don't follow; I think we should just add a dependency or ignore.

Scott wrote:
> One question, what happens if the browser window is too small?
The top-level child dialog isn't actually constrained by browser bounds.
The dialog gets h-centered and v-top-aligned to the omnibox/bookmarks.
It will exceed the browser bounds as needed.

> Also, it feels to me like this modality should be possible at the views level
> and not browser specific.
This new code just places the dialog at the omnibox/bookmarks height.
No modality logic is changed in this CL, just browser-child positioning.
It uses the same DialogDelegate::CreateDialogWidget modality, afaik.

https://codereview.chromium.org/16374006/diff/12001/chrome/browser/ui/views/c...
File chrome/browser/ui/views/constrained_window_views.cc (right):

https://codereview.chromium.org/16374006/diff/12001/chrome/browser/ui/views/c...
chrome/browser/ui/views/constrained_window_views.cc:698: if (browser) {
On 2013/06/07 02:38:03, Mike Wittman wrote:
> How about DCHECK(browser)?  Wouldn't invoking this function with a non-browser
> parent window be a coding error? 

I'm not so sure, take a look at the parent wrangling in network_config_view.cc.

[sky, 2013-06-07 19:28:44]

On Fri, Jun 7, 2013 at 10:51 AM,  <msw@chromium.org> wrote:
> Mike wrote:
>>
>> ... consider moving ... constrained_window_views.cc to somewhere else.
>> ... This code should only depend on ui/views + Browser...
>
> The problem is that c/b/chromeos[/ui] can't depend on c/b/ui/views.
> Maybe I don't follow; I think we should just add a dependency or ignore.

You should talk with Ben about this one.

  -Scott

>
> Scott wrote:
>>
>> One question, what happens if the browser window is too small?
>
> The top-level child dialog isn't actually constrained by browser bounds.
> The dialog gets h-centered and v-top-aligned to the omnibox/bookmarks.
> It will exceed the browser bounds as needed.
>
>
>> Also, it feels to me like this modality should be possible at the views
>> level
>> and not browser specific.
>
> This new code just places the dialog at the omnibox/bookmarks height.
> No modality logic is changed in this CL, just browser-child positioning.
> It uses the same DialogDelegate::CreateDialogWidget modality, afaik.
>
>
>
>
https://codereview.chromium.org/16374006/diff/12001/chrome/browser/ui/views/c...
> File chrome/browser/ui/views/constrained_window_views.cc (right):
>
>
https://codereview.chromium.org/16374006/diff/12001/chrome/browser/ui/views/c...
> chrome/browser/ui/views/constrained_window_views.cc:698: if (browser) {
> On 2013/06/07 02:38:03, Mike Wittman wrote:
>>
>> How about DCHECK(browser)?  Wouldn't invoking this function with a
>
> non-browser
>>
>> parent window be a coding error?
>
>
> I'm not so sure, take a look at the parent wrangling in
> network_config_view.cc.
>
> https://codereview.chromium.org/16374006/

[msw, 2013-06-11 20:04:34]

I reverted c/b/chromeos files for now, please take a look.
I'll leave any changes there as a TODO for two reasons:
1) DEPS complains even after moving files and adding a DEP.
(adding c/b/ui/chromeos/DEPS with +c/b/ui/views doesn't work)
2) These dialogs are less frequently parented to a browser.
(network connecting, enrollment, etc. are often non-child)

[sky, 2013-06-11 21:34:57]

LGTM

[commit-bot: I haz the power, 2013-06-11 21:36:46]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/msw@chromium.org/16374006/32001

[commit-bot: I haz the power, 2013-06-12 04:03:51]

Change committed as 205718