Correctly restore the auto managed window to the original bounds when becoming single window

chrome/browser/ui/window_sizer/window_sizer_ash_unittest.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 "chrome/browser/ui/window_sizer/window_sizer_common_unittest.h"
-
+#include "ash/screen_ash.h"
 #include "ash/shell.h"
 #include "ash/test/ash_test_base.h"
 #include "ash/test/test_shell_delegate.h"
 #include "ash/wm/window_resizer.h"
 #include "ash/wm/window_util.h"
 #include "base/compiler_specific.h"
 #include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/window_sizer/window_sizer_common_unittest.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/test/base/testing_profile.h"
 #include "content/public/test/render_view_test.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/aura/client/activation_client.h"
 #include "ui/aura/client/aura_constants.h"
 #include "ui/aura/env.h"
 #include "ui/aura/root_window.h"
 #include "ui/aura/test/test_windows.h"
 
-typedef ash::test::AshTestBase WindowSizerTest;
+typedef ash::test::AshTestBase WindowSizerAshTest;
 
 namespace {
 
 // A browser window proxy which is able to associate an aura native window with
 // it.
 class TestBrowserWindowAura : public TestBrowserWindow {
  public:
   // |native_window| will still be owned by the caller after the constructor
   // was called.
   explicit TestBrowserWindowAura(aura::Window* native_window)
@@ skipping 19 matching lines @@
   virtual gfx::Rect GetBounds() const OVERRIDE {
     return native_window_->bounds();
   }
 
   Browser* browser() { return browser_.get(); }
 
   void CreateBrowser(const Browser::CreateParams& params) {
     Browser::CreateParams create_params = params;
     create_params.window = this;
     browser_.reset(new Browser(create_params));
+    if (browser_->is_type_tabbed() || browser_->is_app())
+      ash::wm::SetWindowPositionManaged(native_window_.get(), true);
   }
 
  private:
   scoped_ptr<Browser> browser_;
   scoped_ptr<aura::Window> native_window_;
 
   DISALLOW_COPY_AND_ASSIGN(TestBrowserWindowAura);
 };
 
 int AlignToGridRoundDown(int location, int grid_size) {
@@ skipping 11 matching lines @@
   scoped_ptr<TestBrowserWindowAura> browser_window(
       new TestBrowserWindowAura(window));
   browser_window->CreateBrowser(params);
   return browser_window.Pass();
 }
 
 }  // namespace
 
 // Test that the window is sized appropriately for the first run experience
 // where the default window bounds calculation is invoked.
