Compute NTB position dynamically from caption button position.

This makes Refresh use the trailing NTB for high-contrast RTL on Windows, where
we draw with the native frame and the caption buttons are on the right (i.e.
leading) edge.

BUG=822063
TEST=none

Change-Id: I92b58e1db4aef83d76e6e64abb93f9549db84067
Reviewed-on: https://chromium-review.googlesource.com/1029197Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553848}

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

@@ -74,6 +74,10 @@ void BrowserNonClientFrameView::OnMaximizedStateChanged() {}
 
 void BrowserNonClientFrameView::OnFullscreenStateChanged() {}
 
+bool BrowserNonClientFrameView::CaptionButtonsOnLeadingEdge() const {
+  return false;
+}
+
 gfx::ImageSkia BrowserNonClientFrameView::GetIncognitoAvatarIcon() const {
   const SkColor icon_color = color_utils::PickContrastingColor(
       SK_ColorWHITE, gfx::kChromeIconGrey, GetFrameColor());

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

@@ -42,6 +42,10 @@ class BrowserNonClientFrameView : public views::NonClientFrameView,
   // unfullscreened.
   virtual void OnFullscreenStateChanged();
 
+  // Returns whether the caption buttons are drawn at the leading edge (i.e. the
+  // left in LTR mode, or the right in RTL mode).
+  virtual bool CaptionButtonsOnLeadingEdge() const;
+
   // Retrieves the bounds, in non-client view coordinates within which the
   // TabStrip should be laid out.
   virtual gfx::Rect GetBoundsForTabStrip(views::View* tabstrip) const = 0;

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

@@ -17,6 +17,7 @@ class BrowserNonClientFrameViewMac : public BrowserNonClientFrameView {
   ~BrowserNonClientFrameViewMac() override;
 
   // BrowserNonClientFrameView:
+  bool CaptionButtonsOnLeadingEdge() const override;
   gfx::Rect GetBoundsForTabStrip(views::View* tabstrip) const override;
   int GetTopInset(bool restored) const override;
   int GetThemeBackgroundXInset() const override;

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

@@ -39,6 +39,10 @@ BrowserNonClientFrameViewMac::~BrowserNonClientFrameViewMac() {
 ///////////////////////////////////////////////////////////////////////////////
 // BrowserNonClientFrameViewMac, BrowserNonClientFrameView implementation:
 
+bool BrowserNonClientFrameViewMac::CaptionButtonsOnLeadingEdge() const {
+  return true;
+}
+
 gfx::Rect BrowserNonClientFrameViewMac::GetBoundsForTabStrip(
     views::View* tabstrip) const {
   DCHECK(tabstrip);

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

@@ -142,6 +142,13 @@ GlassBrowserFrameView::~GlassBrowserFrameView() {
 ///////////////////////////////////////////////////////////////////////////////
 // GlassBrowserFrameView, BrowserNonClientFrameView implementation:
 
+bool GlassBrowserFrameView::CaptionButtonsOnLeadingEdge() const {
+  // Because we don't set WS_EX_LAYOUTRTL (which would conflict with Chrome's
+  // own RTL layout logic), Windows always draws the caption buttons on the
+  // right, even when we want to be RTL. See crbug.com/560619.
+  return !ShouldCustomDrawSystemTitlebar() && base::i18n::IsRTL();
+}
+
 gfx::Rect GlassBrowserFrameView::GetBoundsForTabStrip(
     views::View* tabstrip) const {
   const int x = GetTabStripLeftInset();
@@ -587,13 +594,6 @@ bool GlassBrowserFrameView::IsToolbarVisible() const {
       !browser_view()->toolbar()->GetPreferredSize().IsEmpty();
 }
 
-bool GlassBrowserFrameView::CaptionButtonsOnLeadingEdge() const {
-  // Because we don't set WS_EX_LAYOUTRTL (which would conflict with Chrome's
-  // own RTL layout logic), Windows always draws the caption buttons on the
-  // right, even when we want to be RTL. See crbug.com/560619.
-  return !ShouldCustomDrawSystemTitlebar() && base::i18n::IsRTL();
-}
-
 bool GlassBrowserFrameView::ShowCustomIcon() const {
   // Don't show the window icon when the incognito badge is visible, since
   // they're competing for the same space.

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

@@ -35,6 +35,7 @@ class GlassBrowserFrameView : public BrowserNonClientFrameView,
   ~GlassBrowserFrameView() override;
 
   // BrowserNonClientFrameView:
+  bool CaptionButtonsOnLeadingEdge() const override;
   gfx::Rect GetBoundsForTabStrip(views::View* tabstrip) const override;
   int GetTopInset(bool restored) const override;
   int GetThemeBackgroundXInset() const override;
@@ -128,10 +129,6 @@ class GlassBrowserFrameView : public BrowserNonClientFrameView,
   // Returns whether the toolbar is currently visible.
   bool IsToolbarVisible() const;
 
-  // Returns whether the caption buttons are drawn at the leading edge (i.e. the
-  // left in LTR mode, or the right in RTL mode).
-  bool CaptionButtonsOnLeadingEdge() const;
-
   bool ShowCustomIcon() const;
   bool ShowCustomTitle() const;
   bool ShowSystemIcon() const;

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

@@ -130,20 +130,6 @@ enum NewTabButtonPosition {
   TRAILING,    // Pinned to the trailing edge of the tabstrip region.
 };
 
-NewTabButtonPosition GetNewTabButtonPosition() {
-  if (MD::GetMode() != MD::MATERIAL_REFRESH)
-    return AFTER_TABS;
-#if defined(OS_MACOSX)
-  return TRAILING;
-#else
-  return LEADING;
-#endif
-}
-
-bool ShouldHideNewTabButtonWhileDragging() {
-  return GetNewTabButtonPosition() == AFTER_TABS;
-}
-
 // Animation delegate used for any automatic tab movement.  Hides the tab if it
 // is not fully visible within the tabstrip area, to prevent overflow clipping.
 class TabAnimationDelegate : public gfx::AnimationDelegate {
@@ -340,6 +326,15 @@ void TabStrip::RemoveObserver(TabStripObserver* observer) {
   observers_.RemoveObserver(observer);
 }
 
+TabStrip::NewTabButtonPosition TabStrip::GetNewTabButtonPosition() const {
+  if (MD::GetMode() != MD::MATERIAL_REFRESH)
+    return AFTER_TABS;
+
+  const auto* frame_view = static_cast<const BrowserNonClientFrameView*>(
+      GetWidget()->non_client_view()->frame_view());
+  return frame_view->CaptionButtonsOnLeadingEdge() ? TRAILING : LEADING;
+}
+
 int TabStrip::GetMaxX() const {
   // This function should not currently be called for TRAILING mode; if it is,
   // the API will need changing, since callers assume the tabstrip uses
@@ -520,7 +515,7 @@ void TabStrip::MoveTab(int from_model_index,
     tabs_.Move(from_model_index, to_model_index);
   }
   StartMoveTabAnimation();
-  if (ShouldHideNewTabButtonWhileDragging() &&
+  if (MayHideNewTabButtonWhileDragging() &&
       TabDragController::IsAttachedTo(this) &&
       (last_tab != GetLastVisibleTab() || last_tab->dragging())) {
     new_tab_button_->SetVisible(false);
@@ -608,7 +603,7 @@ bool TabStrip::ShouldTabBeVisible(const Tab* tab) const {
   int right_edge = tab->bounds().right();
   const bool trailing_new_tab_button =
       (GetNewTabButtonPosition() == TRAILING) ||
-      (ShouldHideNewTabButtonWhileDragging() && !tab->dragging());
+      (MayHideNewTabButtonWhileDragging() && !tab->dragging());
   const int tabstrip_right =
       trailing_new_tab_button ? GetTabAreaWidth() : width();
   if (right_edge > tabstrip_right)
@@ -1470,6 +1465,10 @@ bool TabStrip::ShouldHighlightCloseButtonAfterRemove() {
   return in_tab_close_;
 }
 
+bool TabStrip::MayHideNewTabButtonWhileDragging() const {
+  return GetNewTabButtonPosition() == AFTER_TABS;
+}
+
 bool TabStrip::TitlebarBackgroundIsTransparent() const {
 #if defined(OS_WIN)
   // Windows 8+ uses transparent window contents (because the titlebar area is
@@ -1627,7 +1626,7 @@ void TabStrip::LayoutDraggedTabsAt(const Tabs& tabs,
                                    bool initial_drag) {
   // Immediately hide the new tab button if the last tab is being dragged.
   const Tab* last_visible_tab = GetLastVisibleTab();
-  if (ShouldHideNewTabButtonWhileDragging() && last_visible_tab &&
+  if (MayHideNewTabButtonWhileDragging() && last_visible_tab &&
       last_visible_tab->dragging())
     new_tab_button_->SetVisible(false);
   std::vector<gfx::Rect> bounds;
@@ -1770,7 +1769,7 @@ void TabStrip::StartedDraggingTabs(const Tabs& tabs) {
   controller_->OnStartedDraggingTabs();
 
   // Hide the new tab button immediately if we didn't originate the drag.
-  if (ShouldHideNewTabButtonWhileDragging() && !drag_controller_)
+  if (MayHideNewTabButtonWhileDragging() && !drag_controller_)
     new_tab_button_->SetVisible(false);
 
   PrepareForAnimation();

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

@@ -62,6 +62,12 @@ class TabStrip : public views::View,
                  public views::ViewTargeterDelegate,
                  public TabController {
  public:
+  enum NewTabButtonPosition {
+    LEADING,     // Pinned to the leading edge of the tabstrip region.
+    AFTER_TABS,  // After the last tab.
+    TRAILING,    // Pinned to the trailing edge of the tabstrip region.
+  };
+
   explicit TabStrip(std::unique_ptr<TabStripController> controller);
   ~TabStrip() override;
 
@@ -72,6 +78,9 @@ class TabStrip : public views::View,
   void AddObserver(TabStripObserver* observer);
   void RemoveObserver(TabStripObserver* observer);
 
+  // Returns the position of the new tab button within the strip.
+  NewTabButtonPosition GetNewTabButtonPosition() const;
+
   // Max x-coordinate the tabstrip draws at, which is the right edge of the new
   // tab button.
   int GetMaxX() const;
@@ -348,9 +357,13 @@ class TabStrip : public views::View,
   // currently set in ideal_bounds.
   void AnimateToIdealBounds();
 
-  // Returns whether the highlight button should be highlighted after a remove.
+  // Returns whether the close button should be highlighted after a remove.
   bool ShouldHighlightCloseButtonAfterRemove();
 
+  // Returns whether dragging tabs should ever result in the new tab button
+  // being hidden.
+  bool MayHideNewTabButtonWhileDragging() const;
+
   // Returns whether the window background behind the tabstrip is transparent.
   bool TitlebarBackgroundIsTransparent() const;