MacViews: Fix WebDialogBrowserTest.SizeWindow to get browser_tests compiling on MacViews

chrome/browser/ui/views/web_dialog_view_browsertest.cc

Index: chrome/browser/ui/views/web_dialog_view_browsertest.cc
diff --git a/chrome/browser/ui/views/web_dialog_view_browsertest.cc b/chrome/browser/ui/views/web_dialog_view_browsertest.cc
index dafe6baa39104f8f0cf9f1f0d6e8844e12719449..9069d81f2edf4dc8398418ea09208ab31245e50c 100644
--- a/chrome/browser/ui/views/web_dialog_view_browsertest.cc
+++ b/chrome/browser/ui/views/web_dialog_view_browsertest.cc
@@ -84,12 +84,15 @@ class WebDialogBrowserTest : public InProcessBrowserTest {
   WebDialogBrowserTest() {}
 };
 
-// http://code.google.com/p/chromium/issues/detail?id=52602
-// Windows has some issues resizing windows- an off by one problem,
-// and a minimum size that seems too big.  This file isn't included in
-// Mac builds yet. On Chrome OS, this test doesn't apply since ChromeOS
-// doesn't allow resizing of windows.
-IN_PROC_BROWSER_TEST_F(WebDialogBrowserTest, DISABLED_SizeWindow) {
+// Windows has some issues resizing windows. An off by one problem, and a
+// minimum size that seems too big. See http://crbug.com/52602. On Chrome OS,
+// this test doesn't apply since ChromeOS doesn't allow resizing of windows.
+#if defined(OS_WIN) || defined(OS_CHROMEOS)

[tapted, 2014/08/19 13:17:57]

There are some bot runs in crrev.com/487143002. As per the comment, the test
isn't happy on Windows (seems content on CrOS though..)

[msw, 2014/08/19 18:56:36]

Go ahead and enable on CrOS, that comment is surely out of date.

[tapted, 2014/08/20 11:32:23]

Done.

+#define MAYBE_SizeWindow DISABLED_SizeWindow
+#else
+#define MAYBE_SizeWindow SizeWindow
+#endif
+IN_PROC_BROWSER_TEST_F(WebDialogBrowserTest, MAYBE_SizeWindow) {
   ui::test::TestWebDialogDelegate* delegate =
       new ui::test::TestWebDialogDelegate(
           GURL(chrome::kChromeUIChromeURLsURL));
@@ -100,8 +103,7 @@ IN_PROC_BROWSER_TEST_F(WebDialogBrowserTest, DISABLED_SizeWindow) {
   WebContents* web_contents =
       browser()->tab_strip_model()->GetActiveWebContents();
   ASSERT_TRUE(web_contents != NULL);
-  views::Widget::CreateWindowWithParent(
-      view, web_contents->GetTopLevelNativeWindow());
+  views::Widget::CreateWindowWithParent(view, web_contents->GetNativeView());
   view->GetWidget()->Show();
 
   // TestWebDialogView should quit current message loop on size change.
@@ -160,7 +162,12 @@ IN_PROC_BROWSER_TEST_F(WebDialogBrowserTest, DISABLED_SizeWindow) {
   EXPECT_GE(set_bounds.width(), rwhv_bounds.width());
   EXPECT_GE(set_bounds.height(), rwhv_bounds.height());
 
-  // Check to make sure we can't get to 0x0
+  // Check to make sure we can't get to 0x0. First make it larger than the
+  // minimum size to ensure there's something to change.

[msw, 2014/08/19 18:56:36]

Is this verifying that re-sizing to 0x0 enforces the minimum size? Is there a
good reason to add the extra 250 step without verifying that there is indeed a
change between that and the 0x0 step? Could you clarify the comment a bit?
Consider something like: "Expand beyond the minimum size to ..., then verify
that attempts to re-size to 0x0 enforces the minimum size."

[tapted, 2014/08/20 11:32:23]

Yes - the old code was doing that below, but it was already at the minimum size
at this point. So, not only is SaveWindowPlacement() skipped, but the override
in the test harness above explicitly skips posting the QuitClosure if the bounds
haven't changed.
No harm in verifying (I just skipped it here for being redundant). Added a
check.
Done.

+  set_bounds.set_height(250);
+  view->MoveContents(web_contents, set_bounds);
+  content::RunMessageLoop();  // TestWebDialogView will quit.
+
   set_bounds.set_width(0);
   set_bounds.set_height(0);