-TEST_F(WindowSizerTest, DefaultSizeCase) {
+TEST_F(WindowSizerAshTest, DefaultSizeCase) {
   int grid = WindowSizer::kDesktopBorderSize;
   { // 4:3 monitor case, 1024x768, no taskbar
     gfx::Rect window_bounds;
     GetWindowBounds(p1024x768, p1024x768, gfx::Rect(), gfx::Rect(),
                     gfx::Rect(), DEFAULT, NULL, gfx::Rect(), &window_bounds);
     EXPECT_EQ(gfx::Rect(WindowSizer::kDesktopBorderSize,
                         WindowSizer::kDesktopBorderSize,
                         1024 - WindowSizer::kDesktopBorderSize * 2,
                         768 - WindowSizer::kDesktopBorderSize),
               window_bounds);
@@ skipping 101 matching lines @@
     EXPECT_EQ(gfx::Rect((1920 - WindowSizer::kMaximumWindowWidth) / 2,
                         WindowSizer::kDesktopBorderSize,
                         WindowSizer::kMaximumWindowWidth,
                         1200 - WindowSizer::kDesktopBorderSize),
               window_bounds);
   }
 }
 
 // Test that the next opened window is positioned appropriately given the
 // bounds of an existing window of the same type.
-TEST_F(WindowSizerTest, LastWindowBoundsCase) {
+TEST_F(WindowSizerAshTest, LastWindowBoundsCase) {
   { // normal, in the middle of the screen somewhere.
     gfx::Rect window_bounds;
     GetWindowBounds(p1024x768, p1024x768, gfx::Rect(),
                     gfx::Rect(WindowSizer::kDesktopBorderSize,
                               WindowSizer::kDesktopBorderSize, 500, 400),
                     gfx::Rect(), LAST_ACTIVE, NULL, gfx::Rect(),
                     &window_bounds);
     EXPECT_EQ(gfx::Rect(kWindowTilePixels + WindowSizer::kDesktopBorderSize,
                         kWindowTilePixels + WindowSizer::kDesktopBorderSize,
                         500, 400).ToString(),
@@ skipping 37 matching lines @@
                     gfx::Rect(), LAST_ACTIVE, NULL, gfx::Rect(),
                     &window_bounds);
     EXPECT_EQ(gfx::Rect(kWindowTilePixels + WindowSizer::kDesktopBorderSize,
                         kWindowTilePixels + WindowSizer::kDesktopBorderSize,
                         500, 400).ToString(),
               window_bounds.ToString());
   }
 }
 
 // Test that the window opened is sized appropriately given persisted sizes.
-TEST_F(WindowSizerTest, PersistedBoundsCase) {
+TEST_F(WindowSizerAshTest, PersistedBoundsCase) {
   { // normal, in the middle of the screen somewhere.
     gfx::Rect initial_bounds(WindowSizer::kDesktopBorderSize,
                              WindowSizer::kDesktopBorderSize, 500, 400);
 
     gfx::Rect window_bounds;
     GetWindowBounds(p1024x768, p1024x768, gfx::Rect(), initial_bounds,
                     gfx::Rect(), PERSISTED, NULL, gfx::Rect(), &window_bounds);
     EXPECT_EQ(initial_bounds.ToString(), window_bounds.ToString());
   }
 
@@ skipping 74 matching lines @@
               window_bounds.ToString());
   }
 }
 
 //////////////////////////////////////////////////////////////////////////////
 // The following unittests have different results on Mac/non-Mac because we
 // reposition windows aggressively on Mac.  The *WithAggressiveReposition tests
 // are run on Mac, and the *WithNonAggressiveRepositioning tests are run on
 // other platforms.
 
-TEST_F(WindowSizerTest, LastWindowOffscreenWithNonAggressiveRepositioning) {
+TEST_F(WindowSizerAshTest, LastWindowOffscreenWithNonAggressiveRepositioning) {
   { // taskbar on left.
     gfx::Rect window_bounds;
     GetWindowBounds(p1024x768, taskbar_left_work_area, gfx::Rect(),
                     gfx::Rect(WindowSizer::kDesktopBorderSize,
                               WindowSizer::kDesktopBorderSize, 500, 400),
                     gfx::Rect(), LAST_ACTIVE, NULL, gfx::Rect(),
                     &window_bounds);
     EXPECT_EQ(gfx::Rect(kWindowTilePixels + WindowSizer::kDesktopBorderSize,
                         kWindowTilePixels + WindowSizer::kDesktopBorderSize,
                         500, 400).ToString(),
@@ skipping 54 matching lines @@
                     NULL, gfx::Rect(), &window_bounds);
     EXPECT_EQ(gfx::Rect(994 /* not 995 */,
                         738 /* not 739 */,
                         500,
                         400).ToString(),
               window_bounds.ToString());
   }
 }
 
 // Test the placement of newly created windows.
-TEST_F(WindowSizerTest, PlaceNewWindows) {
+TEST_F(WindowSizerAshTest, PlaceNewWindows) {
   // Create a browser which we can use to pass into the GetWindowBounds
   // function.
   scoped_ptr<TestingProfile> profile(new TestingProfile());
   // Creating a popup handler here to make sure it does not interfere with the
   // existing windows.
   Browser::CreateParams native_params(profile.get(),
                                       chrome::HOST_DESKTOP_TYPE_ASH);
   scoped_ptr<Browser> browser(
       chrome::CreateBrowserWithTestWindowForParams(&native_params));
 
@@ skipping 10 matching lines @@
       gfx::Rect(16, 32, 128, 256),
       Browser::CreateParams(Browser::TYPE_POPUP, profile.get(),
                             chrome::HOST_DESKTOP_TYPE_ASH)));
 
   // Creating a panel to make sure it does not interfere with the positioning.
   scoped_ptr<BrowserWindow> browser_panel(CreateTestBrowserWindow(
       CreateTestWindowInShellWithId(2),
       gfx::Rect(32, 48, 256, 512),
       Browser::CreateParams(Browser::TYPE_POPUP, profile.get(),
                             chrome::HOST_DESKTOP_TYPE_ASH)));
-
   browser_window->Show();
-  { // With a shown window it's size should get returned.
-    gfx::Rect window_bounds;
-    GetWindowBounds(p1600x1200, p1600x1200, gfx::Rect(),
-                    gfx::Rect(50, 100, 300, 150), bottom_s1600x1200,
-                    PERSISTED, browser.get(), gfx::Rect(), &window_bounds);
-    // The position should be right flush.
-    EXPECT_EQ("960,32 640x320", window_bounds.ToString());
-  }
-
-  { // With the window shown - but more on the right side then on the left
-    // side (and partially out of the screen), it should default to the other
-    // side and inside the screen.
-    browser_window->GetNativeWindow()->SetBounds(
-        gfx::Rect(1000, 600, 640, 320));
-    gfx::Rect window_bounds;
-    GetWindowBounds(p1600x1200, p1600x1200, gfx::Rect(),
-                    gfx::Rect(50, 100, 300, 150), bottom_s1600x1200,
-                    PERSISTED, browser.get(), gfx::Rect(), &window_bounds);
-    // The position should be left & bottom flush.
-    EXPECT_EQ("0,600 640x320", window_bounds.ToString());
-  }
-
-  { // If the second windows right side is already over the right side of the
-    // screen, it will not move back into the screen.
-    browser_window->GetNativeWindow()->SetBounds(
-        gfx::Rect(1000, 600, 640, 320));
-    gfx::Rect window_bounds;
-    GetWindowBounds(p1600x1200, p1600x1200, gfx::Rect(),
-                    gfx::Rect(50, 100, 300, 150), bottom_s1600x1200,
-                    PERSISTED, browser.get(), gfx::Rect(), &window_bounds);
-    // The position should be left & bottom flush.
-    EXPECT_EQ("0,600 640x320", window_bounds.ToString());
-    // If the other window was already beyond the point to get right flush
-    // it will remain where it is.
-    EXPECT_EQ("1000,600 640x320",
-              browser_window->GetNativeWindow()->bounds().ToString());
-  }
-
   { // Make sure that popups do not get changed.
     gfx::Rect window_bounds;
     GetWindowBounds(p1600x1200, p1600x1200, gfx::Rect(),
                     gfx::Rect(50, 100, 300, 150), bottom_s1600x1200,
                     PERSISTED, browser_popup->browser(),
                     gfx::Rect(), &window_bounds);
     EXPECT_EQ("50,100 300x150", window_bounds.ToString());
   }
 
   browser_window->Hide();
@@ skipping 18 matching lines @@
                         std::min(WindowSizer::kMaximumWindowWidth,
                                  1600 - 2 * WindowSizer::kDesktopBorderSize),
                         1200 - WindowSizer::kDesktopBorderSize).ToString(),
               window_bounds.ToString());
   }
 }
 
 // Test the placement of newly created windows on an empty desktop.
 // This test supplements "PlaceNewWindows" by testing the creation of a newly
 // created browser window on an empty desktop.
