Clamp dialog bounds to be fully visible on the nearest display

components/constrained_window/constrained_window_views_unittest.cc

Index: components/constrained_window/constrained_window_views_unittest.cc
diff --git a/components/constrained_window/constrained_window_views_unittest.cc b/components/constrained_window/constrained_window_views_unittest.cc
index a0dd9dfddc3cca626e77cba1198e509f81d0cd0c..2394cdb37096a917c05506a6e27438089d68b3fd 100644
--- a/components/constrained_window/constrained_window_views_unittest.cc
+++ b/components/constrained_window/constrained_window_views_unittest.cc
@@ -5,10 +5,13 @@
 #include "components/constrained_window/constrained_window_views.h"
 
 #include <memory>
+#include <vector>
 
 #include "base/macros.h"
 #include "components/constrained_window/constrained_window_views_client.h"
 #include "components/web_modal/test_web_contents_modal_dialog_host.h"
+#include "ui/display/display.h"
+#include "ui/display/screen.h"
 #include "ui/gfx/geometry/point.h"
 #include "ui/gfx/geometry/rect.h"
 #include "ui/gfx/geometry/size.h"
@@ -228,4 +231,48 @@ TEST_F(ConstrainedWindowViewsTest, NullModalParent) {
   widget->CloseNow();
 }
 
+// Make sure dialogs presented off-screen are properly clamped to the nearest
+// screen.
+TEST_F(ConstrainedWindowViewsTest, ClampDialogToNearestDisplay) {
+  // First, make sure the host and dialog are sized and positioned.
+  UpdateWebContentsModalDialogPosition(dialog(), dialog_host());
+
+  // Calculate the largest screen rectangle that will contain all the displays.
+  const display::Screen* screen = display::Screen::GetScreen();
+  const std::vector<display::Display>& displays = screen->GetAllDisplays();
+  gfx::Rect extents = gfx::Rect();
+  for (auto display : displays) {
+    const gfx::Rect bounds = display.bounds();

[msw, 2016/12/15 20:46:09]

use Rect::Union (or skip and just assert that there's only one display)

[kylix_rd, 2016/12/15 21:30:15]

Done.

+    extents.set_x(std::min(bounds.x(), extents.x()));
+    extents.set_y(std::min(bounds.y(), extents.y()));
+    extents.set_width(std::max(bounds.right(),
+                               extents.right()) - extents.x());
+    extents.set_height(std::max(bounds.bottom(),
+                                extents.bottom()) - extents.y());
+  }
+
+  // Move the host completely off the screen.
+  views::Widget* host_widget =
+      views::Widget::GetWidgetForNativeView(dialog_host()->GetHostView());
+  gfx::Rect host_bounds = host_widget->GetWindowBoundsInScreen();
+  host_bounds.set_origin(gfx::Point(extents.right(), extents.bottom()));
+  host_widget->SetBounds(host_bounds);
+
+  // Precalculate the expected location of the dialog.

[msw, 2016/12/15 20:46:09]

just check that the one display's bounds contains the dialog bounds? Otherwise,
this test falls into the "change detector" trap of duplicating the production
code and checking that both yield the same behavior, versus checking the basic
important behavior (ie. even if the host if off-screen, the dialog will be shown
completely on-screen)

[kylix_rd, 2016/12/15 21:30:16]

Yes, that code seemed a little silly... I've reworked. Next patch.

+  const display::Display display = screen->GetDisplayNearestWindow(
+      dialog_host()->GetHostView());
+  gfx::Rect expected_bounds = dialog()->GetRootView()->GetBoundsInScreen();
+  expected_bounds.AdjustToFit(display.work_area());
+
+  // Now make sure the modal dialog is repositioned into one of the displays.
+  // This should be on the display calculated above.
+  UpdateWebContentsModalDialogPosition(dialog(), dialog_host());
+
+  const gfx::Rect dialog_bounds = dialog()->GetRootView()->GetBoundsInScreen();
+
+  // expected_bounds.origin() should be where the dialog is now located.
+  // NOTE: Don't test the size since it is constrained by preferred size.
+  EXPECT_EQ(expected_bounds.origin(), dialog_bounds.origin());
+}
+
 }  // namespace constrained_window