Attempt 3 at: Fixes dragging a tab out of a mazimixed window. We

needed to honor the 'tracked by workspace' property. I added test
coverage of this too.

When ending the drag TabDragController reset the 'tracked by
workspace' property. This could result in the window bounds changing
notifying the observer and attempting to update this. I've moved
around a variable so that doesn't happen.

BUG=151218
TEST=covered by tests now, but see bug for repro steps.
R=ben@chromium.org
TBR=ben@chromium.org

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

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

9 files changed, 185 insertions, 17 deletions

diff --git a/ash/wm/frame_painter.cc b/ash/wm/frame_painter.cc
index 568438f..fc950ae 100644
--- a/ash/wm/frame_painter.cc
+++ b/ash/wm/frame_painter.cc
@@ -8,6 +8,7 @@
 #include "ash/shell.h"
 #include "ash/shell_window_ids.h"
 #include "ash/wm/property_util.h"
+#include "ash/wm/window_properties.h"
 #include "ash/wm/window_util.h"
 #include "ash/wm/workspace_controller.h"
 #include "base/logging.h"  // DCHECK
@@ -475,7 +476,8 @@ void FramePainter::LayoutHeader(views::NonClientFrameView* view,
                                 bool shorter_layout) {
   // The new assets only make sense if the window is actually maximized.
   if (internal::WorkspaceController::IsWorkspace2Enabled() &&
-      shorter_layout && frame_->IsMaximized()) {
+      shorter_layout && frame_->IsMaximized() &&
+      GetTrackedByWorkspace(frame_->GetNativeWindow())) {
     SetButtonImages(close_button_,
                     IDR_AURA_WINDOW_MAXIMIZED_CLOSE2,
                     IDR_AURA_WINDOW_MAXIMIZED_CLOSE2_H,
@@ -537,6 +539,13 @@ void FramePainter::LayoutHeader(views::NonClientFrameView* view,
 void FramePainter::OnWindowPropertyChanged(aura::Window* window,
                                            const void* key,
                                            intptr_t old) {
+  if (key == internal::kWindowTrackedByWorkspaceKey &&
+      GetTrackedByWorkspace(window)) {
+    // When 'kWindowTrackedByWorkspaceKey' changes we're going to paint the
+    // header differently. Schedule a paint to ensure everything is updated
+    // correctly.
+    frame_->non_client_view()->SchedulePaint();
+  }
   if (key != aura::client::kShowStateKey)
     return;
 
@@ -627,9 +636,11 @@ int FramePainter::GetHeaderOpacity(HeaderMode header_mode,
   if (theme_frame_overlay)
     return kFullyOpaque;
 
-  // Maximized windows with workspace2 are totally transparent.
+  // Maximized windows with workspace2 are totally transparent, except those not
+  // tracked by workspace code (which are used for tab dragging).
   if (frame_->IsMaximized() &&
-      internal::WorkspaceController::IsWorkspace2Enabled())
+      internal::WorkspaceController::IsWorkspace2Enabled() &&
+      GetTrackedByWorkspace(frame_->GetNativeWindow()))
     return 0;
 
   // Single browser window is very transparent.
diff --git a/ash/wm/toplevel_window_event_handler.cc b/ash/wm/toplevel_window_event_handler.cc
index 8206a33..81dfdb4 100644
--- a/ash/wm/toplevel_window_event_handler.cc
+++ b/ash/wm/toplevel_window_event_handler.cc
@@ -98,7 +98,8 @@ void ToplevelWindowEventHandler::ScopedWindowResizer::OnWindowDestroying(
 
 ToplevelWindowEventHandler::ToplevelWindowEventHandler(aura::Window* owner)
     : in_move_loop_(false),
-      move_cancelled_(false) {
+      move_cancelled_(false),
+      in_gesture_resize_(false) {
   aura::client::SetWindowMoveClient(owner, this);
   Shell::GetInstance()->display_controller()->AddObserver(this);
   owner->AddPreTargetHandler(this);
diff --git a/ash/wm/workspace/workspace2.cc b/ash/wm/workspace/workspace2.cc
index 03567b6..089e83c 100644
--- a/ash/wm/workspace/workspace2.cc
+++ b/ash/wm/workspace/workspace2.cc
@@ -63,7 +63,8 @@ bool Workspace2::ShouldMoveToPending() const {
   bool has_visible_non_maximized_window = false;
   for (size_t i = 0; i < window_->children().size(); ++i) {
     aura::Window* child(window_->children()[i]);
-    if (!child->TargetVisibility() || wm::IsWindowMinimized(child))
+    if (!GetTrackedByWorkspace(child) || !child->TargetVisibility() ||
+        wm::IsWindowMinimized(child))
       continue;
     if (WorkspaceManager2::IsMaximized(child))
       return false;
@@ -78,8 +79,9 @@ int Workspace2::GetNumMaximizedWindows() const {
   int count = 0;
   for (size_t i = 0; i < window_->children().size(); ++i) {
     aura::Window* child = window_->children()[i];
-    if (WorkspaceManager2::IsMaximized(child) ||
-        WorkspaceManager2::WillRestoreMaximized(child)) {
+    if (GetTrackedByWorkspace(child) &&
+        (WorkspaceManager2::IsMaximized(child) ||
+         WorkspaceManager2::WillRestoreMaximized(child))) {
       if (++count == 2)
         return count;
     }
diff --git a/ash/wm/workspace/workspace_layout_manager2.cc b/ash/wm/workspace/workspace_layout_manager2.cc
index 8f966f7..7f3be99 100644
--- a/ash/wm/workspace/workspace_layout_manager2.cc
+++ b/ash/wm/workspace/workspace_layout_manager2.cc
@@ -129,6 +129,10 @@ void WorkspaceLayoutManager2::OnChildWindowVisibilityChanged(Window* child,
 void WorkspaceLayoutManager2::SetChildBounds(
     Window* child,
     const gfx::Rect& requested_bounds) {
+  if (!GetTrackedByWorkspace(child)) {
+    SetChildBoundsDirect(child, requested_bounds);
+    return;
+  }
   gfx::Rect child_bounds(requested_bounds);
   // Some windows rely on this to set their initial bounds.
   if (!SetMaximizedOrFullscreenBounds(child))
@@ -211,6 +215,11 @@ void WorkspaceLayoutManager2::OnWindowPropertyChanged(Window* window,
       SetRestoreBoundsInScreen(window, restore);
   }
 
+  if (key == internal::kWindowTrackedByWorkspaceKey &&
+      GetTrackedByWorkspace(window)) {
+    workspace_manager()->OnTrackedByWorkspaceChanged(workspace_, window);
+  }
+
   if (key == aura::client::kAlwaysOnTopKey &&
       window->GetProperty(aura::client::kAlwaysOnTopKey)) {
     internal::AlwaysOnTopController* controller =
@@ -274,7 +283,8 @@ void WorkspaceLayoutManager2::AdjustWindowSizesForScreenChange(
 void WorkspaceLayoutManager2::AdjustWindowSizeForScreenChange(
     Window* window,
     AdjustWindowReason reason) {
-  if (!SetMaximizedOrFullscreenBounds(window)) {
+  if (GetTrackedByWorkspace(window) &&
+      !SetMaximizedOrFullscreenBounds(window)) {
     if (reason == ADJUST_WINDOW_SCREEN_SIZE_CHANGED) {
       // The work area may be smaller than the full screen.  Put as much of the
       // window as possible within the display area.
@@ -339,6 +349,9 @@ void WorkspaceLayoutManager2::UpdateBoundsFromShowState(Window* window) {
 
 bool WorkspaceLayoutManager2::SetMaximizedOrFullscreenBounds(
     aura::Window* window) {
+  if (!GetTrackedByWorkspace(window))
+    return false;
+
   // During animations there is a transform installed on the workspace
   // windows. For this reason this code uses the parent so that the transform is
   // ignored.
diff --git a/ash/wm/workspace/workspace_manager2.cc b/ash/wm/workspace/workspace_manager2.cc
index 247a961..ad7546b 100644
--- a/ash/wm/workspace/workspace_manager2.cc
+++ b/ash/wm/workspace/workspace_manager2.cc
@@ -199,16 +199,28 @@ void WorkspaceManager2::SetActiveWorkspaceByWindow(Window* window) {
     return;
 
   if (workspace != active_workspace_) {
-    // If the window persists across all workspaces, move it to the current
-    // workspace.
-    // A popup which was first maximized, is minimized and getts restored has
-    // to go through the SetActiveWorkspace since it persists across workspaces
-    // and is not (yet) maximized (see crbug.com/151698).
-    if (GetPersistsAcrossAllWorkspaces(window) && !IsMaximized(window) &&
-        !(wm::IsWindowMinimized(window) && WillRestoreMaximized(window)))
+    // A window is being made active. In the following cases we reparent to
+    // the active desktop:
+    // . The window is not tracked by workspace code. This is used for tab
+    //   dragging. Since tab dragging needs to happen in the active workspace we
+    //   have to reparent the window (otherwise the window you dragged the tab
+    //   out of would disappear since the workspace changed). Since this case is
+    //   only transiently used (property reset on input release) we don't worry
+    //   about window state. In fact we can't consider window state here as we
+    //   have to allow dragging of a maximized window to work in this case.
+    // . The window persists across all workspaces. For example, the task
+    //   manager is in the desktop worskpace and the current workspace is
+    //   maximized. If we swapped to the desktop you would lose context. Instead
+    //   we reparent.  The exception to this is if the window is maximized (it
+    //   needs its own workspace then) or we're in the process of maximizing. If
+    //   we're in the process of maximizing the window needs its own workspace.
+    if (!GetTrackedByWorkspace(window) ||
+        (GetPersistsAcrossAllWorkspaces(window) && !IsMaximized(window) &&
+         !(wm::IsWindowMinimized(window) && WillRestoreMaximized(window)))) {
       ReparentWindow(window, active_workspace_->window(), NULL);
-    else
+    } else {
       SetActiveWorkspace(workspace, ANIMATE_OLD_AND_NEW);
+    }
   }
 }
 
@@ -222,6 +234,9 @@ Window* WorkspaceManager2::GetParentForNewWindow(Window* window) {
     // Fall through to normal logic.
   }
 
+  if (!GetTrackedByWorkspace(window))
+    return active_workspace_->window();
+
   if (IsMaximized(window)) {
     // Wait for the window to be made active before showing the workspace.
     Workspace2* workspace = CreateWorkspace(true);
@@ -577,5 +592,27 @@ void WorkspaceManager2::OnWorkspaceWindowShowStateChanged(
   UpdateShelfVisibility();
 }
 
+void WorkspaceManager2::OnTrackedByWorkspaceChanged(Workspace2* workspace,
+                                                    aura::Window* window) {
+  Workspace2* new_workspace = NULL;
+  if (IsMaximized(window)) {
+    new_workspace = CreateWorkspace(true);
+    pending_workspaces_.insert(new_workspace);
+  } else if (workspace->is_maximized()) {
+    new_workspace = desktop_workspace();
+  } else {
+    return;
+  }
+  // If the window is active we need to make sure the destination Workspace
+  // window is showing. Otherwise the window will be parented to a hidden window
+  // and lose activation.
+  const bool is_active = wm::IsActiveWindow(window);
+  if (is_active)
+    new_workspace->window()->Show();
+  ReparentWindow(window, new_workspace->window(), NULL);
+  if (is_active)
+    SetActiveWorkspace(new_workspace, ANIMATE_OLD_AND_NEW);
+}
+
 }  // namespace internal
 }  // namespace ash
diff --git a/ash/wm/workspace/workspace_manager2.h b/ash/wm/workspace/workspace_manager2.h
index 653d684..6c85243 100644
--- a/ash/wm/workspace/workspace_manager2.h
+++ b/ash/wm/workspace/workspace_manager2.h
@@ -155,6 +155,8 @@ class ASH_EXPORT WorkspaceManager2
                                          aura::Window* child,
                                          ui::WindowShowState last_show_state,
                                          ui::Layer* old_layer);
+  void OnTrackedByWorkspaceChanged(Workspace2* workspace,
+                                   aura::Window* window);
 
   aura::Window* contents_view_;
 
diff --git a/ash/wm/workspace/workspace_manager2_unittest.cc b/ash/wm/workspace/workspace_manager2_unittest.cc
index 10ac744..786f54b 100644
--- a/ash/wm/workspace/workspace_manager2_unittest.cc
+++ b/ash/wm/workspace/workspace_manager2_unittest.cc
@@ -1037,5 +1037,46 @@ TEST_F(WorkspaceManager2Test, TransientParent) {
   EXPECT_EQ(w2->parent(), w1->parent());
 }
 
+// Verifies changing TrackedByWorkspace works.
+TEST_F(WorkspaceManager2Test, TrackedByWorkspace) {
+  // Create a window maximized.
+  scoped_ptr<Window> w1(CreateTestWindow());
+  w1->Show();
+  wm::ActivateWindow(w1.get());
+  w1->SetProperty(aura::client::kShowStateKey, ui::SHOW_STATE_MAXIMIZED);
+  EXPECT_TRUE(wm::IsActiveWindow(w1.get()));
+  EXPECT_TRUE(w1->IsVisible());
+
+  // Create a second window maximized and mark it not tracked by workspace
+  // manager.
+  scoped_ptr<Window> w2(CreateTestWindowUnparented());
+  w2->SetBounds(gfx::Rect(1, 6, 25, 30));
+  w2->SetProperty(aura::client::kShowStateKey, ui::SHOW_STATE_MAXIMIZED);
+  w2->SetParent(NULL);
+  w2->Show();
+  SetTrackedByWorkspace(w2.get(), false);
+  wm::ActivateWindow(w2.get());
+
+  // Activating |w2| should force it to have the same parent as |w1|.
+  EXPECT_EQ(w1->parent(), w2->parent());
+  EXPECT_TRUE(wm::IsActiveWindow(w2.get()));
+  EXPECT_TRUE(w1->IsVisible());
+  EXPECT_TRUE(w2->IsVisible());
+
+  // Because |w2| isn't tracked we should be able to set the bounds of it.
+  gfx::Rect bounds(w2->bounds());
+  bounds.Offset(4, 5);
+  w2->SetBounds(bounds);
+  EXPECT_EQ(bounds.ToString(), w2->bounds().ToString());
+
+  // Transition it to tracked by worskpace. It should end up in its own
+  // workspace.
+  SetTrackedByWorkspace(w2.get(), true);
+  EXPECT_TRUE(wm::IsActiveWindow(w2.get()));
+  EXPECT_FALSE(w1->IsVisible());
+  EXPECT_TRUE(w2->IsVisible());
+  EXPECT_NE(w1->parent(), w2->parent());
+}
+
 }  // namespace internal
 }  // namespace ash
diff --git a/chrome/browser/ui/views/tabs/tab_drag_controller.cc b/chrome/browser/ui/views/tabs/tab_drag_controller.cc
index 3f50828..fe20d7a 100644
--- a/chrome/browser/ui/views/tabs/tab_drag_controller.cc
+++ b/chrome/browser/ui/views/tabs/tab_drag_controller.cc
@@ -1546,12 +1546,16 @@ void TabDragController::EndDragImpl(EndDragType type) {
   move_stacked_timer_.Stop();
 
   if (is_dragging_window_) {
+    // SetTrackedByWorkspace() may call us back (by way of the window bounds
+    // changing). Set |waiting_for_run_loop_to_exit_| here so that if that
+    // happens we ignore it.
+    waiting_for_run_loop_to_exit_ = true;
+
     if (type == NORMAL || (type == TAB_DESTROYED && drag_data_.size() > 1))
       SetTrackedByWorkspace(GetAttachedBrowserWidget()->GetNativeView(), true);
 
     // End the nested drag loop.
     GetAttachedBrowserWidget()->EndMoveLoop();
-    waiting_for_run_loop_to_exit_ = true;
   }
 
   // Hide the current dock controllers.
@@ -1953,6 +1957,9 @@ Browser* TabDragController::CreateBrowserForDrag(
   create_params.initial_bounds = new_bounds;
   Browser* browser = new Browser(create_params);
   SetTrackedByWorkspace(browser->window()->GetNativeWindow(), false);
+  // If the window is created maximized then the bounds we supplied are ignored.
+  // We need to reset them again so they are honored.
+  browser->window()->SetBounds(new_bounds);
   return browser;
 }
 
diff --git a/chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc b/chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc
index 0677143..11d183b 100644
--- a/chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc
+++ b/chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc
@@ -843,6 +843,60 @@ IN_PROC_BROWSER_TEST_P(DetachToBrowserTabDragControllerTest,
 
 #if defined(USE_ASH)
 
+namespace {
+
+void DragInMaximizedWindowStep2(DetachToBrowserTabDragControllerTest* test,
+                                Browser* browser,
+                                TabStrip* tab_strip) {
+  // There should be another browser.
+  ASSERT_EQ(2u, BrowserList::size());
+  Browser* new_browser = *(++BrowserList::begin());
+  EXPECT_NE(browser, new_browser);
+  ASSERT_TRUE(new_browser->window()->IsActive());
+  TabStrip* tab_strip2 = GetTabStripForBrowser(new_browser);
+
+  ASSERT_TRUE(tab_strip2->IsDragSessionActive());
+  ASSERT_FALSE(tab_strip->IsDragSessionActive());
+
+  // Both windows should be visible.
+  EXPECT_TRUE(tab_strip->GetWidget()->IsVisible());
+  EXPECT_TRUE(tab_strip2->GetWidget()->IsVisible());
+
+  // Stops dragging.
+  ASSERT_TRUE(test->ReleaseInput());
+}
+
+}  // namespace
+
+// Creates a browser with two tabs, maximizes it, drags the tab out.
+IN_PROC_BROWSER_TEST_P(DetachToBrowserTabDragControllerTest,
+                       DragInMaximizedWindow) {
+  AddTabAndResetBrowser(browser());
+  browser()->window()->Maximize();
+
+  TabStrip* tab_strip = GetTabStripForBrowser(browser());
+
+  // Move to the first tab and drag it enough so that it detaches.
+  gfx::Point tab_0_center(
+      GetCenterInScreenCoordinates(tab_strip->tab_at(0)));
+  ASSERT_TRUE(PressInput(tab_0_center));
+  ASSERT_TRUE(DragInputToNotifyWhenDone(
+      tab_0_center.x(), tab_0_center.y() + GetDetachY(tab_strip),
+      base::Bind(&DragInMaximizedWindowStep2, this, browser(), tab_strip)));
+  QuitWhenNotDragging();
+
+  ASSERT_FALSE(TabDragController::IsActive());
+
+  // Should be two browsers.
+  ASSERT_EQ(2u, BrowserList::size());
+  Browser* new_browser = *(++BrowserList::begin());
+  ASSERT_TRUE(new_browser->window()->IsActive());
+
+  // Only the new browser should be visible.
+  EXPECT_FALSE(browser()->window()->GetNativeWindow()->IsVisible());
+  EXPECT_TRUE(new_browser->window()->GetNativeWindow()->IsVisible());
+}
+
 // Subclass of DetachToBrowserInSeparateDisplayTabDragControllerTest that
 // creates multiple displays.
 class DetachToBrowserInSeparateDisplayTabDragControllerTest