[Refresh] Fix tab hit area not extending to top of window on tiling WMs

BUG=882854
R=pkasting

Change-Id: I8dc419e43882d723a117da19e87abe20505aa01a
Reviewed-on: https://chromium-review.googlesource.com/1228201
Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#592148}

chrome/browser/defaults.cc

@@ -36,7 +36,6 @@ const bool kShowHelpMenuItemIcon = false;
 #endif
 
 const bool kDownloadPageHasShowInFolder = true;
-const bool kSizeTabButtonToTopOfTabStrip = false;
 
 #if defined(OS_CHROMEOS) || defined(OS_ANDROID)
 const bool kSyncAutoStarts = true;

chrome/browser/defaults.h

@@ -36,9 +36,6 @@ extern const bool kAlwaysCreateTabbedBrowserOnSessionRestore;
 // Does the download page have the show in folder option?
 extern const bool kDownloadPageHasShowInFolder;
 
-// Should the tab strip be sized to the top of the tab strip?
-extern const bool kSizeTabButtonToTopOfTabStrip;
-
 // If true, we want to automatically start sync signin whenever we have
 // credentials (user doesn't need to go through the startup flow). This is
 // typically enabled on platforms (like ChromeOS) that have their own

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

@@ -101,6 +101,10 @@ bool BrowserNonClientFrameView::ShouldHideTopUIForFullscreen() const {
   return frame_->IsFullscreen();
 }
 
+bool BrowserNonClientFrameView::IsFrameCondensed() const {
+  return frame_ && (frame_->IsMaximized() || frame_->IsFullscreen());
+}
+
 bool BrowserNonClientFrameView::HasClientEdge() const {
   return !MD::IsRefreshUi();
 }

chrome/browser/ui/views/frame/browser_non_client_frame_view.h

@@ -87,6 +87,11 @@ class BrowserNonClientFrameView : public views::NonClientFrameView,
   // Returns whether the top UI should hide.
   virtual bool ShouldHideTopUIForFullscreen() const;
 
+  // Determines whether the top frame is condensed vertically, as when the
+  // window is maximized. If true, the top frame is just the height of a tab,
+  // rather than having extra vertical space above the tabs.
+  virtual bool IsFrameCondensed() const;
+
   // Returns whether the content is painted with a client edge or not.
   virtual bool HasClientEdge() const;
 

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

@@ -55,7 +55,6 @@ class TestLayoutDelegate : public OpaqueBrowserFrameViewLayoutDelegate {
   }
   bool IsMaximized() const override { return false; }
   bool IsMinimized() const override { return false; }
