tapted@: I'm having trouble following the code in //components/constrained_window - it seems like there is some support for using sheets, but I can't figure out where the right place is to splice in a view_mode_controller check. (Also, I can't use views_mode_controller there, because it's a component - argh.)

Any ideas?

In theory, we shouldn't need a views_mode_controller check -- each browser window type should have a different subclass of WebContentsModalDialogHost as a member. For browser windows, there's one in chrome/browser/ui/cocoa/web_contents_modal_dialog_host_cocoa.h and one in chrome/browser/ui/views/frame/browser_view_layout.cc - these are what's responsible for positioning things correctly.

Applying "M-67" label as I believe we're targeting poylychome for M67.

lgrey: can you figure this one out?

Is "web-modal" = JS alerts? I can't repro in views mode (Cocoa mode crashes in my debug build for reasons that are 80% likely related, but I want to make sure I'm looking for the right thing)

To follow up:

1) Yes, "Web-modal" = prompt/alert
2) As of this writing, Polychrome views is fine but Polychrome Cocoa behaves as described
3) Root cause: https://cs.chromium.org/chromium/src/components/constrained_window/BUILD.gn?file:%5Esrc/components/constrained_window/+package:%5Echromium$&l=19

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1a37b1651971b41c71ba51040bee38a51584697c

commit 1a37b1651971b41c71ba51040bee38a51584697c
Author: Leonard Grey <lgrey@chromium.org>
Date: Fri Feb 23 18:32:04 2018

[MacViews] Shim |ShowModalDialog| for Polychrome

Since "ViewsBrowserWindows" is a Chrome feature, this CL follows
CreateNativeWebModalManager in declaring a function in components
and defining it in c/b/ui/cocoa.

Background:
Polychrome is an intermediate step to shipping MacViews browser.
Currently, switching between Cocoa browser and MacViews browser requires
a buildflag. Polychrome builds both Cocoa and MacViews into the same
binary and allows switching between them based on a feature flag.
More details at  crbug.com/802257 

As Cocoa and (much of) Views were not typically built together,
some symbols were reused. With Polychrome, these are duplicate symbols.
This CL resolves the issue by renaming the Cocoa symbols, and then
shimming them for the (regular, not Polychrome) Cocoa build.

Bug:  808017 
Change-Id: I9bf1845aa22305928aafd74884793f0fa86afe56
Reviewed-on: https://chromium-review.googlesource.com/929942
Reviewed-by: Mike Wittman <wittman@chromium.org>
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Commit-Queue: Leonard Grey <lgrey@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538832}
[modify] https://crrev.com/1a37b1651971b41c71ba51040bee38a51584697c/chrome/browser/ui/cocoa/create_native_web_modal_manager_cocoa.mm
[modify] https://crrev.com/1a37b1651971b41c71ba51040bee38a51584697c/components/constrained_window/BUILD.gn
[modify] https://crrev.com/1a37b1651971b41c71ba51040bee38a51584697c/components/constrained_window/constrained_window_views.h
[modify] https://crrev.com/1a37b1651971b41c71ba51040bee38a51584697c/components/constrained_window/show_modal_dialog_cocoa.cc
[modify] https://crrev.com/1a37b1651971b41c71ba51040bee38a51584697c/components/constrained_window/show_modal_dialog_views.cc
[modify] https://crrev.com/1a37b1651971b41c71ba51040bee38a51584697c/components/web_modal/web_contents_modal_dialog_manager.h

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6939cb837bb631b316aeb0cb623ba8960fb1f8a6

commit 6939cb837bb631b316aeb0cb623ba8960fb1f8a6
Author: Colin Blundell <blundell@chromium.org>
Date: Wed Feb 28 11:53:24 2018

Revert "[MacViews] Shim |ShowModalDialog| for Polychrome"

This reverts commit 1a37b1651971b41c71ba51040bee38a51584697c.

Reason for revert: Causing components_unittests to fail compile due to missing definition of the static function. See https://bugs.chromium.org/p/chromium/issues/detail?id=817302#c1 for more details.

