Fixes tab dragging out of a window with maximzied bounds

chrome/browser/ui/views/tabs/tab_drag_controller.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/views/tabs/tab_drag_controller.h"
 
 #include <math.h>
 #include <set>
 
 #include "base/auto_reset.h"
@@ skipping 59 matching lines @@
 
 // Delay for moving tabs after the initial delay has passed.
 const int kMoveAttachedSubsequentDelay = 300;
 
 const int kHorizontalMoveThreshold = 16;  // Pixels.
 
 // Distance from the next/previous stacked before before we consider the tab
 // close enough to trigger moving.
 const int kStackedDistance = 36;
 
+// A dragged window is forced to be a bit smaller than maximized bounds during a
+// drag. This prevents the dragged browser widget from getting maximized at
+// creation and makes it easier to drag tabs out of a restored window that had
+// maximized size.
+const int kMaximizedWindowInset = 10;  // Pixels.

[sky, 2015/06/06 17:59:30]

Again, is this really pixels?

[varkha, 2015/06/06 20:13:17]

I thought so. This is same units as mouse travel, window bounds, etc. Are there
cases when this is something else?

+
 #if defined(USE_ASH)
 void SetWindowPositionManaged(gfx::NativeWindow window, bool value) {
   ash::wm::GetWindowState(window)->set_window_position_managed(value);
 }
 
 // Returns true if |tab_strip| browser window is docked.
 bool IsDockedOrSnapped(const TabStrip* tab_strip) {
   DCHECK(tab_strip);
   ash::wm::WindowState* window_state =
       ash::wm::GetWindowState(tab_strip->GetWidget()->GetNativeWindow());
@@ skipping 1543 matching lines @@
   return true;
 }
 
 gfx::Rect TabDragController::CalculateDraggedBrowserBounds(
     TabStrip* source,
     const gfx::Point& point_in_screen,
     std::vector<gfx::Rect>* drag_bounds) {
   gfx::Point center(0, source->height() / 2);
   views::View::ConvertPointToWidget(source, &center);
   gfx::Rect new_bounds(source->GetWidget()->GetRestoredBounds());
+
+  gfx::Rect work_area =
+      screen_->GetDisplayNearestPoint(last_point_in_screen_).work_area();

[sky, 2015/06/05 19:24:27]

Shouldn't this be an else condition of maximized? Also, make sure you try this
on windows.

[varkha, 2015/06/05 19:48:48]

I thinks this should be complimentary rather than an else on IsMaximized.
Consider the case when a window is manually resized to work_area bounds and then
maximized. Its restore bounds will be equal to work_area and unless we adjust
the restore bounds the tab will be undraggable. I have tested this theory and
indeed the bug remains in that corner case if I move the logic under else. Both
this corner case and the one described in the bug are fixed if I leave the logic
where it is now.
I have added a unit test that tests this corner case on Windows (where the only
new behavior is that the window will be maximized after drag). Would you trust
that?

+  if (new_bounds.size().width() >= work_area.size().width() &&
+      new_bounds.size().height() >= work_area.size().height()) {
+    new_bounds = work_area;
+    new_bounds.Inset(kMaximizedWindowInset, kMaximizedWindowInset,
+                     kMaximizedWindowInset, kMaximizedWindowInset);
+    // Behave as if the |source| was maximized at the start of a drag since this
+    // is consistent with a browser window creation logic in case of windows
+    // that are as large as the |work_area|.
+    was_source_maximized_ = true;
+  }
+
   if (source->GetWidget()->IsMaximized()) {
     // If the restore bounds is really small, we don't want to honor it
     // (dragging a really small window looks wrong), instead make sure the new
     // window is at least 50% the size of the old.
     const gfx::Size max_size(
         source->GetWidget()->GetWindowBoundsInScreen().size());
     new_bounds.set_width(
         std::max(max_size.width() / 2, new_bounds.width()));
     new_bounds.set_height(
         std::max(max_size.height() / 2, new_bounds.height()));
@@ skipping 143 matching lines @@
        it != browser_list->end(); ++it) {
     if ((*it)->tab_strip_model()->empty())
       exclude.insert((*it)->window()->GetNativeWindow());
   }
 #endif
   return GetLocalProcessWindowAtPoint(host_desktop_type_,
                                       screen_point,
                                       exclude);
 
 }