Make the maximize button more easily hittable when there is a huge amount of tabs present

chrome/browser/ui/views/frame/opaque_browser_frame_view.cc

Index: chrome/browser/ui/views/frame/opaque_browser_frame_view.cc
diff --git a/chrome/browser/ui/views/frame/opaque_browser_frame_view.cc b/chrome/browser/ui/views/frame/opaque_browser_frame_view.cc
index 3e0547b6155530fcf009f0bda1bb006f30496d36..60356581bac5bbe699509ce4ef0340fe04a117fb 100644
--- a/chrome/browser/ui/views/frame/opaque_browser_frame_view.cc
+++ b/chrome/browser/ui/views/frame/opaque_browser_frame_view.cc
@@ -438,29 +438,66 @@ void OpaqueBrowserFrameView::Layout() {
 }
 
 bool OpaqueBrowserFrameView::HitTestRect(const gfx::Rect& rect) const {
-  // If |rect| does not intersect the bounds of the client area, claim it.
-  bool in_nonclient = NonClientFrameView::HitTestRect(rect);
-  if (in_nonclient)
-    return in_nonclient;
+  if (!views::View::HitTestRect(rect)) {
+    // |rect| is outside OpaqueBrowserFrameView's bounds.
+    return false;
+  }
 
-  // Otherwise claim it only if it's in a non-tab portion of the tabstrip.
-  if (!browser_view()->tabstrip())
+  // If the rect is outside the bounds of the client area, claim it.
+  gfx::Point rect_in_client_view_coords_origin(rect.origin());
+  View::ConvertPointToTarget(this, frame()->client_view(),

[tdanderson, 2013/07/18 20:47:04]

Please add a "TODO(tdanderson): Update this once ConvertRectToTarget() is
implemented."

But I am in the process of writing a CL for views fuzzing (crbug.com/129794),
which will consider non-unit rectangles representing tap areas, so I will be
implementing ConvertRectToTarget() in my CL.

+      &rect_in_client_view_coords_origin);
+  gfx::Rect rect_in_client_view_coords(
+      rect_in_client_view_coords_origin, rect.size());
+  if (!frame()->client_view()->HitTestRect(rect_in_client_view_coords))

[pkotwicz, 2013/07/18 14:38:29]

I fixed this from the pervious patch set. In the previous patch set I was not
converting |rect| to the client view bounds.

As a side note, I think it is finally time to add
views::View::ConvertRectToTarget() but I want to do that in a separate CL

[James Cook, 2013/07/18 18:39:45]

Agreed that ConvertRectToTarget both sounds like a good idea and should be done
in another patch.

+    return true;
+
+  // Otherwise, claim |rect| only if it is above the bottom of the tabstrip in
+  // a non-tab portion.
+  TabStrip* tabstrip = browser_view()->tabstrip();
+  if (!tabstrip || !browser_view()->IsTabStripVisible())
     return false;
-  gfx::Rect tabstrip_bounds(browser_view()->tabstrip()->bounds());
-  gfx::Point tabstrip_origin(tabstrip_bounds.origin());
-  View::ConvertPointToTarget(frame()->client_view(), this, &tabstrip_origin);
-  tabstrip_bounds.set_origin(tabstrip_origin);
-  if (rect.bottom() > tabstrip_bounds.bottom())
+
+  gfx::Point rect_in_tabstrip_coords_origin(rect.origin());
+  View::ConvertPointToTarget(this, tabstrip,

[tdanderson, 2013/07/18 20:47:04]

Also add a TODO for me here.

+      &rect_in_tabstrip_coords_origin);
+  gfx::Rect rect_in_tabstrip_coords(
+      rect_in_tabstrip_coords_origin, rect.size());
+
+  if (rect_in_tabstrip_coords.bottom() > tabstrip->GetLocalBounds().bottom()) {
+    // |rect| is below the tabstrip.
     return false;

[tdanderson, 2013/07/18 20:47:04]

Have you tested that this return is hit at the right times? (i.e., are you
confident that the existing code at line 453, 'if (rect.bottom() >
tabstrip_bounds.bottom())', is incorrect?)

+  }
 
-  // We convert from our parent's coordinates since we assume we fill its bounds
-  // completely. We need to do this since we're not a parent of the tabstrip,
-  // meaning ConvertPointToTarget would otherwise return something bogus.
-  // TODO(tdanderson): Initialize |browser_view_point| using |rect| instead of
-  // its center point once GetEventHandlerForRect() is implemented.
-  gfx::Point browser_view_point(rect.CenterPoint());
-  View::ConvertPointToTarget(parent(), browser_view(), &browser_view_point);
-  return browser_view()->IsPositionInWindowCaption(browser_view_point);
+  if (tabstrip->HitTestRect(rect_in_tabstrip_coords)) {
+    // Claim |rect| if it is in a non-tab portion of the tabstrip.
+    // TODO(tdanderson): Pass |rect_in_tabstrip_coords| instead of its center
+    // point to TabStrip::IsPositionInWindowCaption() once
+    // GetEventHandlerForRect() is implemented.
+    return tabstrip->IsPositionInWindowCaption(
+        rect_in_tabstrip_coords.CenterPoint());
+  }
+
+  // The window switcher button is to the right of the tabstrip but is
+  // part of the client view.
+  views::View* window_switcher_button =
+      browser_view()->window_switcher_button();
+  if (window_switcher_button && window_switcher_button->visible()) {
+    gfx::Point rect_in_window_switcher_coords_origin(rect.origin());
+    View::ConvertPointToTarget(this, window_switcher_button,

[tdanderson, 2013/07/18 20:47:04]

Also add a TODO for me here.

+        &rect_in_window_switcher_coords_origin);
+    gfx::Rect rect_in_window_switcher_coords(
+        rect_in_window_switcher_coords_origin, rect.size());
+
+    if (window_switcher_button->HitTestRect(rect_in_window_switcher_coords))
+      return false;
+  }
+
+  // We claim |rect| because it is above the bottom of the tabstrip, but
+  // neither in the tabstrip nor in the window switcher button. In particular,
+  // the avatar label/button is left of the tabstrip and the window controls
+  // are right of the tabstrip.
+  return true;
 }
 
 void OpaqueBrowserFrameView::GetAccessibleState(