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

chrome/browser/ui/views/web_dialog_view_browsertest.cc

 // 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.
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/files/file_path.h"
 #include "base/memory/singleton.h"
 #include "base/message_loop/message_loop.h"
 #include "base/strings/utf_string_conversions.h"
@@ skipping 66 matching lines @@
   DISALLOW_COPY_AND_ASSIGN(TestWebDialogView);
 };
 
 }  // namespace
 
 class WebDialogBrowserTest : public InProcessBrowserTest {
  public:
   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));
   delegate->set_size(kInitialWidth, kInitialHeight);
 
   TestWebDialogView* view =
       new TestWebDialogView(browser()->profile(), delegate);
   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.
   view->set_should_quit_on_size_change(true);
 
   gfx::Rect bounds = view->GetWidget()->GetClientAreaBoundsInScreen();
 
   gfx::Rect set_bounds = bounds;
   gfx::Rect actual_bounds, rwhv_bounds;
 
@@ skipping 38 matching lines @@
   actual_bounds = view->GetWidget()->GetClientAreaBoundsInScreen();
   EXPECT_EQ(set_bounds, actual_bounds);
 
   rwhv_bounds =
       view->web_contents()->GetRenderWidgetHostView()->GetViewBounds();
   EXPECT_LT(0, rwhv_bounds.width());
   EXPECT_LT(0, rwhv_bounds.height());
   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);
 
   view->MoveContents(web_contents, set_bounds);
   content::RunMessageLoop();  // TestWebDialogView will quit.
   actual_bounds = view->GetWidget()->GetClientAreaBoundsInScreen();
   EXPECT_LT(0, actual_bounds.width());
   EXPECT_LT(0, actual_bounds.height());
 }