Makes a window that has been resized to maximized bounds, then maximized and...

Makes a window that has been resized to maximized bounds, then maximized and then restored shrink. This allows it to be resized easier after this sequence.

BUG=392599
TEST=ash_unittests --gtest_filter="WindowStateTest.RestoredWindowBoundsShrink"
TEST=unit_tests --gtest_filter="WindowSizerAshTest.DefaultStateBecomesMaximized"
TEST=unit_tests --gtest_filter="WindowSizerAshTest*"

Review URL: https://codereview.chromium.org/424463002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@287675 0039d316-1c4b-4281-b951-d872f2087c98

ash/wm/default_state.cc

@@ -511,10 +511,22 @@ void DefaultState::UpdateBoundsFromState(WindowState* window_state,
     case WINDOW_STATE_TYPE_NORMAL: {
       gfx::Rect work_area_in_parent =
           ScreenUtil::GetDisplayWorkAreaBoundsInParent(window_state->window());
-      if (window_state->HasRestoreBounds())
+      if (window_state->HasRestoreBounds()) {
         bounds_in_parent = window_state->GetRestoreBoundsInParent();
-      else
+        // Check if the |window|'s restored size is bigger than the working area
+        // This may happen if a window was resized to maximized bounds or if the
+        // display resolution changed while the window was maximized.
+        if (previous_state_type == WINDOW_STATE_TYPE_MAXIMIZED &&
+            bounds_in_parent.width() >= work_area_in_parent.width() &&
+            bounds_in_parent.height() >= work_area_in_parent.height()) {
+          // Inset the bounds slightly so that they are not exactly same as
+          // the work area bounds and it is easier to resize the window.
+          bounds_in_parent = work_area_in_parent;
+          bounds_in_parent.Inset(10, 10, 10, 10);
+        }
+      } else {
         bounds_in_parent = window->bounds();
+      }
       // Make sure that part of the window is always visible.
       AdjustBoundsToEnsureMinimumWindowVisibility(
           work_area_in_parent, &bounds_in_parent);

ash/wm/window_state_unittest.cc

@@ -312,6 +312,26 @@ TEST_F(WindowStateTest, StateSwapRestore) {
   EXPECT_FALSE(window_state->IsMaximized());
 }
 
+// Tests that a window that had same bounds as the work area shrinks after the
+// window is maximized and then restored.
+TEST_F(WindowStateTest, RestoredWindowBoundsShrink) {
+  scoped_ptr<aura::Window> window(CreateTestWindowInShellWithId(0));
+  WindowState* window_state = GetWindowState(window.get());
+  EXPECT_FALSE(window_state->IsMaximized());
+  gfx::Rect work_area =
+      ash::Shell::GetScreen()->GetPrimaryDisplay().work_area();
+
+  window->SetBounds(work_area);
+  window_state->Maximize();
+  EXPECT_TRUE(window_state->IsMaximized());
+  EXPECT_EQ(work_area.ToString(), window->bounds().ToString());
+
+  window_state->Restore();
+  EXPECT_FALSE(window_state->IsMaximized());
+  EXPECT_NE(work_area.ToString(), window->bounds().ToString());
+  EXPECT_TRUE(work_area.Contains(window->bounds()));
+}
+
 // TODO(skuhne): Add more unit test to verify the correctness for the restore
 // operation.
 

chrome/browser/ui/window_sizer/window_sizer.h

@@ -163,6 +163,9 @@ class WindowSizer {
   // Ash specific logic for window placement. Returns true if |bounds| and
   // |show_state| have been fully determined, otherwise returns false (but
   // may still affect |show_state|).
+  // If the window is too big to fit in the display work area then the |bounds|
+  // are adjusted to default bounds and the |show_state| is adjusted to
+  // SHOW_STATE_MAXIMIZED.
   bool GetBrowserBoundsAsh(gfx::Rect* bounds,
                            ui::WindowShowState* show_state) const;
 

chrome/browser/ui/window_sizer/window_sizer_ash.cc

@@ -19,40 +19,51 @@ bool WindowSizer::GetBrowserBoundsAsh(gfx::Rect* bounds,
       browser_->host_desktop_type() != chrome::HOST_DESKTOP_TYPE_ASH)
     return false;
 
+  bool determined = false;
   if (bounds->IsEmpty()) {
     if (browser_->is_type_tabbed()) {
       GetTabbedBrowserBoundsAsh(bounds, show_state);
-      return true;
-    }
-
-    if (browser_->is_trusted_source()) {
+      determined = true;
+    } else if (browser_->is_trusted_source()) {
       // For trusted popups (v1 apps and system windows), do not use the last
       // active window bounds, only use saved or default bounds.
       if (!GetSavedWindowBounds(bounds, show_state))
         GetDefaultWindowBounds(GetTargetDisplay(gfx::Rect()), bounds);
-      return true;
+      determined = true;
+    } else {
+      // In Ash, prioritize the last saved |show_state|. If you have questions
+      // or comments about this behavior please contact oshima@chromium.org.
+      if (state_provider_) {
+        gfx::Rect ignored_bounds, ignored_work_area;
+        state_provider_->GetPersistentState(&ignored_bounds,
+                                            &ignored_work_area,
+                                            show_state);
+      }
     }
-
-    // In Ash, prioritize the last saved |show_state|. If you have questions
-    // or comments about this behavior please contact oshima@chromium.org.
-    if (state_provider_) {
-      gfx::Rect ignored_bounds, ignored_work_area;
-      state_provider_->GetPersistentState(&ignored_bounds,
-                                          &ignored_work_area,
-                                          show_state);
-    }
-    return false;
-  }
-
-  // In case of a popup with an 'unspecified' location in ash, we are
-  // looking for a good screen location. We are interpreting (0,0) as an
-  // unspecified location.
-  if (browser_->is_type_popup() && bounds->origin().IsOrigin()) {
+  } else if (browser_->is_type_popup() && bounds->origin().IsOrigin()) {
+    // In case of a popup with an 'unspecified' location in ash, we are
+    // looking for a good screen location. We are interpreting (0,0) as an
+    // unspecified location.
     *bounds = ash::Shell::GetInstance()->window_positioner()->
         GetPopupPosition(*bounds);
-    return true;
+    determined = true;
+  }
+
+  if (browser_->is_type_tabbed() && *show_state == ui::SHOW_STATE_DEFAULT) {
+    gfx::Display display = screen_->GetDisplayMatching(*bounds);
+    gfx::Rect work_area = display.work_area();
+    bounds->AdjustToFit(work_area);
+    if (*bounds == work_area) {
+      // A |browser_| that occupies the whole work area gets maximized.
+      // |bounds| returned here become the restore bounds once the window
+      // gets maximized after this method returns. Return a sensible default
+      // in order to make restored state visibly different from maximized.
+      *show_state = ui::SHOW_STATE_MAXIMIZED;
+      *bounds = ash::WindowPositioner::GetDefaultWindowBounds(display);
+      determined = true;
+    }
   }
-  return false;
+  return determined;
 }
 
 void WindowSizer::GetTabbedBrowserBoundsAsh(

chrome/browser/ui/window_sizer/window_sizer_ash_unittest.cc

@@ -469,8 +469,7 @@ TEST_F(WindowSizerAshTest, LastWindowOffscreenWithNonAggressiveRepositioning) {
 
 // Test the placement of newly created windows.
 TEST_F(WindowSizerAshTest, MAYBE_PlaceNewWindows) {
-  // Create a browser which we can use to pass into the GetWindowBounds
-  // function.
+  // Create a browser 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.
@@ -549,8 +548,7 @@ TEST_F(WindowSizerAshTest, MAYBE_PlaceNewWindows) {
 // This test supplements "PlaceNewWindows" by testing the creation of a newly
 // created browser window on an empty desktop.
 TEST_F(WindowSizerAshTest, MAYBE_PlaceNewBrowserWindowOnEmptyDesktop) {
-  // Create a browser which we can use to pass into the GetWindowBounds
-  // function.
+  // Create a browser to pass into the GetWindowBoundsAndShowState function.
   scoped_ptr<TestingProfile> profile(new TestingProfile());
   Browser::CreateParams native_params(profile.get(),
                                       chrome::HOST_DESKTOP_TYPE_ASH);
@@ -752,12 +750,23 @@ TEST_F(WindowSizerAshTest, MAYBE_TestShowState) {
                                ui::SHOW_STATE_NORMAL,
                                BOTH,
                                browser_window->browser(),
+                               p1600x1200,
                                p1600x1200));
+  // A window that is smaller than the whole work area is set to default state.
   EXPECT_EQ(ui::SHOW_STATE_DEFAULT,
             GetWindowShowState(ui::SHOW_STATE_DEFAULT,
                                ui::SHOW_STATE_NORMAL,
                                BOTH,
                                browser_window->browser(),
+                               p1280x1024,
+                               p1600x1200));
+  // A window that is sized to occupy the whole work area is maximized.
+  EXPECT_EQ(ui::SHOW_STATE_MAXIMIZED,
+            GetWindowShowState(ui::SHOW_STATE_DEFAULT,
+                               ui::SHOW_STATE_NORMAL,
+                               BOTH,
+                               browser_window->browser(),
+                               p1600x1200,
                                p1600x1200));
   // Non tabbed windows should always follow the window saved visibility state.
   EXPECT_EQ(ui::SHOW_STATE_MAXIMIZED,
@@ -765,6 +774,7 @@ TEST_F(WindowSizerAshTest, MAYBE_TestShowState) {
                                ui::SHOW_STATE_NORMAL,
                                BOTH,
                                browser_popup->browser(),
+                               p1600x1200,
                                p1600x1200));
   // The non tabbed window will take the status of the last active of its kind.
   EXPECT_EQ(ui::SHOW_STATE_NORMAL,
@@ -772,6 +782,7 @@ TEST_F(WindowSizerAshTest, MAYBE_TestShowState) {
                                ui::SHOW_STATE_NORMAL,
                                BOTH,
                                browser_popup->browser(),
+                               p1600x1200,
                                p1600x1200));
 
   // Now create a top level window and check again for both. Only the tabbed
@@ -789,6 +800,7 @@ TEST_F(WindowSizerAshTest, MAYBE_TestShowState) {
                                ui::SHOW_STATE_DEFAULT,
                                BOTH,
                                browser_window->browser(),
+                               p1600x1200,
                                p1600x1200));
   // Non tabbed windows should always follow the window saved visibility state.
   EXPECT_EQ(ui::SHOW_STATE_MAXIMIZED,
@@ -796,6 +808,7 @@ TEST_F(WindowSizerAshTest, MAYBE_TestShowState) {
                                ui::SHOW_STATE_MINIMIZED,
                                BOTH,
                                browser_popup->browser(),
+                               p1600x1200,
                                p1600x1200));
 
   // In smaller screen resolutions we default to maximized if there is no other
@@ -808,6 +821,7 @@ TEST_F(WindowSizerAshTest, MAYBE_TestShowState) {
                                  ui::SHOW_STATE_DEFAULT,
                                  BOTH,
                                  browser_window->browser(),
+                                 tiny_screen,
                                  tiny_screen));
     browser_window->Hide();
     EXPECT_EQ(ui::SHOW_STATE_MAXIMIZED,
@@ -815,6 +829,7 @@ TEST_F(WindowSizerAshTest, MAYBE_TestShowState) {
                                  ui::SHOW_STATE_DEFAULT,
                                  BOTH,
                                  browser_window2->browser(),
+                                 tiny_screen,
                                  tiny_screen));
 
   }
@@ -846,6 +861,7 @@ TEST_F(WindowSizerAshTest, TestShowStateDefaults) {
                          ui::SHOW_STATE_MAXIMIZED,
                          DEFAULT,
                          browser_window->browser(),
+                         p1600x1200,
                          p1600x1200);
 #if defined(OS_WIN)
   EXPECT_EQ(window_show_state, ui::SHOW_STATE_MAXIMIZED);
@@ -858,18 +874,21 @@ TEST_F(WindowSizerAshTest, TestShowStateDefaults) {
                                ui::SHOW_STATE_MAXIMIZED,
                                BOTH,
                                browser_window->browser(),
+                               p1600x1200,
                                p1600x1200), ui::SHOW_STATE_MINIMIZED);
   browser_window->browser()->set_initial_show_state(ui::SHOW_STATE_NORMAL);
   EXPECT_EQ(GetWindowShowState(ui::SHOW_STATE_MAXIMIZED,
                                ui::SHOW_STATE_MAXIMIZED,
                                BOTH,
                                browser_window->browser(),
+                               p1600x1200,
                                p1600x1200), ui::SHOW_STATE_NORMAL);
   browser_window->browser()->set_initial_show_state(ui::SHOW_STATE_MAXIMIZED);
   EXPECT_EQ(GetWindowShowState(ui::SHOW_STATE_NORMAL,
                                ui::SHOW_STATE_NORMAL,
                                BOTH,
                                browser_window->browser(),
+                               p1600x1200,
                                p1600x1200), ui::SHOW_STATE_MAXIMIZED);
 
   // Check that setting the maximized command line option is forcing the
@@ -881,6 +900,7 @@ TEST_F(WindowSizerAshTest, TestShowStateDefaults) {
                                ui::SHOW_STATE_NORMAL,
                                BOTH,
                                browser_window->browser(),
+                               p1600x1200,
                                p1600x1200), ui::SHOW_STATE_MAXIMIZED);
 
   // The popup should favor the initial show state over the command line.
@@ -888,10 +908,44 @@ TEST_F(WindowSizerAshTest, TestShowStateDefaults) {
                                ui::SHOW_STATE_NORMAL,
                                BOTH,
                                browser_popup->browser(),
+                               p1600x1200,
                                p1600x1200), ui::SHOW_STATE_NORMAL);
 }
 
