Changes maximize/fullscreen to always reset the restore bounds. Doing

otherwise leads to unexpected behavior. I'm also changing double
clicking the caption to go back to restore bounds if set. This too
makes things more consistant.

BUG=122324
TEST=covered by tests
R=jamescook@chromium.org


Review URL: https://chromiumcodereview.appspot.com/10803048

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

8 files changed, 63 insertions, 40 deletions

diff --git a/ash/wm/base_layout_manager.cc b/ash/wm/base_layout_manager.cc
index 33f984b..76e133e 100644
--- a/ash/wm/base_layout_manager.cc
+++ b/ash/wm/base_layout_manager.cc
@@ -133,6 +133,15 @@ void BaseLayoutManager::OnWindowPropertyChanged(aura::Window* window,
                                                 intptr_t old) {
   if (key == aura::client::kShowStateKey) {
     ui::WindowShowState old_state = static_cast<ui::WindowShowState>(old);
+    ui::WindowShowState new_state =
+        window->GetProperty(aura::client::kShowStateKey);
+    if (old_state != new_state &&
+        ((new_state == ui::SHOW_STATE_MAXIMIZED &&
+          old_state != ui::SHOW_STATE_FULLSCREEN) ||
+         (new_state == ui::SHOW_STATE_FULLSCREEN &&
+          old_state != ui::SHOW_STATE_MAXIMIZED))) {
+      SetRestoreBoundsInParent(window, window->bounds());
+    }
     // Minimized state handles its own animations.
     bool animate = (old_state != ui::SHOW_STATE_MINIMIZED);
     UpdateBoundsFromShowState(window, animate);
@@ -191,14 +200,12 @@ void BaseLayoutManager::UpdateBoundsFromShowState(aura::Window* window,
     }
 
     case ui::SHOW_STATE_MAXIMIZED:
-      SetRestoreBoundsIfNotSet(window);
       MaybeAnimateToBounds(window,
                            animate,
                            ScreenAsh::GetMaximizedWindowBoundsInParent(window));
       break;
 
     case ui::SHOW_STATE_FULLSCREEN:
-      SetRestoreBoundsIfNotSet(window);
       // Don't animate the full-screen window transition.
       // TODO(jamescook): Use animation here.  Be sure the lock screen works.
       SetChildBoundsDirect(
diff --git a/ash/wm/base_layout_manager_unittest.cc b/ash/wm/base_layout_manager_unittest.cc
index 47f1306..e02b32a 100644
--- a/ash/wm/base_layout_manager_unittest.cc
+++ b/ash/wm/base_layout_manager_unittest.cc
@@ -8,6 +8,7 @@
 #include "ash/shell.h"
 #include "ash/shell_window_ids.h"
 #include "ash/test/ash_test_base.h"
+#include "ash/wm/property_util.h"
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
 #include "ui/aura/client/aura_constants.h"
@@ -175,6 +176,22 @@ TEST_F(BaseLayoutManagerTest, BoundsWithScreenEdgeVisible) {
   EXPECT_EQ(max_bounds.ToString(), window->bounds().ToString());
 }
 
+// Verifies maximizing always resets the restore bounds, and similarly restoring
+// resets the restore bounds.
+TEST_F(BaseLayoutManagerTest, MaximizeResetsRestoreBounds) {
+  scoped_ptr<aura::Window> window(CreateTestWindow(gfx::Rect(1, 2, 3, 4)));
+  SetRestoreBoundsInParent(window.get(), gfx::Rect(10, 11, 12, 13));
+
+  // Maximize it, which should reset restore bounds.
+  window->SetProperty(aura::client::kShowStateKey, ui::SHOW_STATE_MAXIMIZED);
+  EXPECT_EQ("1,2 3x4", GetRestoreBoundsInParent(window.get()).ToString());
+
+  // Restore it, which should restore bounds and reset restore bounds.
+  window->SetProperty(aura::client::kShowStateKey, ui::SHOW_STATE_NORMAL);
+  EXPECT_EQ("1,2 3x4", window->bounds().ToString());
+  EXPECT_TRUE(GetRestoreBoundsInScreen(window.get()) == NULL);
+}
+
 }  // namespace
 
 }  // namespace ash
diff --git a/ash/wm/property_util.cc b/ash/wm/property_util.cc
index 8453ce3..733d1b1 100644
--- a/ash/wm/property_util.cc
+++ b/ash/wm/property_util.cc
@@ -29,11 +29,6 @@ void SetRestoreBoundsInParent(aura::Window* window, const gfx::Rect& bounds) {
       new gfx::Rect(ScreenAsh::ConvertRectToScreen(window->parent(), bounds)));
 }
 
