Fixes bugs that resulted in problems with double clicking to maximize

vertically/horizontally:
. Workspace was initially setting a restore bounds even if the window
  was normal. This confused the maximize logic. To fix this I've
  simplified the workspace code so that it uses BaseLayoutManager to
  manage the restore bounds.
. Under X a double click results in press-drag-release,
  press-drag-release. The second press starts a window drag
  resizer as well as maximizing. This means that when we press the
  drag we end up trying to move the window and things snap around.
. Don't toggle size if already maximized.

BUG=117201
TEST=covered by unit tests
R=ben@chromium.org

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

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

11 files changed, 52 insertions, 123 deletions

diff --git a/ash/wm/base_layout_manager.h b/ash/wm/base_layout_manager.h
index 8edbe3d..79fd1fe 100644
--- a/ash/wm/base_layout_manager.h
+++ b/ash/wm/base_layout_manager.h
@@ -49,8 +49,6 @@ class ASH_EXPORT BaseLayoutManager : public aura::LayoutManager,
 
   // RootWindowObserver overrides:
   virtual void OnRootWindowResized(const gfx::Size& new_size) OVERRIDE;
-
-  // RootWindowObserver overrides:
   virtual void OnScreenWorkAreaInsetsChanged() OVERRIDE;
 
   // WindowObserver overrides:
diff --git a/ash/wm/toplevel_window_event_filter.cc b/ash/wm/toplevel_window_event_filter.cc
index 7b98b51..c2266e4 100644
--- a/ash/wm/toplevel_window_event_filter.cc
+++ b/ash/wm/toplevel_window_event_filter.cc
@@ -62,7 +62,9 @@ bool ToplevelWindowEventFilter::PreHandleMouseEvent(aura::Window* target,
       // pressed without mouse move event.
       int component =
           target->delegate()->GetNonClientComponent(event->location());
-      if (WindowResizer::GetBoundsChangeForWindowComponent(component)) {
+      if ((event->flags() &
+           (ui::EF_IS_DOUBLE_CLICK | ui::EF_IS_TRIPLE_CLICK)) == 0 &&
+          WindowResizer::GetBoundsChangeForWindowComponent(component)) {
         gfx::Point parent_location(
             ConvertPointToParent(target, event->location()));
         window_resizer_.reset(
diff --git a/ash/wm/workspace/workspace_event_filter.cc b/ash/wm/workspace/workspace_event_filter.cc
index a137b15..84046b9 100644
--- a/ash/wm/workspace/workspace_event_filter.cc
+++ b/ash/wm/workspace/workspace_event_filter.cc
@@ -123,7 +123,8 @@ void WorkspaceEventFilter::UpdateHoveredWindow(
 void WorkspaceEventFilter::HandleVerticalResizeDoubleClick(
     aura::Window* target,
     aura::MouseEvent* event) {
-  if (event->flags() & ui::EF_IS_DOUBLE_CLICK) {
+  if (event->flags() & ui::EF_IS_DOUBLE_CLICK &&
+      !wm::IsWindowMaximized(target)) {
     int component =
         target->delegate()->GetNonClientComponent(event->location());
     gfx::Rect work_area =
diff --git a/ash/wm/workspace/workspace_event_filter_unittest.cc b/ash/wm/workspace/workspace_event_filter_unittest.cc
index 331b5ee..06e5762 100644
--- a/ash/wm/workspace/workspace_event_filter_unittest.cc
+++ b/ash/wm/workspace/workspace_event_filter_unittest.cc
@@ -58,7 +58,15 @@ TEST_F(WorkspaceEventFilterTest, DoubleClickSingleAxisResizeEdge) {
 
   // Double-click the top resize edge.
   wd.set_window_component(HTTOP);
-  generator.DoubleClickLeftButton();
+  // On X a double click actually generates a drag between each press/release.
+  // Explicitly trigger this path since we had bugs in dealing with it
+  // correctly.
+  generator.PressLeftButton();
+  generator.ReleaseLeftButton();
+  generator.set_flags(ui::EF_IS_DOUBLE_CLICK);
+  generator.PressLeftButton();
+  generator.MoveMouseTo(generator.current_location(), 1);
+  generator.ReleaseLeftButton();
   EXPECT_EQ(work_area.y(), window->bounds().y());
   EXPECT_EQ(work_area.height(), window->bounds().height());
   // Single-axis maximization is not considered real maximization.
diff --git a/ash/wm/workspace/workspace_layout_manager.cc b/ash/wm/workspace/workspace_layout_manager.cc
index b1f8f66..52c50c4 100644
--- a/ash/wm/workspace/workspace_layout_manager.cc
+++ b/ash/wm/workspace/workspace_layout_manager.cc
@@ -31,30 +31,20 @@ WorkspaceLayoutManager::WorkspaceLayoutManager(
 }
 
 WorkspaceLayoutManager::~WorkspaceLayoutManager() {
-  const aura::Window::Windows& windows(
-      workspace_manager_->contents_view()->children());
-  for (size_t i = 0; i < windows.size(); ++i)
-    windows[i]->RemoveObserver(this);
 }
 
 void WorkspaceLayoutManager::OnWindowResized() {
-  // Workspace is updated via RootWindowObserver::OnRootWindowResized.
+  // Workspace is updated via OnRootWindowResized.
 }
 
 void WorkspaceLayoutManager::OnWindowAddedToLayout(aura::Window* child) {
-  child->AddObserver(this);
+  BaseLayoutManager::OnWindowAddedToLayout(child);
   if (!workspace_manager_->IsManagedWindow(child))
     return;
 
   if (child->IsVisible()) {
     workspace_manager_->AddWindow(child);
-  } else if (wm::IsWindowMaximized(child)) {
-    SetChildBoundsDirect(child,
-                         gfx::Screen::GetMonitorWorkAreaNearestWindow(child));
-  } else if (wm::IsWindowFullscreen(child)) {
-    SetChildBoundsDirect(child,
-                         gfx::Screen::GetMonitorAreaNearestWindow(child));
-  } else {
+  } else if (wm::IsWindowNormal(child)) {
     // Align non-maximized/fullscreen windows to a grid.
     SetChildBoundsDirect(
         child, workspace_manager_->AlignBoundsToGrid(child->GetTargetBounds()));
@@ -63,9 +53,8 @@ void WorkspaceLayoutManager::OnWindowAddedToLayout(aura::Window* child) {
 
 void WorkspaceLayoutManager::OnWillRemoveWindowFromLayout(
     aura::Window* child) {
-  child->RemoveObserver(this);
-  ClearRestoreBounds(child);
   workspace_manager_->RemoveWindow(child);
+  BaseLayoutManager::OnWillRemoveWindowFromLayout(child);
 }
 
 void WorkspaceLayoutManager::OnChildWindowVisibilityChanged(
@@ -82,25 +71,28 @@ void WorkspaceLayoutManager::OnChildWindowVisibilityChanged(
 void WorkspaceLayoutManager::SetChildBounds(
     aura::Window* child,
     const gfx::Rect& requested_bounds) {
-  gfx::Rect child_bounds(requested_bounds);
-  if (GetTrackedByWorkspace(child)) {
-    if (wm::IsWindowMaximized(child)) {
-      child_bounds = gfx::Screen::GetMonitorWorkAreaNearestWindow(child);
-    } else if (wm::IsWindowFullscreen(child)) {
-      child_bounds = gfx::Screen::GetMonitorAreaNearestWindow(child);
-    } else {
-      child_bounds = gfx::Screen::GetMonitorWorkAreaNearestWindow(child).
-          AdjustToFit(requested_bounds);
-    }
-  }
-  SetChildBoundsDirect(child, child_bounds);
+  if (GetTrackedByWorkspace(child))
+    BaseLayoutManager::SetChildBounds(child, requested_bounds);
+  else
+    SetChildBoundsDirect(child, requested_bounds);
+}
+
+void WorkspaceLayoutManager::OnRootWindowResized(const gfx::Size& new_size) {
+  workspace_manager_->SetWorkspaceSize(new_size);
+}
+
+void WorkspaceLayoutManager::OnScreenWorkAreaInsetsChanged() {
+  workspace_manager_->OnScreenWorkAreaInsetsChanged();
 }
 
 void WorkspaceLayoutManager::OnWindowPropertyChanged(aura::Window* window,
                                                      const void* key,
                                                      intptr_t old) {
-  if (key == ash::kWindowTrackedByWorkspaceSplitPropKey &&
-      ash::GetTrackedByWorkspace(window)) {
+  BaseLayoutManager::OnWindowPropertyChanged(window, key, old);
+  if (key == aura::client::kShowStateKey) {
+    workspace_manager_->ShowStateChanged(window);
+  } else if (key == ash::kWindowTrackedByWorkspaceSplitPropKey &&
+             ash::GetTrackedByWorkspace(window)) {
     // We currently don't need to support transitioning from true to false, so
     // we ignore it.
     workspace_manager_->AddWindow(window);
diff --git a/ash/wm/workspace/workspace_layout_manager.h b/ash/wm/workspace/workspace_layout_manager.h
index 001a06c..8ea5135 100644
--- a/ash/wm/workspace/workspace_layout_manager.h
+++ b/ash/wm/workspace/workspace_layout_manager.h
@@ -6,7 +6,7 @@
 #define ASH_WM_WORKSPACE_WORKSPACE_LAYOUT_MANAGER_H_
 #pragma once
 
-#include "ash/ash_export.h"
+#include "ash/wm/base_layout_manager.h"
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
 #include "ui/aura/layout_manager.h"
@@ -27,8 +27,7 @@ namespace internal {
 class WorkspaceManager;
 
 // LayoutManager for top level windows when WorkspaceManager is enabled.
-class ASH_EXPORT WorkspaceLayoutManager : public aura::LayoutManager,
-                                          public aura::WindowObserver {
+class ASH_EXPORT WorkspaceLayoutManager : public BaseLayoutManager {
  public:
   explicit WorkspaceLayoutManager(WorkspaceManager* workspace_manager);
   virtual ~WorkspaceLayoutManager();
@@ -38,7 +37,7 @@ class ASH_EXPORT WorkspaceLayoutManager : public aura::LayoutManager,
     return workspace_manager_;
   }
 
-  // Overridden from aura::LayoutManager:
+  // Overridden from BaseLayoutManager:
   virtual void OnWindowResized() OVERRIDE;
   virtual void OnWindowAddedToLayout(aura::Window* child) OVERRIDE;
   virtual void OnWillRemoveWindowFromLayout(aura::Window* child) OVERRIDE;
@@ -46,6 +45,8 @@ class ASH_EXPORT WorkspaceLayoutManager : public aura::LayoutManager,
                                               bool visibile) OVERRIDE;
   virtual void SetChildBounds(aura::Window* child,
                               const gfx::Rect& requested_bounds) OVERRIDE;
+  virtual void OnRootWindowResized(const gfx::Size& new_size) OVERRIDE;
+  virtual void OnScreenWorkAreaInsetsChanged() OVERRIDE;
 
   // Overriden from aura::WindowObserver:
   virtual void OnWindowPropertyChanged(aura::Window* window,
diff --git a/ash/wm/workspace/workspace_manager.cc b/ash/wm/workspace/workspace_manager.cc
index 8fe68ca..caf7d93 100644
--- a/ash/wm/workspace/workspace_manager.cc
+++ b/ash/wm/workspace/workspace_manager.cc
@@ -77,11 +77,6 @@ WorkspaceManager::WorkspaceManager(aura::Window* contents_view)
 }
 
 WorkspaceManager::~WorkspaceManager() {
-  for (size_t i = 0; i < workspaces_.size(); ++i) {
-    Workspace* workspace = workspaces_[i];
-    for (size_t j = 0; j < workspace->windows().size(); ++j)
-      workspace->windows()[j]->RemoveObserver(this);
-  }
   std::vector<Workspace*> copy_to_delete(workspaces_);
   STLDeleteElements(&copy_to_delete);
 }
@@ -105,22 +100,11 @@ void WorkspaceManager::AddWindow(aura::Window* window) {
     return;
   }
 
-  if (!wm::IsWindowMaximized(window) && !wm::IsWindowFullscreen(window)) {
-    SetRestoreBounds(window, window->bounds());
-  }
-
   if (!window->GetProperty(aura::client::kShowStateKey))
     window->SetProperty(aura::client::kShowStateKey, ui::SHOW_STATE_NORMAL);
 
-  if (wm::IsWindowMaximized(window) || wm::IsWindowFullscreen(window)) {
-    SetFullScreenOrMaximizedBounds(window);
-  } else {
-    if (grid_size_ > 1)
-      SetWindowBounds(window, AlignBoundsToGrid(window->GetTargetBounds()));
-  }
-
-  // Add the observer after we change the state in anyway.
-  window->AddObserver(this);
+  if (wm::IsWindowNormal(window) && grid_size_ > 1)
+    SetWindowBounds(window, AlignBoundsToGrid(window->GetTargetBounds()));
 
   Workspace* workspace = NULL;
   Workspace::Type type_for_window = Workspace::TypeForWindow(window);
@@ -145,7 +129,6 @@ void WorkspaceManager::RemoveWindow(aura::Window* window) {
   Workspace* workspace = FindBy(window);
   if (!workspace)
     return;
-  window->RemoveObserver(this);
   workspace->RemoveWindow(window);
   if (workspace->is_empty())
     delete workspace;
@@ -184,23 +167,14 @@ gfx::Rect WorkspaceManager::AlignBoundsToGrid(const gfx::Rect& bounds) {
   return AlignRectToGrid(bounds, grid_size_);
 }
 
-void WorkspaceManager::OnWindowPropertyChanged(aura::Window* window,
-                                               const void* key,
-                                               intptr_t old) {
-  if (key != aura::client::kShowStateKey || !IsManagedWindow(window))
+void WorkspaceManager::ShowStateChanged(aura::Window* window) {
+  if (!IsManagedWindow(window) || !FindBy(window))
     return;
 
-  DCHECK(FindBy(window));
-
   Workspace::Type old_type = FindBy(window)->type();
   Workspace::Type new_type = Workspace::TypeForWindow(window);
-  if (new_type != old_type) {
+  if (new_type != old_type)
     OnTypeOfWorkspacedNeededChanged(window);
-  } else if (new_type == Workspace::TYPE_MAXIMIZED) {
-    // Even though the type didn't change, the window may have gone from
-    // maximized to fullscreen. Adjust the bounds appropriately.
-    SetFullScreenOrMaximizedBounds(window);
-  }
   UpdateShelfVisibility();
 }
 
@@ -336,26 +310,6 @@ void WorkspaceManager::SetWindowBounds(aura::Window* window,
   ignored_window_ = NULL;
 }
 
-void WorkspaceManager::SetWindowBoundsFromRestoreBounds(aura::Window* window) {
-  const gfx::Rect* restore = GetRestoreBounds(window);
-  gfx::Rect bounds;
-  if (restore)
-    bounds = restore->AdjustToFit(GetWorkAreaBounds());
-  else
-    bounds = window->bounds().AdjustToFit(GetWorkAreaBounds());
-  SetWindowBounds(window, bounds);
-  ash::ClearRestoreBounds(window);
-}
-
-void WorkspaceManager::SetFullScreenOrMaximizedBounds(aura::Window* window) {
-  if (!GetRestoreBounds(window))
-    SetRestoreBounds(window, window->GetTargetBounds());
-  if (wm::IsWindowMaximized(window))
-    SetWindowBounds(window, GetWorkAreaBounds());
-  else if (wm::IsWindowFullscreen(window))
-    SetWindowBounds(window, gfx::Screen::GetMonitorAreaNearestWindow(window));
-}
-
 void WorkspaceManager::SetWorkspaceBounds() {
   for (Workspaces::const_iterator i = workspaces_.begin();
        i != workspaces_.end(); ++i) {
@@ -370,11 +324,9 @@ void WorkspaceManager::OnTypeOfWorkspacedNeededChanged(aura::Window* window) {
   Workspace* new_workspace = NULL;
   if (Workspace::TypeForWindow(window) == Workspace::TYPE_MAXIMIZED) {
     // Unmaximized -> maximized; create a new workspace.
-    SetRestoreBoundsIfNotSet(window);
     current_workspace->RemoveWindow(window);
     new_workspace = CreateWorkspace(Workspace::TYPE_MAXIMIZED);
     new_workspace->AddWindowAfter(window, NULL);
-    SetFullScreenOrMaximizedBounds(window);
   } else {
     // Maximized -> unmaximized; move window to unmaximized workspace.
     wm::SetOpenWindowSplit(window, false);
@@ -382,7 +334,6 @@ void WorkspaceManager::OnTypeOfWorkspacedNeededChanged(aura::Window* window) {
     current_workspace->RemoveWindow(window);
     if (!new_workspace)
       new_workspace = CreateWorkspace(Workspace::TYPE_MANAGED);
-    SetWindowBoundsFromRestoreBounds(window);
     new_workspace->AddWindowAfter(window, NULL);
   }
   SetActiveWorkspace(new_workspace);
diff --git a/ash/wm/workspace/workspace_manager.h b/ash/wm/workspace/workspace_manager.h
index 3106e77..00715cd 100644
--- a/ash/wm/workspace/workspace_manager.h
+++ b/ash/wm/workspace/workspace_manager.h
@@ -11,7 +11,6 @@
 #include "ash/wm/workspace/workspace.h"
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
-#include "ui/aura/window_observer.h"
 #include "ui/gfx/insets.h"
 #include "ui/gfx/size.h"
 
@@ -31,7 +30,7 @@ class ShelfLayoutManager;
 class WorkspaceManagerTest;
 
 // WorkspaceManager manages multiple workspaces in the desktop.
-class ASH_EXPORT WorkspaceManager : public aura::WindowObserver {
+class ASH_EXPORT WorkspaceManager {
  public:
   explicit WorkspaceManager(aura::Window* viewport);
   virtual ~WorkspaceManager();
@@ -81,10 +80,8 @@ class ASH_EXPORT WorkspaceManager : public aura::WindowObserver {
 
   void set_shelf(ShelfLayoutManager* shelf) { shelf_ = shelf; }
 
-  // Overriden from aura::WindowObserver:
-  virtual void OnWindowPropertyChanged(aura::Window* window,
-                                       const void* key,
-                                       intptr_t old) OVERRIDE;
+  // Invoked when the show state of the specified window changes.
+  void ShowStateChanged(aura::Window* window);
 
  private:
   // Enumeration of whether windows should animate or not.
@@ -135,13 +132,6 @@ class ASH_EXPORT WorkspaceManager : public aura::WindowObserver {
   // that the bounds change is allowed through.
   void SetWindowBounds(aura::Window* window, const gfx::Rect& bounds);
 
-  // Resets the bounds of |window| to its restored bounds (if set), ensuring
-  // it fits in the the windows current workspace.
-  void SetWindowBoundsFromRestoreBounds(aura::Window* window);
-
-  // Reset the bounds of |window|. Use when |window| is fullscreen or maximized.
-  void SetFullScreenOrMaximizedBounds(aura::Window* window);
-
   // Sets the bounds of all workspaces.
   void SetWorkspaceBounds();
 
diff --git a/ash/wm/workspace/workspace_manager_unittest.cc b/ash/wm/workspace/workspace_manager_unittest.cc
index 4969ab3..a9312ca 100644
--- a/ash/wm/workspace/workspace_manager_unittest.cc
+++ b/ash/wm/workspace/workspace_manager_unittest.cc
@@ -101,8 +101,12 @@ TEST_F(WorkspaceManagerTest, AddNormalWindowWhenEmpty) {
   ASSERT_TRUE(manager_->IsManagedWindow(w1.get()));
   EXPECT_FALSE(FindBy(w1.get()));
 
+  EXPECT_TRUE(GetRestoreBounds(w1.get()) == NULL);
+
   w1->Show();
 
+  EXPECT_TRUE(GetRestoreBounds(w1.get()) == NULL);
+
   ASSERT_TRUE(w1->layer() != NULL);
   EXPECT_TRUE(w1->layer()->visible());
 
diff --git a/ash/wm/workspace_controller.cc b/ash/wm/workspace_controller.cc
index d13379d..f12eafc 100644
--- a/ash/wm/workspace_controller.cc
+++ b/ash/wm/workspace_controller.cc
@@ -36,7 +36,6 @@ WorkspaceController::WorkspaceController(aura::Window* viewport)
   viewport->SetEventFilter(event_filter_);
   layout_manager_ = new WorkspaceLayoutManager(workspace_manager_.get());
   viewport->SetLayoutManager(layout_manager_);
-  Shell::GetRootWindow()->AddRootWindowObserver(this);
   Shell::GetRootWindow()->AddObserver(this);
   workspace_manager_->set_grid_size(kGridSize);
   event_filter_->set_grid_size(kGridSize);
@@ -44,7 +43,6 @@ WorkspaceController::WorkspaceController(aura::Window* viewport)
 
 WorkspaceController::~WorkspaceController() {
   Shell::GetRootWindow()->RemoveObserver(this);
-  Shell::GetRootWindow()->RemoveRootWindowObserver(this);
   // WorkspaceLayoutManager may attempt to access state from us. Destroy it now.
   if (viewport_->layout_manager() == layout_manager_)
     viewport_->SetLayoutManager(NULL);
@@ -72,14 +70,6 @@ void WorkspaceController::ShowMenu(views::Widget* widget,
 #endif  // !defined(OS_MACOSX)
 }
 
-void WorkspaceController::OnRootWindowResized(const gfx::Size& new_size) {
-  workspace_manager_->SetWorkspaceSize(new_size);
-}
-
-void WorkspaceController::OnScreenWorkAreaInsetsChanged() {
-  workspace_manager_->OnScreenWorkAreaInsetsChanged();
-}
-
 void WorkspaceController::OnWindowPropertyChanged(aura::Window* window,
                                                   const void* key,
                                                   intptr_t old) {
diff --git a/ash/wm/workspace_controller.h b/ash/wm/workspace_controller.h
index fb45cc2..9cffc2f 100644
--- a/ash/wm/workspace_controller.h
+++ b/ash/wm/workspace_controller.h
@@ -9,7 +9,6 @@
 #include "ash/ash_export.h"
 #include "base/basictypes.h"
 #include "base/memory/scoped_ptr.h"
-#include "ui/aura/root_window_observer.h"
 #include "ui/aura/window_observer.h"
 #include "ui/base/models/simple_menu_model.h"
 
@@ -38,7 +37,6 @@ class WorkspaceManager;
 // various workspace pieces: WorkspaceManager, WorkspaceLayoutManager and
 // WorkspaceEventFilter.
 class ASH_EXPORT WorkspaceController :
-      public aura::RootWindowObserver,
       public aura::WindowObserver,
       public ui::SimpleMenuModel::Delegate {
  public:
@@ -55,12 +53,6 @@ class ASH_EXPORT WorkspaceController :
   // Shows the menu allowing you to configure various aspects of workspaces.
   void ShowMenu(views::Widget* widget, const gfx::Point& location);
 
-  // aura::RootWindowObserver overrides:
-  virtual void OnRootWindowResized(const gfx::Size& new_size) OVERRIDE;
-
-  // aura::RootWindowObserver overrides:
-  virtual void OnScreenWorkAreaInsetsChanged() OVERRIDE;
-
   // aura::WindowObserver overrides:
   virtual void OnWindowPropertyChanged(aura::Window* window,
                                        const void* key,