-// Test that the target root window is used as the destionation of
+TEST_F(WindowSizerAshTest, DefaultStateBecomesMaximized) {
+  // Create a browser 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));
+
+  gfx::Rect display_bounds = ash::Shell::GetInstance()->GetScreen()->
+      GetPrimaryDisplay().bounds();
+  gfx::Rect specified_bounds = display_bounds;
+
+  // Make a window bigger than the display work area.
+  specified_bounds.Inset(-20, -20);
+  ui::WindowShowState show_state = ui::SHOW_STATE_DEFAULT;
+  gfx::Rect bounds;
+  WindowSizer::GetBrowserWindowBoundsAndShowState(
+      std::string(), specified_bounds, browser.get(), &bounds, &show_state);
+  // The window should start maximized with its restore bounds shrunken.
+  EXPECT_EQ(ui::SHOW_STATE_MAXIMIZED, show_state);
+  EXPECT_NE(display_bounds.ToString(), bounds.ToString());
+  EXPECT_TRUE(display_bounds.Contains(bounds));
+
+  // Make a window smaller than the display work area.
+  specified_bounds.Inset(100, 100);
+  show_state = ui::SHOW_STATE_DEFAULT;
+  WindowSizer::GetBrowserWindowBoundsAndShowState(
+      std::string(), specified_bounds, browser.get(), &bounds, &show_state);
+  // The window should start in default state.
+  EXPECT_EQ(ui::SHOW_STATE_DEFAULT, show_state);
+  EXPECT_EQ(specified_bounds.ToString(), bounds.ToString());
+}
+
+// Test that the target root window is used as the destination of
 // the non browser window. This differ from PersistedBoundsCase
 // in that this uses real ash shell implementations + StateProvider
 // TargetDisplayProvider, rather than mocks.