-void SetRestoreBoundsIfNotSet(aura::Window* window) {
-  if (!GetRestoreBoundsInScreen(window))
-    SetRestoreBoundsInParent(window, window->bounds());
-}
-
 const gfx::Rect* GetRestoreBoundsInScreen(aura::Window* window) {
   return window->GetProperty(aura::client::kRestoreBoundsKey);
 }
@@ -49,12 +44,6 @@ void ClearRestoreBounds(aura::Window* window) {
   window->ClearProperty(aura::client::kRestoreBoundsKey);
 }
 
-void ToggleMaximizedState(aura::Window* window) {
-  window->SetProperty(aura::client::kShowStateKey,
-                      wm::IsWindowMaximized(window) ? ui::SHOW_STATE_NORMAL
-                                                    : ui::SHOW_STATE_MAXIMIZED);
-}
-
 void SetTrackedByWorkspace(aura::Window* window, bool value) {
   window->SetProperty(internal::kWindowTrackedByWorkspaceKey, value);
 }
diff --git a/ash/wm/property_util.h b/ash/wm/property_util.h
index 75adf0a..bb8e431 100644
--- a/ash/wm/property_util.h
+++ b/ash/wm/property_util.h
@@ -30,10 +30,6 @@ ASH_EXPORT void SetRestoreBoundsInScreen(aura::Window* window,
 ASH_EXPORT void SetRestoreBoundsInParent(aura::Window* window,
                                          const gfx::Rect& parent_bounds);
 
-// Same as SetRestoreBounds(), but does nothing if the restore bounds have
-// already been set. The bounds used are the bounds of the window.
-ASH_EXPORT void SetRestoreBoundsIfNotSet(aura::Window* window);
-
 // Returns the restore bounds property on |window| in the virtual screen
 // coordinates. The bounds can be NULL if the bounds property does not
 // exist for |window|. |window| owns the bounds object.
@@ -45,9 +41,6 @@ ASH_EXPORT gfx::Rect GetRestoreBoundsInParent(aura::Window* window);
 // Deletes and clears the restore bounds property on |window|.
 ASH_EXPORT void ClearRestoreBounds(aura::Window* window);
 
-// Toggles the maximized state of the specified window.
-ASH_EXPORT void ToggleMaximizedState(aura::Window* window);
-
 enum WindowPersistsAcrossAllWorkspacesType {
   WINDOW_PERSISTS_ACROSS_ALL_WORKSPACES_VALUE_DEFAULT,
   WINDOW_PERSISTS_ACROSS_ALL_WORKSPACES_VALUE_NO,
diff --git a/ash/wm/toplevel_window_event_filter.cc b/ash/wm/toplevel_window_event_filter.cc
index 43abc80..c950036 100644
--- a/ash/wm/toplevel_window_event_filter.cc
+++ b/ash/wm/toplevel_window_event_filter.cc
@@ -87,8 +87,6 @@ bool ToplevelWindowEventFilter::PreHandleMouseEvent(aura::Window* target,
       } else {
         window_resizer_.reset();
       }
-      if (component == HTCAPTION && event->flags() & ui::EF_IS_DOUBLE_CLICK)
-        ToggleMaximizedState(target);
       return WindowResizer::GetBoundsChangeForWindowComponent(component) != 0;
     }
     case ui::ET_MOUSE_DRAGGED:
diff --git a/ash/wm/toplevel_window_event_filter_unittest.cc b/ash/wm/toplevel_window_event_filter_unittest.cc
index ca9aeeb..5240527 100644
--- a/ash/wm/toplevel_window_event_filter_unittest.cc
+++ b/ash/wm/toplevel_window_event_filter_unittest.cc
@@ -334,19 +334,6 @@ TEST_F(ToplevelWindowEventFilterTest, BottomRightPastMinimum) {
   EXPECT_EQ(gfx::Size(40, 40), w1->bounds().size());
 }
 
-TEST_F(ToplevelWindowEventFilterTest, DoubleClickCaptionTogglesMaximize) {
-  scoped_ptr<aura::Window> w1(CreateWindow(HTCAPTION));
-  EXPECT_FALSE(wm::IsWindowMaximized(w1.get()));
-
-  aura::test::EventGenerator generator(Shell::GetPrimaryRootWindow(), w1.get());
-  generator.DoubleClickLeftButton();
-
-  EXPECT_TRUE(wm::IsWindowMaximized(w1.get()));
-  generator.DoubleClickLeftButton();
-
-  EXPECT_FALSE(wm::IsWindowMaximized(w1.get()));
-}
-
 TEST_F(ToplevelWindowEventFilterTest, BottomRightWorkArea) {
   scoped_ptr<aura::Window> target(CreateWindow(HTBOTTOMRIGHT));
   gfx::Rect work_area =
diff --git a/ash/wm/workspace/workspace_event_filter.cc b/ash/wm/workspace/workspace_event_filter.cc
index 564e176..ae9459c 100644
--- a/ash/wm/workspace/workspace_event_filter.cc
+++ b/ash/wm/workspace/workspace_event_filter.cc
@@ -50,6 +50,19 @@ void SingleAxisUnmaximize(aura::Window* window,
   window->ClearProperty(aura::client::kRestoreBoundsKey);
 }
 
+void ToggleMaximizedState(aura::Window* window) {
+  if (GetRestoreBoundsInScreen(window)) {
+    if (window->GetProperty(aura::client::kShowStateKey) ==
+        ui::SHOW_STATE_NORMAL) {
+      window->SetBounds(GetRestoreBoundsInParent(window));
+    } else {
+      window->SetProperty(aura::client::kShowStateKey, ui::SHOW_STATE_NORMAL);
+    }
+  } else {
+    window->SetProperty(aura::client::kShowStateKey, ui::SHOW_STATE_MAXIMIZED);
+  }
+}
+
 }  // namespace
 
 namespace internal {
@@ -83,6 +96,10 @@ bool WorkspaceEventFilter::PreHandleMouseEvent(aura::Window* target,
       UpdateHoveredWindow(NULL);
       break;
     case ui::ET_MOUSE_PRESSED:
+      if (event->flags() & ui::EF_IS_DOUBLE_CLICK &&
+          target->delegate()->GetNonClientComponent(event->location()) ==
+          HTCAPTION)
+        ToggleMaximizedState(target);
       multi_window_resize_controller_.Hide();
       HandleVerticalResizeDoubleClick(target, event);
       break;
diff --git a/ash/wm/workspace/workspace_event_filter_unittest.cc b/ash/wm/workspace/workspace_event_filter_unittest.cc
index 00c2b44..3965e93 100644
--- a/ash/wm/workspace/workspace_event_filter_unittest.cc
+++ b/ash/wm/workspace/workspace_event_filter_unittest.cc
@@ -79,15 +79,12 @@ TEST_F(WorkspaceEventFilterTest, DoubleClickSingleAxisResizeEdge) {
   EXPECT_EQ(restored_bounds.height(), window->bounds().height());
 
   // Double-click the top resize edge again to maximize vertically, then double
-  // click the caption to maximize in all directions.
+  // click again to restore.
   generator.DoubleClickLeftButton();
   wd.set_window_component(HTCAPTION);
   generator.DoubleClickLeftButton();
-  EXPECT_TRUE(wm::IsWindowMaximized(window.get()));
-
-  // Double clicking the caption again should completely restore the window.
-  generator.DoubleClickLeftButton();
   EXPECT_FALSE(wm::IsWindowMaximized(window.get()));
+
   EXPECT_EQ(restored_bounds.y(), window->bounds().y());
   EXPECT_EQ(restored_bounds.height(), window->bounds().height());
 
@@ -100,5 +97,23 @@ TEST_F(WorkspaceEventFilterTest, DoubleClickSingleAxisResizeEdge) {
   EXPECT_FALSE(wm::IsWindowMaximized(window.get()));
 }
 
+TEST_F(WorkspaceEventFilterTest, DoubleClickCaptionTogglesMaximize) {
+  aura::test::TestWindowDelegate wd;
+  scoped_ptr<aura::Window> window(CreateTestWindow(&wd, gfx::Rect(1, 2, 3, 4)));
+  wd.set_window_component(HTCAPTION);
+  EXPECT_FALSE(wm::IsWindowMaximized(window.get()));
+  aura::test::EventGenerator generator(Shell::GetPrimaryRootWindow(),
+                                       window.get());
+  generator.DoubleClickLeftButton();
+  EXPECT_NE("1,2 3x4", window->bounds().ToString());
+
+  EXPECT_TRUE(wm::IsWindowMaximized(window.get()));
+  generator.DoubleClickLeftButton();
+
+  EXPECT_FALSE(wm::IsWindowMaximized(window.get()));
+  EXPECT_EQ("1,2 3x4", window->bounds().ToString());
+}
+
+
 }  // namespace internal
 }  // namespace ash