-  bool IsFullscreen() const override { return false; }
   bool IsTabStripVisible() const override { return true; }
   int GetTabStripHeight() const override {
     return GetLayoutConstant(TAB_HEIGHT);
@@ -69,6 +68,7 @@ class TestLayoutDelegate : public OpaqueBrowserFrameViewLayoutDelegate {
   }
   int GetTopAreaHeight() const override { return 0; }
   bool UseCustomFrame() const override { return true; }
+  bool IsFrameCondensed() const override { return false; }
   bool EverHasVisibleBackgroundTabShapes() const override { return false; }
 
  private:

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

@@ -290,7 +290,7 @@ int OpaqueBrowserFrameView::NonClientHitTest(const gfx::Point& point) {
     gfx::Rect sysmenu_rect(IconBounds());
     // In maximized mode we extend the rect to the screen corner to take
     // advantage of Fitts' Law.
-    if (layout_->IsTitleBarCondensed())
+    if (IsFrameCondensed())
       sysmenu_rect.SetRect(0, 0, sysmenu_rect.right(), sysmenu_rect.bottom());
     sysmenu_rect = GetMirroredRect(sysmenu_rect);
     if (sysmenu_rect.Contains(point))
@@ -343,7 +343,7 @@ void OpaqueBrowserFrameView::GetWindowMask(const gfx::Size& size,
                                            gfx::Path* window_mask) {
   DCHECK(window_mask);
 
-  if (layout_->IsTitleBarCondensed() || frame()->IsFullscreen())
+  if (IsFrameCondensed())
     return;
 
   views::GetDefaultWindowMask(
@@ -504,10 +504,6 @@ bool OpaqueBrowserFrameView::IsMinimized() const {
   return frame()->IsMinimized();
 }
 
-bool OpaqueBrowserFrameView::IsFullscreen() const {
-  return frame()->IsFullscreen();
-}
-
 bool OpaqueBrowserFrameView::IsTabStripVisible() const {
   return browser_view()->IsTabStripVisible();
 }
@@ -546,6 +542,11 @@ bool OpaqueBrowserFrameView::UseCustomFrame() const {
   return frame()->UseCustomFrame();
 }
 
+bool OpaqueBrowserFrameView::IsFrameCondensed() const {
+  return BrowserNonClientFrameView::IsFrameCondensed() ||
+         !ShouldShowCaptionButtons();
+}
+
 bool OpaqueBrowserFrameView::EverHasVisibleBackgroundTabShapes() const {
   return BrowserNonClientFrameView::EverHasVisibleBackgroundTabShapes();
 }
@@ -574,7 +575,7 @@ void OpaqueBrowserFrameView::OnPaint(gfx::Canvas* canvas) {
   frame_background_->set_theme_overlay_image(GetFrameOverlayImage());
   frame_background_->set_top_area_height(GetTopAreaHeight());
 
-  if (layout_->IsTitleBarCondensed())
+  if (IsFrameCondensed())
     PaintMaximizedFrameBorder(canvas);
   else
     PaintRestoredFrameBorder(canvas);
@@ -777,7 +778,7 @@ void OpaqueBrowserFrameView::PaintClientEdge(gfx::Canvas* canvas) const {
   }
 
   // In maximized mode, the only edge to draw is the top one, so we're done.
-  if (layout_->IsTitleBarCondensed())
+  if (IsFrameCondensed())
     return;
 
   const ui::ThemeProvider* tp = GetThemeProvider();

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

@@ -99,7 +99,6 @@ class OpaqueBrowserFrameView : public BrowserNonClientFrameView,
   gfx::ImageSkia GetIncognitoAvatarIcon() const override;
   bool IsMaximized() const override;
   bool IsMinimized() const override;
-  bool IsFullscreen() const override;
   bool IsTabStripVisible() const override;
   int GetTabStripHeight() const override;
   bool IsToolbarVisible() const override;
@@ -107,6 +106,7 @@ class OpaqueBrowserFrameView : public BrowserNonClientFrameView,
   gfx::Size GetNewTabButtonPreferredSize() const override;
   int GetTopAreaHeight() const override;
   bool UseCustomFrame() const override;
+  bool IsFrameCondensed() const override;
   bool EverHasVisibleBackgroundTabShapes() const override;
 
  protected:

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

@@ -144,14 +144,12 @@ gfx::Rect OpaqueBrowserFrameViewLayout::GetWindowBoundsForClientBounds(
 }
 
 int OpaqueBrowserFrameViewLayout::FrameBorderThickness(bool restored) const {
-  return (!restored && (IsTitleBarCondensed() || delegate_->IsFullscreen())) ?
-      0 : kFrameBorderThickness;
+  return !restored && delegate_->IsFrameCondensed() ? 0 : kFrameBorderThickness;
 }
 
 int OpaqueBrowserFrameViewLayout::FrameTopBorderThickness(bool restored) const {
   int thickness = FrameBorderThickness(restored);
-  if (MD::IsRefreshUi() &&
-      (restored || (!IsTitleBarCondensed() && !delegate_->IsFullscreen())))
+  if (MD::IsRefreshUi() && (restored || !delegate_->IsFrameCondensed()))
     thickness += kRefreshNonClientExtraTopThickness;
   return thickness;
 }
@@ -163,8 +161,7 @@ bool OpaqueBrowserFrameViewLayout::HasClientEdge() const {
 int OpaqueBrowserFrameViewLayout::NonClientBorderThickness() const {
   const int frame = FrameBorderThickness(false);
   // When we fill the screen, we don't show a client edge.
-  return (!HasClientEdge() || IsTitleBarCondensed() ||
-          delegate_->IsFullscreen())
+  return !HasClientEdge() || delegate_->IsFrameCondensed()
              ? frame
              : (frame + views::NonClientFrameView::kClientEdgeThickness);
 }
@@ -194,7 +191,7 @@ int OpaqueBrowserFrameViewLayout::NonClientTopHeight(bool restored) const {
 
 int OpaqueBrowserFrameViewLayout::GetTabStripInsetsTop(bool restored) const {
   const int top = NonClientTopHeight(restored);
-  return (!restored && (IsTitleBarCondensed() || delegate_->IsFullscreen()))
+  return !restored || !delegate_->IsFrameCondensed()
              ? top
              : (top + GetNonClientRestoredExtraThickness());
 }
@@ -202,17 +199,18 @@ int OpaqueBrowserFrameViewLayout::GetTabStripInsetsTop(bool restored) const {
 int OpaqueBrowserFrameViewLayout::TitlebarTopThickness(bool restored) const {
   if (!delegate_->UseCustomFrame())
     return 0;
-  return (restored || !IsTitleBarCondensed()) ?
-      kTitlebarTopEdgeThickness : FrameBorderThickness(false);
+  return restored || !delegate_->IsFrameCondensed()
+             ? kTitlebarTopEdgeThickness
+             : FrameBorderThickness(false);
 }
 
 int OpaqueBrowserFrameViewLayout::DefaultCaptionButtonY(bool restored) const {
   // Maximized buttons start at window top, since the window has no border. This
   // offset is for the image (the actual clickable bounds extend all the way to
   // the top to take Fitts' Law into account).
-  const int frame = (!restored && IsTitleBarCondensed()) ?
-      FrameBorderThickness(false) :
-      views::NonClientFrameView::kFrameShadowThickness;
+  const int frame = !restored && delegate_->IsFrameCondensed()
+                        ? FrameBorderThickness(false)
+                        : views::NonClientFrameView::kFrameShadowThickness;
   return frame + extra_caption_y_;
 }
 
@@ -266,9 +264,9 @@ int OpaqueBrowserFrameViewLayout::GetWindowCaptionSpacing(
     if (is_leading_button) {
       // If we're the first button and maximized, add width to the right
       // hand side of the screen.
-      return (IsTitleBarCondensed() && is_leading_button)
-                 ? (kFrameBorderThickness -
-                    views::NonClientFrameView::kFrameShadowThickness)
+      return delegate_->IsFrameCondensed() && is_leading_button
+                 ? kFrameBorderThickness -
+                       views::NonClientFrameView::kFrameShadowThickness
                  : 0;
     }
     if (forced_window_caption_spacing_ >= 0)
@@ -278,13 +276,6 @@ int OpaqueBrowserFrameViewLayout::GetWindowCaptionSpacing(
   return 0;
 }
 
-bool OpaqueBrowserFrameViewLayout::IsTitleBarCondensed() const {
-  // If there are no caption buttons, there is no need to have an uncondensed
-  // title bar. If the window is maximized, the title bar is condensed
-  // regardless of whether there are caption buttons.
-  return !delegate_->ShouldShowCaptionButtons() || delegate_->IsMaximized();
-}
-
 int OpaqueBrowserFrameViewLayout::GetNonClientRestoredExtraThickness() const {
   // Besides the frame border, there's empty space atop the window in restored
   // mode, to use to drag the window around.
@@ -315,14 +306,14 @@ void OpaqueBrowserFrameViewLayout::LayoutNewStyleAvatar(views::View* host) {
     button_width_with_offset += kCaptionSpacing;
 
   const int button_x = available_space_trailing_x_ - button_width_with_offset;
-  const int button_y = DefaultCaptionButtonY(!IsTitleBarCondensed());
+  const int button_y = DefaultCaptionButtonY(!delegate_->IsFrameCondensed());
 
   minimum_size_for_buttons_ += button_width_with_offset;
   available_space_trailing_x_ -= button_width_with_offset;
 
   // In non-maximized mode, allow the new tab button to completely slide under
   // the avatar button.
-  if (!IsTitleBarCondensed()) {
+  if (!delegate_->IsFrameCondensed()) {
     available_space_trailing_x_ +=
         delegate_->GetNewTabButtonPreferredSize().width() + kCaptionSpacing;
   }
@@ -353,8 +344,9 @@ int OpaqueBrowserFrameViewLayout::TabStripCaptionSpacing() const {
   if (MD::IsRefreshUi())
     return 0;
 
-  return (has_trailing_buttons_ && IsTitleBarCondensed()) ?
-      kNewTabCaptionCondensedSpacing : kCaptionSpacing;
+  return has_trailing_buttons_ && delegate_->IsFrameCondensed()
+             ? kNewTabCaptionCondensedSpacing
+             : kCaptionSpacing;
 }
 
 void OpaqueBrowserFrameViewLayout::LayoutWindowControls(views::View* host) {
@@ -554,7 +546,7 @@ void OpaqueBrowserFrameViewLayout::SetBoundsForButton(
   // side of the caption buttons.  In maximized mode we extend buttons to the
   // screen top and the rightmost button to the screen right (or leftmost button
   // to the screen left, for left-aligned buttons) to obey Fitts' Law.
-  bool title_bar_condensed = IsTitleBarCondensed();
+  bool is_frame_condensed = delegate_->IsFrameCondensed();
 
   // When we are the first button on the leading side and are the close
   // button, we must flip ourselves, because the close button assets have
@@ -571,9 +563,9 @@ void OpaqueBrowserFrameViewLayout::SetBoundsForButton(
       available_space_leading_x_ += button_start_spacing;
       minimum_size_for_buttons_ += button_start_spacing;
 
-      bool top_spacing_clickable = title_bar_condensed;
+      bool top_spacing_clickable = is_frame_condensed;
       bool start_spacing_clickable =
-          title_bar_condensed && !has_leading_buttons_;
+          is_frame_condensed && !has_leading_buttons_;
       button->SetBounds(
           available_space_leading_x_ - (start_spacing_clickable
                                             ? button_start_spacing + extra_width
@@ -598,9 +590,9 @@ void OpaqueBrowserFrameViewLayout::SetBoundsForButton(
       available_space_trailing_x_ -= button_start_spacing;
       minimum_size_for_buttons_ += button_start_spacing;
 
-      bool top_spacing_clickable = title_bar_condensed;
+      bool top_spacing_clickable = is_frame_condensed;
       bool start_spacing_clickable =
-          title_bar_condensed && !has_trailing_buttons_;
+          is_frame_condensed && !has_trailing_buttons_;
       button->SetBounds(
           available_space_trailing_x_ - button_size.width(),
           top_spacing_clickable ? 0 : caption_y,

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

@@ -134,12 +134,6 @@ class OpaqueBrowserFrameViewLayout : public views::LayoutManager {
 
   const gfx::Rect& client_view_bounds() const { return client_view_bounds_; }
 
-  // Determines whether the title bar is condensed vertically, as when the
-  // window is maximized. If true, the title bar is just the height of a tab,
-  // rather than having extra vertical space above the tabs. This also removes
-  // the thick frame border and rounded corners.
-  bool IsTitleBarCondensed() const;
-
   // Returns the extra thickness of the area above the tabs. The value returned
   // is dependent on whether in material refresh mode or not.
   int GetNonClientRestoredExtraThickness() const;
@@ -205,7 +199,7 @@ class OpaqueBrowserFrameViewLayout : public views::LayoutManager {
   // Internal implementation of ViewAdded() and ViewRemoved().
   void SetView(int id, views::View* view);
 
-  // Overriden from views::LayoutManager:
+  // views::LayoutManager:
   void Layout(views::View* host) override;
   gfx::Size GetPreferredSize(const views::View* host) const override;
   void ViewAdded(views::View* host, views::View* view) override;

chrome/browser/ui/views/frame/opaque_browser_frame_view_layout_delegate.h

@@ -42,7 +42,6 @@ class OpaqueBrowserFrameViewLayoutDelegate {
   // Controls window state.
   virtual bool IsMaximized() const = 0;
   virtual bool IsMinimized() const = 0;
-  virtual bool IsFullscreen() const = 0;
 
   virtual bool IsTabStripVisible() const = 0;
   virtual int GetTabStripHeight() const = 0;
@@ -61,6 +60,12 @@ class OpaqueBrowserFrameViewLayoutDelegate {
   // Returns true if the window frame is rendered by Chrome.
   virtual bool UseCustomFrame() const = 0;
 
+  // Determines whether the top frame is condensed vertically, as when the
+  // window is maximized. If true, the top frame is just the height of a tab,
+  // rather than having extra vertical space above the tabs. This also removes
+  // the thick frame border and rounded corners.
+  virtual bool IsFrameCondensed() const = 0;
+
   // Returns whether the shapes of background tabs are visible against the frame
   // for either active or inactive windows.
   virtual bool EverHasVisibleBackgroundTabShapes() const = 0;

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

@@ -70,7 +70,6 @@ class TestLayoutDelegate : public OpaqueBrowserFrameViewLayoutDelegate {
   }
   bool IsMaximized() const override { return maximized_; }
   bool IsMinimized() const override { return false; }
-  bool IsFullscreen() const override { return false; }
   bool IsTabStripVisible() const override { return window_title_.empty(); }
   int GetTabStripHeight() const override {
     return IsTabStripVisible() ? GetLayoutConstant(TAB_HEIGHT) : 0;
@@ -84,6 +83,9 @@ class TestLayoutDelegate : public OpaqueBrowserFrameViewLayoutDelegate {
   }
   int GetTopAreaHeight() const override { return 0; }
   bool UseCustomFrame() const override { return true; }
+  bool IsFrameCondensed() const override {
+    return !show_caption_buttons_ || maximized_;
+  }
   bool EverHasVisibleBackgroundTabShapes() const override { return false; }
 
  private:

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

@@ -380,6 +380,10 @@ void BrowserTabStripController::OnStoppedDraggingTabs() {
   immersive_reveal_lock_.reset();
 }
 
+bool BrowserTabStripController::IsFrameCondensed() const {
+  return GetFrameView()->IsFrameCondensed();
+}
+
 bool BrowserTabStripController::HasVisibleBackgroundTabShapes() const {
   return GetFrameView()->HasVisibleBackgroundTabShapes(
       BrowserNonClientFrameView::kUseCurrent);

chrome/browser/ui/views/tabs/browser_tab_strip_controller.h

@@ -75,6 +75,7 @@ class BrowserTabStripController : public TabStripController,
   bool ShouldDrawStrokes() const override;
   void OnStartedDraggingTabs() override;
   void OnStoppedDraggingTabs() override;
+  bool IsFrameCondensed() const override;
   bool HasVisibleBackgroundTabShapes() const override;
   bool EverHasVisibleBackgroundTabShapes() const override;
   SkColor GetFrameColor() const override;

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

@@ -140,10 +140,12 @@ bool FakeBaseTabStripController::ShouldDrawStrokes() const {
   return false;
 }
 
-void FakeBaseTabStripController::OnStartedDraggingTabs() {
-}
+void FakeBaseTabStripController::OnStartedDraggingTabs() {}
+
+void FakeBaseTabStripController::OnStoppedDraggingTabs() {}
 
-void FakeBaseTabStripController::OnStoppedDraggingTabs() {
+bool FakeBaseTabStripController::IsFrameCondensed() const {
+  return false;
 }
 
 bool FakeBaseTabStripController::HasVisibleBackgroundTabShapes() const {

chrome/browser/ui/views/tabs/fake_base_tab_strip_controller.h

@@ -51,6 +51,7 @@ class FakeBaseTabStripController : public TabStripController {
   bool ShouldDrawStrokes() const override;
   void OnStartedDraggingTabs() override;
   void OnStoppedDraggingTabs() override;
+  bool IsFrameCondensed() const override;
   bool HasVisibleBackgroundTabShapes() const override;
   bool EverHasVisibleBackgroundTabShapes() const override;
   SkColor GetFrameColor() const override;

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

@@ -219,8 +219,9 @@ bool NewTabButton::GetHitTestMask(gfx::Path* mask) const {
 
   const float scale = GetWidget()->GetCompositor()->device_scale_factor();
   // TODO(pkasting): Fitts' Law horizontally when appropriate.
-  gfx::Path border = GetBorderPath(GetContentsBounds().origin(), scale,
-                                   tab_strip_->SizeTabButtonToTopOfTabStrip());
+  gfx::Path border =
+      GetBorderPath(GetContentsBounds().origin(), scale,
+                    tab_strip_->controller()->IsFrameCondensed());
   mask->addPath(border, SkMatrix::MakeScale(1 / scale));
   return true;
 }

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

@@ -347,7 +347,7 @@ bool TabStrip::IsRectInWindowCaption(const gfx::Rect& rect) {
       (height() - Tab::GetTabSeparatorHeight()) / 2 - 1;
 
   // Disable drag handle extension when tab shapes are visible.
-  bool extend_drag_handle = !SizeTabButtonToTopOfTabStrip() &&
+  bool extend_drag_handle = !controller_->IsFrameCondensed() &&
                             !controller_->EverHasVisibleBackgroundTabShapes();
 
   // A hit on the tab is not in the caption unless it is in the thin strip
@@ -427,13 +427,6 @@ void TabStrip::SetStackedLayout(bool stacked_layout) {
     tab_at(i)->Layout();
 }
 
-bool TabStrip::SizeTabButtonToTopOfTabStrip() {
-  // Extend the button to the screen edge in maximized and immersive fullscreen.
-  views::Widget* widget = GetWidget();
-  return browser_defaults::kSizeTabButtonToTopOfTabStrip ||
-         (widget && (widget->IsMaximized() || widget->IsFullscreen()));
-}
-
 void TabStrip::StartHighlight(int model_index) {
   tab_at(model_index)->StartPulse();
 }

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

@@ -131,10 +131,6 @@ class TabStrip : public views::View,
   // Returns the bounds of the new tab button.
   gfx::Rect new_tab_button_bounds() const { return new_tab_button_bounds_; }
 
-  // Returns true if the new tab button should be sized to the top of the tab
-  // strip.
-  bool SizeTabButtonToTopOfTabStrip();
-
   // Starts highlighting the tab at the specified index.
   void StartHighlight(int model_index);
 

chrome/browser/ui/views/tabs/tab_strip_controller.h

@@ -115,6 +115,11 @@ class TabStripController {
   // from this tabstrip but the user is still dragging the tabs.
   virtual void OnStoppedDraggingTabs() = 0;
 
+  // Determines whether the top frame is condensed vertically, as when the
+  // window is maximized. If true, the top frame is just the height of a tab,
+  // rather than having extra vertical space above the tabs.
+  virtual bool IsFrameCondensed() const = 0;
+
   // Returns whether the shapes of background tabs are visible against the
   // frame.
   virtual bool HasVisibleBackgroundTabShapes() const = 0;