Reorganize some code and add comments while figuring out how to fix some bugs.

Bug: none
Change-Id: I01f71178f5319556d5d749ff360b8433fac507ed
Reviewed-on: https://chromium-review.googlesource.com/1139180Reviewed-by: Allen Bauer <kylixrd@chromium.org>
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581246}

chrome/browser/ui/views/frame/browser_non_client_frame_view_mac.mm

@@ -21,12 +21,6 @@
 #include "ui/base/theme_provider.h"
 #include "ui/gfx/canvas.h"
 
-namespace {
-
-constexpr int kTabstripTopInset = 8;
-
-}  // namespace
-
 ///////////////////////////////////////////////////////////////////////////////
 // BrowserNonClientFrameViewMac, public:
 
@@ -85,6 +79,11 @@ int BrowserNonClientFrameViewMac::GetTopInset(bool restored) const {
   if (!browser_view()->IsTabStripVisible())
     return 0;
 
+  // TODO(pkasting): https://crbug.com/862276  Increase this height when we
+  // can't extend the drag handle into the tabstrip.  Mac seems to reserve 1 DIP
+  // of this as resize handle, so the actual top drag height is 7 DIP.
+  constexpr int kTabstripTopInset = 8;
+
   // Calculate the y offset for the tab strip because in fullscreen mode the tab
   // strip may need to move under the slide down menu bar.
   CGFloat y_offset = kTabstripTopInset;
@@ -94,11 +93,12 @@ int BrowserNonClientFrameViewMac::GetTopInset(bool restored) const {
     CGFloat title_bar_height =
         NSHeight([NSWindow frameRectForContentRect:NSZeroRect
                                          styleMask:NSWindowStyleMaskTitled]);
-    y_offset +=
+    CGFloat added_height =
         [[fullscreen_toolbar_controller_ menubarTracker] menubarFraction] *
         (menu_bar_height + title_bar_height);
+    y_offset += added_height;
 
-    if (y_offset > kTabstripTopInset) {
+    if (added_height > 0) {
       // When menubar shows up, we need to update mouse tracking area.
       NSWindow* window = GetWidget()->GetNativeWindow();
       NSRect content_bounds = [[window contentView] bounds];

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

@@ -488,6 +488,8 @@ bool GlassBrowserFrameView::DoesIntersectRect(const views::View* target,
   // TODO(bsep): This override has "dead zones" where you can't click on the
   // custom titlebar buttons. It's not clear why it's necessary at all.
   // Investigate tearing this out.
+  // TODO(pkasting): https://crbug.com/862276  This interferes with drag handle
+  // extension because we never run BrowserNonClientFrameView's code.
   CHECK_EQ(target, this);
   bool hit_incognito_icon =
       profile_indicator_icon() &&
@@ -533,6 +535,8 @@ int GlassBrowserFrameView::FrameTopBorderThickness(bool restored) const {
   // not maximized or fullscreen. When maximized, the OS sizes the window such
   // that the border extends beyond the screen edges. In that case, we must
   // return the default value.
+  // TODO(pkasting): https://crbug.com/862276  Increase this height when we
+  // can't extend the drag handle into the tabstrip.
   if (MD::IsRefreshUi() &&
       ((!frame()->IsFullscreen() && !IsMaximized()) || restored)) {
     constexpr int kTopResizeFrameArea = 5;

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

@@ -289,6 +289,8 @@ int OpaqueBrowserFrameViewLayout::GetNonClientRestoredExtraThickness() {
   // Besides the frame border, there's empty space atop the window in restored
   // mode, to use to drag the window around.
   constexpr int kNonClientRestoredExtraThickness = 11;
+  // TODO(pkasting): https://crbug.com/862276  Increase this height when we
+  // can't extend the drag handle into the tabstrip.
   constexpr int kRefreshNonClientRestoredExtraThickness = 4;
   return MD::IsRefreshUi() ? kRefreshNonClientRestoredExtraThickness
                            : kNonClientRestoredExtraThickness;

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

@@ -417,6 +417,9 @@ void NewTabButton::GetBorderPath(float button_y,
   const gfx::Rect contents_bounds = GetContentsBounds();
 
   if (MD::IsRefreshUi()) {
+    // TODO(pkasting): This should really be a circle with (potentially) its top
+    // half extended rectangularly upward... try to reuse
+    // GetTouchOptimizedButtonPath().
     path->addRect(0, extend_to_top ? 0 : button_y,
                   contents_bounds.width() * scale,
                   button_y + contents_bounds.height() * scale);
@@ -470,7 +473,7 @@ void NewTabButton::PaintFill(bool pressed,
       }
 
       const bool succeeded = canvas->InitPaintFlagsForTiling(
-          *tp->GetImageSkiaNamed(bg_id), x, GetContentsBounds().y() + offset_y,
+          *tp->GetImageSkiaNamed(bg_id), x, contents_bounds.y() + offset_y,
           x_scale * scale, scale, 0, 0, SkShader::kRepeat_TileMode,
           SkShader::kRepeat_TileMode, &flags);
       DCHECK(succeeded);

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

@@ -333,41 +333,37 @@ void TabStrip::SetBackgroundOffset(int offset) {
 }
 
 bool TabStrip::IsRectInWindowCaption(const gfx::Rect& rect) {
-  views::View* v = GetEventHandlerForRect(rect);
-
-  // If there is no control at this location, claim the hit was in the title
-  // bar to get a move action.
+  // If there is no control at this location, the hit is in the caption area.
+  const views::View* v = GetEventHandlerForRect(rect);
   if (v == this)
     return true;
 
-  // Set the hit test overlap to 1 dip above the top edge of the tab separator.
-  const int inactive_tab_hit_test_overlap =
+  // Under refresh, a thin strip at the top of inactive tabs and the new tab
+  // button is treated as part of the window drag handle, to increase
+  // draggability.  This region starts 1 DIP above the top of the separator.
+  const int drag_handle_extension =
       (height() - Tab::GetTabSeparatorHeight()) / 2 - 1;
-  // If there is a tab at this location, this hit is not likely in the title
-  // bar, except under the conditions below.
+  // TODO(pkasting): https://crbug.com/862276  Set this condition false when tab
+  // shapes are visible.
+  const bool extend_drag_handle =
+      MD::IsRefreshUi() && !SizeTabButtonToTopOfTabStrip();
+
+  // A hit on the tab is not in the caption unless it is in the thin strip
+  // mentioned above.
   const int tab_index = tabs_.GetIndexOfView(v);
   if (IsValidModelIndex(tab_index)) {
     Tab* tab = tab_at(tab_index);
-    // Under refresh, a 7 dip area at the top of an inactive tab should be
-    // considered part of the window caption. This makes the window drag region
-    // a little larger which makes it easier to target.
-    if (MD::IsRefreshUi() && !SizeTabButtonToTopOfTabStrip() &&
-        !tab->IsActive()) {
-      return gfx::Rect(tab->bounds().origin(),
-                       gfx::Size(tab->width(), inactive_tab_hit_test_overlap))
-          .Intersects(rect);
-    }
-    return false;
+    gfx::Rect tab_drag_handle = tab->bounds();
+    tab_drag_handle.set_height(drag_handle_extension);
+    return extend_drag_handle && !tab->IsActive() &&
+           tab_drag_handle.Intersects(rect);
   }
 
-  // Under refresh, check if the rect intersects a thin 7 dip strip along the
-  // top of the new tab button. This also makes the window drag region above the
-  // new tab button a little larger for ease of window dragging.
-  if (MD::IsRefreshUi() && !SizeTabButtonToTopOfTabStrip() &&
-      gfx::Rect(
-          new_tab_button_->bounds().origin(),
-          gfx::Size(new_tab_button_->width(), inactive_tab_hit_test_overlap))
-          .Intersects(rect))
+  // Similarly, a hit in the new tab button is considered to be in the caption
+  // if it's in this thin strip.
+  gfx::Rect new_tab_button_drag_handle = new_tab_button_->bounds();
+  new_tab_button_drag_handle.set_height(drag_handle_extension);
+  if (extend_drag_handle && new_tab_button_drag_handle.Intersects(rect))
     return true;
 
   // Check to see if the rect intersects the non-button parts of the new tab
@@ -377,14 +373,8 @@ bool TabStrip::IsRectInWindowCaption(const gfx::Rect& rect) {
   View::ConvertRectToTarget(this, new_tab_button_, &rect_in_new_tab_coords_f);
   gfx::Rect rect_in_new_tab_coords =
       gfx::ToEnclosingRect(rect_in_new_tab_coords_f);
-  if (new_tab_button_->GetLocalBounds().Intersects(rect_in_new_tab_coords) &&
-      !new_tab_button_->HitTestRect(rect_in_new_tab_coords))
-    return true;
-
-  // All other regions, including the new Tab button, should be considered part
-  // of the containing Window's client area so that regular events can be
-  // processed for them.
-  return false;
+  return new_tab_button_->GetLocalBounds().Intersects(rect_in_new_tab_coords) &&
+         !new_tab_button_->HitTestRect(rect_in_new_tab_coords);
 }
 
 bool TabStrip::IsPositionInWindowCaption(const gfx::Point& point) {