Original change's description:
> [MacViews] Shim |ShowModalDialog| for Polychrome
> 
> Since "ViewsBrowserWindows" is a Chrome feature, this CL follows
> CreateNativeWebModalManager in declaring a function in components
> and defining it in c/b/ui/cocoa.
> 
> Background:
> Polychrome is an intermediate step to shipping MacViews browser.
> Currently, switching between Cocoa browser and MacViews browser requires
> a buildflag. Polychrome builds both Cocoa and MacViews into the same
> binary and allows switching between them based on a feature flag.
> More details at  crbug.com/802257 
> 
> As Cocoa and (much of) Views were not typically built together,
> some symbols were reused. With Polychrome, these are duplicate symbols.
> This CL resolves the issue by renaming the Cocoa symbols, and then
> shimming them for the (regular, not Polychrome) Cocoa build.
> 
> Bug:  808017 
> Change-Id: I9bf1845aa22305928aafd74884793f0fa86afe56
> Reviewed-on: https://chromium-review.googlesource.com/929942
> Reviewed-by: Mike Wittman <wittman@chromium.org>
> Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
> Commit-Queue: Leonard Grey <lgrey@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#538832}

TBR=ellyjones@chromium.org,wittman@chromium.org,lgrey@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  808017 
Change-Id: I523fee5108b570a8e36bb5f5bc91a3896c916742
Reviewed-on: https://chromium-review.googlesource.com/941161
Reviewed-by: Colin Blundell <blundell@chromium.org>
Commit-Queue: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#539793}
[modify] https://crrev.com/6939cb837bb631b316aeb0cb623ba8960fb1f8a6/chrome/browser/ui/cocoa/create_native_web_modal_manager_cocoa.mm
[modify] https://crrev.com/6939cb837bb631b316aeb0cb623ba8960fb1f8a6/components/constrained_window/BUILD.gn
[modify] https://crrev.com/6939cb837bb631b316aeb0cb623ba8960fb1f8a6/components/constrained_window/constrained_window_views.h
[modify] https://crrev.com/6939cb837bb631b316aeb0cb623ba8960fb1f8a6/components/constrained_window/show_modal_dialog_cocoa.cc
[modify] https://crrev.com/6939cb837bb631b316aeb0cb623ba8960fb1f8a6/components/constrained_window/show_modal_dialog_views.cc
[modify] https://crrev.com/6939cb837bb631b316aeb0cb623ba8960fb1f8a6/components/web_modal/web_contents_modal_dialog_manager.h

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/989dbd8fc3af087ef525b437d498ee7941cbc6a3

commit 989dbd8fc3af087ef525b437d498ee7941cbc6a3
Author: Leonard Grey <lgrey@chromium.org>
Date: Wed Mar 07 18:10:24 2018

MacViews: reland 'Shim |ShowModalDialog| for Polychrome'

This was originally https://chromium-review.googlesource.com/c/chromium/src/+/929942
but was reverted because of the hacky way it implemented a components/ function
in Chrome.

It's now rebased on https://chromium-review.googlesource.com/c/chromium/src/+/949362
which made that behavior unnecessary.

Background:
Polychrome is an intermediate step to shipping MacViews browser.
Currently, switching between Cocoa browser and MacViews browser requires
a buildflag. Polychrome builds both Cocoa and MacViews into the same
binary and allows switching between them based on a feature flag.
More details at  crbug.com/802257 

As Cocoa and (much of) Views were not typically built together,
some symbols were reused. With Polychrome, these are duplicate symbols.
This CL resolves the issue by renaming the Cocoa symbols, and then
shimming them for the (regular, not Polychrome) Cocoa build.

Bug:  808017 
Change-Id: Ia42c202e474614c97759aab28c88f580bc0ffabd
Reviewed-on: https://chromium-review.googlesource.com/950063
Reviewed-by: Mike Wittman <wittman@chromium.org>
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Commit-Queue: Leonard Grey <lgrey@chromium.org>
Cr-Commit-Position: refs/heads/master@{#541500}
[modify] https://crrev.com/989dbd8fc3af087ef525b437d498ee7941cbc6a3/components/constrained_window/BUILD.gn
[modify] https://crrev.com/989dbd8fc3af087ef525b437d498ee7941cbc6a3/components/constrained_window/constrained_window_views.h
[modify] https://crrev.com/989dbd8fc3af087ef525b437d498ee7941cbc6a3/components/constrained_window/show_modal_dialog_cocoa.cc
[modify] https://crrev.com/989dbd8fc3af087ef525b437d498ee7941cbc6a3/components/constrained_window/show_modal_dialog_views.cc

Is this fixed now? :)

Yup :)