Makes it so we don't allow detaching dragged tabs into a separate

window when in ash, the window is maximized and all the tabs are
selected.

BUG=164338
TEST=see bug
R=jamescook@chromium.org

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

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

3 files changed, 127 insertions, 9 deletions

diff --git a/ash/wm/workspace/workspace.cc b/ash/wm/workspace/workspace.cc
index a573cc2c..6513dea 100644
--- a/ash/wm/workspace/workspace.cc
+++ b/ash/wm/workspace/workspace.cc
@@ -73,19 +73,23 @@ bool Workspace::ShouldMoveToPending() const {
   if (!is_maximized_)
     return false;
 
-  bool has_visible_non_maximized_window = false;
   for (size_t i = 0; i < window_->children().size(); ++i) {
     aura::Window* child(window_->children()[i]);
-    if (!GetTrackedByWorkspace(child) || !child->TargetVisibility() ||
-        wm::IsWindowMinimized(child))
+    if (!child->TargetVisibility() || wm::IsWindowMinimized(child))
       continue;
-    if (WorkspaceManager::IsMaximized(child))
-      return false;
 
-    if (GetTrackedByWorkspace(child) && !GetPersistsAcrossAllWorkspaces(child))
-      has_visible_non_maximized_window = true;
+    if (!GetTrackedByWorkspace(child)) {
+      // If we have a maximized window that isn't tracked don't move to
+      // pending. This handles the case of dragging a maximized window.
+      if (WorkspaceManager::IsMaximized(child))
+        return false;
+      continue;
+    }
+
+    if (!GetPersistsAcrossAllWorkspaces(child))
+      return false;
   }
-  return !has_visible_non_maximized_window;
+  return true;
 }
 
 int Workspace::GetNumMaximizedWindows() const {
diff --git a/ash/wm/workspace/workspace_manager.cc b/ash/wm/workspace/workspace_manager.cc
index 9bc17a7..32899bf 100644
--- a/ash/wm/workspace/workspace_manager.cc
+++ b/ash/wm/workspace/workspace_manager.cc
@@ -526,7 +526,8 @@ void WorkspaceManager::FadeDesktop(aura::Window* window,
     direction = DesktopBackgroundFadeController::FADE_OUT;
     parent = contents_view_;
     stack_above = desktop_workspace()->window();
-    DCHECK_EQ(kCrossFadeSwitchTimeMS, (int)duration.InMilliseconds());
+    DCHECK_EQ(kCrossFadeSwitchTimeMS,
+              static_cast<int>(duration.InMilliseconds()));
     duration = base::TimeDelta::FromMilliseconds(kCrossFadeSwitchTimeMS);
   }
   desktop_fade_controller_.reset(
@@ -784,6 +785,17 @@ void WorkspaceManager::OnTrackedByWorkspaceChanged(Workspace* workspace,
                                                    aura::Window* window) {
   Workspace* new_workspace = NULL;
   if (IsMaximized(window)) {
+    if (workspace->is_maximized() && workspace->GetNumMaximizedWindows() == 1) {
+      // If |window| is the only window in a maximized workspace then leave
+      // it there. Additionally animate it back to the origin.
+      ui::ScopedLayerAnimationSettings settings(window->layer()->GetAnimator());
+      // All bounds changes get routed through WorkspaceLayoutManager and since
+      // the window is maximized WorkspaceLayoutManager is going to force a
+      // value. In other words, it doesn't matter what we supply to SetBounds()
+      // here.
+      window->SetBounds(gfx::Rect());
+      return;
+    }
     new_workspace = CreateWorkspace(true);
     pending_workspaces_.insert(new_workspace);
   } else if (workspace->is_maximized()) {
diff --git a/ash/wm/workspace/workspace_manager_unittest.cc b/ash/wm/workspace/workspace_manager_unittest.cc
index b4d9f7a..dd9dcd0 100644
--- a/ash/wm/workspace/workspace_manager_unittest.cc
+++ b/ash/wm/workspace/workspace_manager_unittest.cc
@@ -28,6 +28,7 @@
 #include "ui/aura/test/test_window_delegate.h"
 #include "ui/aura/test/test_windows.h"
 #include "ui/aura/window.h"
+#include "ui/base/hit_test.h"
 #include "ui/base/ui_base_types.h"
 #include "ui/compositor/layer.h"
 #include "ui/compositor/scoped_animation_duration_scale_mode.h"
@@ -106,6 +107,20 @@ class WorkspaceManagerTest : public test::AshTestBase {
         manager_->workspaces_.begin());
   }
 
+  // Returns a string description of the current state. The string has the
+  // following format:
+  // W* P=W* active=N
+  // Each W corresponds to a workspace. Each workspace is prefixed with an 'M'
+  // if the workspace is maximized and is followed by the number of windows in
+  // the workspace.
+  // 'P=' is used for the pending workspaces (see
+  // WorkspaceManager::pending_workspaces_ for details on pending workspaces).
+  // N is the index of the active workspace (index into
+  // WorkspaceManager::workspaces_).
+  // For example, '2 M1 P=M1 active=1' means the first workspace (the desktop)
+  // has 2 windows, the second workspace is a maximized workspace with 1 window,
+  // there is a pending maximized workspace with 1 window and the second
+  // workspace is active.
   std::string StateString() {
     std::string result;
     for (size_t i = 0; i < manager_->workspaces_.size(); ++i) {
@@ -1447,5 +1462,92 @@ TEST_F(WorkspaceManagerTest, AnimatedNormToMaxToNormRepositionsRemaining) {
   EXPECT_EQ("0,48 256x512", window2->bounds().ToString());
 }
 
+namespace {
+
+// Used by DragMaximizedNonTrackedWindow to track how many times the window
+// hierarchy changes.
+class DragMaximizedNonTrackedWindowObserver
+    : public aura::WindowObserver {
+ public:
+  DragMaximizedNonTrackedWindowObserver() : change_count_(0) {
+  }
+
+  // Number of times OnWindowHierarchyChanged() has been received.
+  void clear_change_count() { change_count_ = 0; }
+  int change_count() const {
+    return change_count_;
+  }
+
+  // aura::WindowObserver overrides:
+  virtual void OnWindowHierarchyChanged(
+      const HierarchyChangeParams& params) OVERRIDE {
+    change_count_++;
+  }
+
+ private:
+  int change_count_;
+
+  DISALLOW_COPY_AND_ASSIGN(DragMaximizedNonTrackedWindowObserver);
+};
+
+}  // namespace
+
+// Verifies setting tracked by workspace to false and then dragging a maximized
+// window doesn't result in changing the window hierarchy (which typically
+// indicates new workspaces have been created).
+TEST_F(WorkspaceManagerTest, DragMaximizedNonTrackedWindow) {
+  aura::test::EventGenerator generator(
+      Shell::GetPrimaryRootWindow(), gfx::Point());
+  generator.MoveMouseTo(5, 5);
+
+  aura::test::TestWindowDelegate delegate;
+  delegate.set_window_component(HTCAPTION);
+  scoped_ptr<Window> w1(
+      aura::test::CreateTestWindowWithDelegate(&delegate,
+                                               aura::client::WINDOW_TYPE_NORMAL,
+                                               gfx::Rect(5, 6, 7, 8),
+                                               NULL));
+  SetDefaultParentByPrimaryRootWindow(w1.get());
+  w1->SetProperty(aura::client::kShowStateKey, ui::SHOW_STATE_MAXIMIZED);
+  w1->Show();
+  wm::ActivateWindow(w1.get());
+  DragMaximizedNonTrackedWindowObserver observer;
+  w1->parent()->parent()->AddObserver(&observer);
+  const gfx::Rect max_bounds(w1->bounds());
+
+  // There should be two workspace, one for the desktop and one for the
+  // maximized window with the maximized active.
+  EXPECT_EQ("0 M1 active=1", StateString());
+
+  generator.PressLeftButton();
+  generator.MoveMouseTo(100, 100);
+  // The bounds shouldn't change (drag should result in nothing happening
+  // now.
+  EXPECT_EQ(max_bounds.ToString(), w1->bounds().ToString());
+  EXPECT_EQ("0 M1 active=1", StateString());
+
+  generator.ReleaseLeftButton();
+  EXPECT_EQ(0, observer.change_count());
+
+  // Set tracked to false and repeat, now the window should move.
+  SetTrackedByWorkspace(w1.get(), false);
+  generator.MoveMouseTo(5, 5);
+  generator.PressLeftButton();
+  generator.MoveMouseBy(100, 100);
+  EXPECT_EQ(gfx::Rect(max_bounds.x() + 100, max_bounds.y() + 100,
+                      max_bounds.width(), max_bounds.height()).ToString(),
+            w1->bounds().ToString());
+  EXPECT_EQ("0 M1 active=1", StateString());
+
+  generator.ReleaseLeftButton();
+  SetTrackedByWorkspace(w1.get(), true);
+  // Marking the window tracked again should snap back to origin.
+  EXPECT_EQ("0 M1 active=1", StateString());
+  EXPECT_EQ(max_bounds.ToString(), w1->bounds().ToString());
+  EXPECT_EQ(0, observer.change_count());
+
+  w1->parent()->parent()->RemoveObserver(&observer);
+}
+
 }  // namespace internal
 }  // namespace ash