-TEST_F(WindowSizerTest, PlaceNewBrowserWindowOnEmptyDesktop) {
+TEST_F(WindowSizerAshTest, PlaceNewBrowserWindowOnEmptyDesktop) {
   // Create a browser which we can use to pass into the GetWindowBounds
   // function.
   scoped_ptr<TestingProfile> profile(new TestingProfile());
   Browser::CreateParams native_params(profile.get(),
                                       chrome::HOST_DESKTOP_TYPE_ASH);
   scoped_ptr<Browser> browser(
       chrome::CreateBrowserWithTestWindowForParams(&native_params));
 
   // A common screen size for Chrome OS devices where this behavior is
   // desirable.
@@ skipping 55 matching lines @@
 }
 
 #if defined(OS_CHROMEOS)
 #define MAYBE_PlaceNewWindowsOnMultipleDisplays PlaceNewWindowsOnMultipleDisplays
 #else
 // No multiple displays on windows ash.
 #define MAYBE_PlaceNewWindowsOnMultipleDisplays DISABLED_PlaceNewWindowsOnMultipleDisplays
 #endif
 
 // Test the placement of newly created windows on multiple dislays.
-TEST_F(WindowSizerTest, MAYBE_PlaceNewWindowsOnMultipleDisplays) {
+TEST_F(WindowSizerAshTest, MAYBE_PlaceNewWindowsOnMultipleDisplays) {
   UpdateDisplay("1600x1200,1600x1200");
-  const gfx::Rect secondary(1600, 0, 1600, 1200);
+  gfx::Rect primary_bounds = ash::Shell::GetInstance()->GetScreen()->
+      GetPrimaryDisplay().bounds();
+  gfx::Rect secondary_bounds = ash::ScreenAsh::GetSecondaryDisplay().bounds();
 
   ash::Shell::GetInstance()->set_active_root_window(
       ash::Shell::GetPrimaryRootWindow());
 
   scoped_ptr<TestingProfile> profile(new TestingProfile());
 
   // Create browser windows that are used as reference.
   scoped_ptr<BrowserWindow> browser_window(CreateTestBrowserWindow(
       CreateTestWindowInShellWithId(0),
       gfx::Rect(10, 10, 200, 200),
       Browser::CreateParams(profile.get(), chrome::HOST_DESKTOP_TYPE_ASH)));
   browser_window->Show();
   EXPECT_EQ(browser_window->GetNativeWindow()->GetRootWindow(),
             ash::Shell::GetActiveRootWindow());
 
   scoped_ptr<BrowserWindow> another_browser_window(CreateTestBrowserWindow(
       CreateTestWindowInShellWithId(1),
-      gfx::Rect(1600 - 200, 10, 300, 300),
+      gfx::Rect(400, 10, 300, 300),
       Browser::CreateParams(profile.get(), chrome::HOST_DESKTOP_TYPE_ASH)));
   another_browser_window->Show();
 
   // Creating a new window to verify the new placement.
   scoped_ptr<TestBrowserWindowAura> new_browser_window(CreateTestBrowserWindow(
       CreateTestWindowInShellWithId(0),
       gfx::Rect(),
       Browser::CreateParams(profile.get(),
                             chrome::HOST_DESKTOP_TYPE_ASH)));
 
   // Make sure the primary root is active.
   ASSERT_EQ(ash::Shell::GetPrimaryRootWindow(),
             ash::Shell::GetActiveRootWindow());
 
   // First new window should be in the primary.
   {
     gfx::Rect window_bounds;
-    GetWindowBounds(p1600x1200, p1600x1200, secondary,
-                    gfx::Rect(), secondary,
+    GetWindowBounds(p1600x1200, p1600x1200, secondary_bounds,
+                    gfx::Rect(), secondary_bounds,
                     PERSISTED, new_browser_window->browser(),
                     gfx::Rect(), &window_bounds);
-    EXPECT_EQ("0,10 300x300", window_bounds.ToString());
+    EXPECT_TRUE(primary_bounds.Contains(window_bounds));
   }
 
   // Move the window to the right side of the secondary display and create a new
-  // window. It should be opened then on the left side on the secondary display.
+  // window. It should be opened then on the secondary display.
   {
     gfx::Display second_display = ash::Shell::GetScreen()->
         GetDisplayNearestPoint(gfx::Point(1600 + 100,10));
     browser_window->GetNativeWindow()->SetBoundsInScreen(
-        gfx::Rect(secondary.CenterPoint().x() + 300, 10, 200, 200),
+        gfx::Rect(secondary_bounds.CenterPoint().x() - 100, 10, 200, 200),

[Mr4D (OOO till 08-26), 2013/08/28 21:58:15]

[this is one comment for multiple lines [615, 637, 681, 692, 707, ..]:

I was under the assumption that this CL's purpose is to move code around - not
to change the basic placement functionality. Shouldn't this be the same as
before? Why did you change the placement outcome?

Also - I like unit tests to validate a predictable outcome - means to check for
an explicit solution (as it was before in e.g, 692) and not a general vague
result (like 656).

[oshima, 2013/08/29 09:18:04]

This is because I removed the code from GetBoundsOverrideAsh, which changed
expectation, and changed the condition to test.
What I wanted to test is if it's opened on the same display as reference window,
instead of if it has some exact bounds because the exact location to be used
is unknown without RearrangeVisibleWindowOnShow (the bounds is updated in this
funtion).

In order to test the actual bounds that will be used, I need to combine
WindowSizerAsh and auto management code, which is next step. I added TODO to
update the test.

If you want to check the exact bounds now, it'll look like this:

    EXPECT_EQ(browser_window->GetNativeWindow()->GetBoundsInScreen().ToString(),
              window_bounds.ToString());

I can change to this if you want. Please let me know.

[Mr4D (OOO till 08-26), 2013/08/29 15:58:15]

Okay - lets keep the TODO for the moment then.

         second_display);
     browser_window->Activate();
     EXPECT_NE(ash::Shell::GetPrimaryRootWindow(),
               ash::Shell::GetActiveRootWindow());
-
     gfx::Rect window_bounds;
-    GetWindowBounds(p1600x1200, p1600x1200, secondary,
-                    gfx::Rect(), secondary,
+    GetWindowBounds(p1600x1200, p1600x1200, secondary_bounds,
+                    gfx::Rect(), secondary_bounds,
                     PERSISTED, new_browser_window->browser(),
                     gfx::Rect(), &window_bounds);
-    EXPECT_EQ("1600,10 200x200", window_bounds.ToString());
+    EXPECT_TRUE(secondary_bounds.Contains(window_bounds));
   }
 
   // Activate another window in the primary display and create a new window.
   // It should be created in the primary display.
   {
     another_browser_window->Activate();
     EXPECT_EQ(ash::Shell::GetPrimaryRootWindow(),
               ash::Shell::GetActiveRootWindow());
 
     gfx::Rect window_bounds;
-    GetWindowBounds(p1600x1200, p1600x1200, secondary,
-                    gfx::Rect(), secondary,
+    GetWindowBounds(p1600x1200, p1600x1200, secondary_bounds,
+                    gfx::Rect(), secondary_bounds,
                     PERSISTED, new_browser_window->browser(),
                     gfx::Rect(), &window_bounds);
-    EXPECT_EQ("0,10 300x300", window_bounds.ToString());
+    EXPECT_TRUE(primary_bounds.Contains(window_bounds));
   }
 }
 
 // Test that the show state is properly returned for non default cases.
-TEST_F(WindowSizerTest, TestShowState) {
+TEST_F(WindowSizerAshTest, TestShowState) {
   scoped_ptr<TestingProfile> profile(new TestingProfile());
 
   // Creating a browser & window to play with.
   scoped_ptr<TestBrowserWindowAura> browser_window(CreateTestBrowserWindow(
       CreateTestWindowInShellWithId(0),
       gfx::Rect(16, 32, 640, 320),
       Browser::CreateParams(Browser::TYPE_TABBED, profile.get(),
                             chrome::HOST_DESKTOP_TYPE_ASH)));
 
   // Create also a popup browser since that behaves different.
@@ skipping 72 matching lines @@
               GetWindowShowState(ui::SHOW_STATE_MAXIMIZED,
                                  ui::SHOW_STATE_DEFAULT,
                                  BOTH,
                                  browser_window2->browser(),
                                  tiny_screen));
 
   }
 }
 
 // Test that the default show state override behavior is properly handled.
-TEST_F(WindowSizerTest, TestShowStateDefaults) {
+TEST_F(WindowSizerAshTest, TestShowStateDefaults) {
   // Creating a browser & window to play with.
   scoped_ptr<TestingProfile> profile(new TestingProfile());
 
   scoped_ptr<TestBrowserWindowAura> browser_window(CreateTestBrowserWindow(
       CreateTestWindowInShellWithId(0),
       gfx::Rect(16, 32, 640, 320),
       Browser::CreateParams(Browser::TYPE_TABBED, profile.get(),
                             chrome::HOST_DESKTOP_TYPE_ASH)));
 
   // Create also a popup browser since that behaves slightly different for
@@ skipping 41 matching lines @@
                                browser_window->browser(),
                                p1600x1200), ui::SHOW_STATE_MAXIMIZED);
 
   // The popup should favor the initial show state over the command line.
   EXPECT_EQ(GetWindowShowState(ui::SHOW_STATE_NORMAL,
                                ui::SHOW_STATE_NORMAL,
                                BOTH,
                                browser_popup->browser(),
                                p1600x1200), ui::SHOW_STATE_NORMAL);
 }