@@ -946,5 +1000,14 @@ TEST_F(WindowSizerAshTest, TrustedPopupBehavior) {
                                ui::SHOW_STATE_NORMAL,
                                BOTH,
                                trusted_popup->browser(),
+                               p1280x1024,
+                               p1600x1200));
+  // A popup that is sized to occupy the whole work area has default state.
+  EXPECT_EQ(ui::SHOW_STATE_DEFAULT,
+            GetWindowShowState(ui::SHOW_STATE_DEFAULT,
+                               ui::SHOW_STATE_NORMAL,
+                               BOTH,
+                               trusted_popup->browser(),
+                               p1600x1200,
                                p1600x1200));
 }

chrome/browser/ui/window_sizer/window_sizer_common_unittest.cc

@@ -235,14 +235,13 @@ ui::WindowShowState GetWindowShowState(
     ui::WindowShowState show_state_last,
     Source source,
     const Browser* browser,
+    const gfx::Rect& bounds,
     const gfx::Rect& display_config) {
-  gfx::Rect bounds = display_config;
-  gfx::Rect work_area = display_config;
   TestScreen test_screen;
   test_screen.AddDisplay(display_config, display_config);
   scoped_ptr<TestStateProvider> sp(new TestStateProvider);
   if (source == PERSISTED || source == BOTH)
-    sp->SetPersistentState(bounds, work_area, show_state_persisted, true);
+    sp->SetPersistentState(bounds, display_config, show_state_persisted, true);
   if (source == LAST_ACTIVE || source == BOTH)
     sp->SetLastActiveState(bounds, show_state_last, true);
   scoped_ptr<WindowSizer::TargetDisplayProvider> tdp(

chrome/browser/ui/window_sizer/window_sizer_common_unittest.h

@@ -89,7 +89,7 @@ class TestStateProvider : public WindowSizer::StateProvider {
 
 enum Source { DEFAULT, LAST_ACTIVE, PERSISTED, BOTH };
 
-// Set up the window bounds, monitor bounds, show states and more to get the
+// Sets up the window bounds, monitor bounds, show states and more to get the
 // resulting |out_bounds| and |out_show_state| from the WindowSizer.
 // |source| specifies which type of data gets set for the test: Either the
 // last active window, the persisted value which was stored earlier, both or
@@ -108,7 +108,7 @@ void GetWindowBoundsAndShowState(const gfx::Rect& monitor1_bounds,
                                  gfx::Rect* out_bounds,
                                  ui::WindowShowState* out_show_state);
 
-// Set up the window bounds, monitor bounds, and work area to get the
+// Sets up the window bounds, monitor bounds, and work area to get the
 // resulting |out_bounds| from the WindowSizer.
 // |source| specifies which type of data gets set for the test: Either the
 // last active window, the persisted value which was stored earlier, both or
@@ -124,14 +124,16 @@ void GetWindowBounds(const gfx::Rect& monitor1_bounds,
                      const gfx::Rect& passed_in,
                      gfx::Rect* out_bounds);
 
-// Set up the various show states and get the resulting show state from
-// the WindowSizer.
-// The |display_config| is the primary display configuration used.
+// Sets up the window |bounds| and various system states which have an influence
+// on the WindowSizer and then determines the resulting show state from it.
+// |bounds| specifies the |browser| last or persistent bounds depending on
+// |source|. The |display_config| is the primary display configuration used.
 ui::WindowShowState GetWindowShowState(
     ui::WindowShowState show_state_persisted,
     ui::WindowShowState show_state_last,
     Source source,
     const Browser* browser,
+    const gfx::Rect& bounds,
     const gfx::Rect& display_config);
 
 #endif  // CHROME_BROWSER_UI_WINDOW_SIZER_WINDOW_SIZER_COMMON_UNITTEST_H_