Initial work on MD Refresh new tab button position.

Does not implement padding in spec.

BUG=822063
TEST=none

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

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

@@ -124,6 +124,26 @@ int GetNewTabButtonWidth(bool is_incognito) {
          GetLayoutConstant(TABSTRIP_NEW_TAB_BUTTON_SPACING);
 }
 
+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.
+};
+
+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 {
@@ -321,7 +341,17 @@ void TabStrip::RemoveObserver(TabStripObserver* observer) {
 }
 
 int TabStrip::GetMaxX() const {
-  return new_tab_button_bounds_.right();
+  // This function should not currently be called for TRAILING mode; if it is,
+  // the API will need changing, since callers assume the tabstrip uses
+  // contiguous space from 0 to GetMaxX() and not after, and TRAILING doesn't
+  // work like that.
+  const auto position = GetNewTabButtonPosition();
+  DCHECK_NE(TRAILING, position);
+
+  const gfx::Rect& last_object_bounds = (position == AFTER_TABS)
+                                            ? new_tab_button_bounds_
+                                            : ideal_bounds(tab_count() - 1);
+  return last_object_bounds.right();
 }
 
 void TabStrip::SetBackgroundOffset(const gfx::Point& offset) {
@@ -490,7 +520,8 @@ void TabStrip::MoveTab(int from_model_index,
     tabs_.Move(from_model_index, to_model_index);
   }
   StartMoveTabAnimation();
-  if (TabDragController::IsAttachedTo(this) &&
+  if (ShouldHideNewTabButtonWhileDragging() &&
+      TabDragController::IsAttachedTo(this) &&
       (last_tab != GetLastVisibleTab() || last_tab->dragging())) {
     new_tab_button_->SetVisible(false);
   }
@@ -566,17 +597,21 @@ bool TabStrip::ShouldTabBeVisible(const Tab* tab) const {
     return true;
 
   // If the tab is currently clipped, it shouldn't be visible.  Note that we
-  // allow dragged tabs to draw over the "New Tab button" region as well,
-  // because either the New Tab button will be hidden, or the dragged tabs will
-  // be animating back to their normal positions and we don't want to hide them
-  // in the New Tab button region in case they re-appear after leaving it.
+  // allow dragged tabs to draw over any trailing "New Tab button" region as
+  // well, because either the New Tab button will be hidden, or the dragged tabs
+  // will be animating back to their normal positions and we don't want to hide
+  // them in the New Tab button region in case they re-appear after leaving it.
   // (This prevents flickeriness.)  We never draw non-dragged tabs in New Tab
   // button area, even when the button is invisible, so that they don't appear
   // to "pop in" when the button disappears.
   // TODO: Probably doesn't work for RTL
   int right_edge = tab->bounds().right();
-  const int visible_width = tab->dragging() ? width() : GetTabAreaWidth();
-  if (right_edge > visible_width)
+  const bool trailing_new_tab_button =
+      (GetNewTabButtonPosition() == TRAILING) ||
+      (ShouldHideNewTabButtonWhileDragging() && !tab->dragging());
+  const int tabstrip_right =
+      trailing_new_tab_button ? GetTabAreaWidth() : width();
+  if (right_edge > tabstrip_right)
     return false;
 
   // Non-clipped dragging tabs should always be visible.
@@ -605,7 +640,7 @@ bool TabStrip::ShouldTabBeVisible(const Tab* tab) const {
   // We need to check what would happen if the active tab were to move to this
   // tab or before.
   return (right_edge + current_active_width_ - current_inactive_width_) <=
-         GetTabAreaWidth();
+         tabstrip_right;
 }
 
 void TabStrip::PrepareForCloseAt(int model_index, CloseTabSource source) {
@@ -626,7 +661,7 @@ void TabStrip::PrepareForCloseAt(int model_index, CloseTabSource source) {
     // user closes tabs with the mouse a tab continues to fall under the mouse.
     Tab* last_tab = tab_at(model_count - 1);
     Tab* tab_being_removed = tab_at(model_index);
-    available_width_for_tabs_ = last_tab->bounds().right() -
+    available_width_for_tabs_ = last_tab->bounds().right() - TabStartX() -
                                 tab_being_removed->width() + Tab::GetOverlap();
     if (model_index == 0 && tab_being_removed->data().pinned &&
         !tab_at(1)->data().pinned) {
@@ -898,7 +933,7 @@ void TabStrip::MaybeStartDrag(
       tabs.push_back(other_tab);
       if (other_tab == tab) {
         size_to_selected = GetSizeNeededForTabs(tabs);
-        x = size_to_selected - tab->width() + x;
+        x += size_to_selected - tab->width();
       }
     }
   }
@@ -1366,7 +1401,7 @@ void TabStrip::StartInsertTabAnimation(int model_index) {
   // Set the current bounds to be the correct place but 0 width.
   Tab* tab = tab_at(model_index);
   if (model_index == 0) {
-    tab->SetBounds(0, ideal_bounds(model_index).y(), 0,
+    tab->SetBounds(TabStartX(), ideal_bounds(model_index).y(), 0,
                    ideal_bounds(model_index).height());
   } else {
     Tab* prev_tab = tab_at(model_index - 1);
@@ -1572,7 +1607,8 @@ void TabStrip::StackDraggedTabs(int delta) {
       new_bounds.set_x(std::min(min_x, new_bounds.x() + delta));
       tabs_.set_ideal_bounds(i, new_bounds);
     }
-    if (ideal_bounds(tab_count() - 1).right() >= new_tab_button_->x())
+    if ((GetNewTabButtonPosition() == AFTER_TABS) &&
+        (ideal_bounds(tab_count() - 1).right() >= new_tab_button_->x()))
       new_tab_button_->SetVisible(false);
   }
   views::ViewModelUtils::SetViewBoundsToIdealBounds(tabs_);
@@ -1591,7 +1627,8 @@ 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 (last_visible_tab && last_visible_tab->dragging())
+  if (ShouldHideNewTabButtonWhileDragging() && last_visible_tab &&
+      last_visible_tab->dragging())
     new_tab_button_->SetVisible(false);
   std::vector<gfx::Rect> bounds;
   CalculateBoundsForDraggedTabs(tabs, &bounds);
@@ -1633,6 +1670,31 @@ void TabStrip::CalculateBoundsForDraggedTabs(const Tabs& tabs,
   }
 }
 
+int TabStrip::TabStartX() const {
+  return (GetNewTabButtonPosition() == LEADING)
+             ? GetNewTabButtonWidth(IsIncognito())
+             : 0;
+}
+
+int TabStrip::NewTabButtonX() const {
+  const auto position = GetNewTabButtonPosition();
+  if (position == LEADING)
+    return 0;
+
+  // This is not GetTabAreaWidth() because we don't want to subtract the
+  // separator region between the tabs and the new tab button.
+  const int tab_area_width = width() - new_tab_button_bounds_.width();
+  if (position == TRAILING)
+    return tab_area_width;
+
+  // For non-stacked tabs the ideal bounds may go outside the bounds of the
+  // tabstrip. Constrain the x-coordinate of the new tab button so that it is
+  // always visible.
+  return std::min(tab_area_width,
+                  tabs_.ideal_bounds(tabs_.view_size() - 1).right() +
+                      GetLayoutConstant(TABSTRIP_NEW_TAB_BUTTON_SPACING));
+}
+
 int TabStrip::GetSizeNeededForTabs(const Tabs& tabs) {
   int width = 0;
   for (size_t i = 0; i < tabs.size(); ++i) {
@@ -1708,7 +1770,7 @@ void TabStrip::StartedDraggingTabs(const Tabs& tabs) {
   controller_->OnStartedDraggingTabs();
 
   // Hide the new tab button immediately if we didn't originate the drag.
-  if (!drag_controller_.get())
+  if (ShouldHideNewTabButtonWhileDragging() && !drag_controller_)
     new_tab_button_->SetVisible(false);
 
   PrepareForAnimation();
@@ -2121,30 +2183,25 @@ void TabStrip::GenerateIdealBounds() {
   if (tab_count() == 0)
     return;  // Should only happen during creation/destruction, ignore.
 
+  const int old_max_x = GetMaxX();
+
   if (!touch_layout_) {
     const int available_width = (available_width_for_tabs_ < 0)
                                     ? GetTabAreaWidth()
                                     : available_width_for_tabs_;
-    const std::vector<gfx::Rect> tabs_bounds =
-        CalculateBounds(GetTabSizeInfo(), GetPinnedTabCount(), tab_count(),
-                        controller_->GetActiveIndex(), available_width,
-                        &current_active_width_, &current_inactive_width_);
+    const std::vector<gfx::Rect> tabs_bounds = CalculateBounds(
+        GetTabSizeInfo(), GetPinnedTabCount(), tab_count(),
+        controller_->GetActiveIndex(), TabStartX(), available_width,
+        &current_active_width_, &current_inactive_width_);
     DCHECK_EQ(static_cast<size_t>(tab_count()), tabs_bounds.size());
 
     for (size_t i = 0; i < tabs_bounds.size(); ++i)
       tabs_.set_ideal_bounds(i, tabs_bounds[i]);
   }
 
-  const int max_new_tab_x = width() - new_tab_button_bounds_.width();
-  // For non-stacked tabs the ideal bounds may go outside the bounds of the
-  // tabstrip. Constrain the x-coordinate of the new tab button so that it is
-  // always visible.
-  const int new_tab_x = std::min(
-      max_new_tab_x, tabs_.ideal_bounds(tabs_.view_size() - 1).right() +
-                         GetLayoutConstant(TABSTRIP_NEW_TAB_BUTTON_SPACING));
-  const int old_max_x = new_tab_button_bounds_.right();
-  new_tab_button_bounds_.set_origin(gfx::Point(new_tab_x, 0));
-  if (new_tab_button_bounds_.right() != old_max_x) {
+  new_tab_button_bounds_.set_origin(gfx::Point(NewTabButtonX(), 0));
+
+  if (GetMaxX() != old_max_x) {
     for (TabStripObserver& observer : observers_)
       observer.TabStripMaxXChanged(this);
   }
@@ -2156,12 +2213,13 @@ int TabStrip::GenerateIdealBoundsForPinnedTabs(int* first_non_pinned_index) {
   if (first_non_pinned_index)
     *first_non_pinned_index = num_pinned_tabs;
 
+  const int start_x = TabStartX();
   if (num_pinned_tabs == 0)
-    return 0;
+    return start_x;
 
   std::vector<gfx::Rect> tab_bounds(tab_count());
   CalculateBoundsForPinnedTabs(GetTabSizeInfo(), num_pinned_tabs, tab_count(),
-                               &tab_bounds);
+                               start_x, &tab_bounds);
   for (int i = 0; i < num_pinned_tabs; ++i)
     tabs_.set_ideal_bounds(i, tab_bounds[i]);
   return (num_pinned_tabs < tab_count())
@@ -2219,10 +2277,12 @@ bool TabStrip::IsPointInTab(Tab* tab,
 }
 
 int TabStrip::GetStartXForNormalTabs() const {
-  int pinned_tab_count = GetPinnedTabCount();
+  const int start_x = TabStartX();
+  const int pinned_tab_count = GetPinnedTabCount();
   if (pinned_tab_count == 0)
-    return 0;
-  return pinned_tab_count * (Tab::GetPinnedWidth() - Tab::GetOverlap()) +
+    return start_x;
+  return start_x +
+         pinned_tab_count * (Tab::GetPinnedWidth() - Tab::GetOverlap()) +
          kPinnedToNonPinnedOffset;
 }
 

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

@@ -172,11 +172,12 @@ class TabStrip : public views::View,
   void SetTabNeedsAttention(int model_index, bool attention);
 
   // Retrieves the ideal bounds for the Tab at the specified index.
-  const gfx::Rect& ideal_bounds(int tab_data_index) {
+  const gfx::Rect& ideal_bounds(int tab_data_index) const {
     return tabs_.ideal_bounds(tab_data_index);
   }
 
   // Returns the Tab at |index|.
+  // TODO(pkasting): Make const correct
   Tab* tab_at(int index) const { return tabs_.view_at(index); }
 
   // Returns the NewTabButton.
@@ -387,6 +388,13 @@ class TabStrip : public views::View,
   void CalculateBoundsForDraggedTabs(const Tabs& tabs,
                                      std::vector<gfx::Rect>* bounds);
 
+  // Returns the X coordinate the first tab should start at.
+  int TabStartX() const;
+
+  // Returns the X coordinate the new tab button should be placed at.  Requires
+  // |tabs_| to have correct ideal bounds.
+  int NewTabButtonX() const;
+
   // Returns the size needed for the specified tabs. This is invoked during drag
   // and drop to calculate offsets and positioning.
   int GetSizeNeededForTabs(const Tabs& tabs);

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

@@ -52,27 +52,29 @@ void CalculateNormalTabWidths(const TabSizeInfo& tab_size_info,
 
 }  // namespace
 
-void CalculateBoundsForPinnedTabs(const TabSizeInfo& tab_size_info,
-                                  int num_pinned_tabs,
-                                  int num_tabs,
-                                  std::vector<gfx::Rect>* tabs_bounds) {
+int CalculateBoundsForPinnedTabs(const TabSizeInfo& tab_size_info,
+                                 int num_pinned_tabs,
+                                 int num_tabs,
+                                 int start_x,
+                                 std::vector<gfx::Rect>* tabs_bounds) {
   DCHECK_EQ(static_cast<size_t>(num_tabs), tabs_bounds->size());
   int index = 0;
-  int next_x = 0;
+  int next_x = start_x;
   for (; index < num_pinned_tabs; ++index) {
     (*tabs_bounds)[index].SetRect(next_x, 0, tab_size_info.pinned_tab_width,
                                   tab_size_info.max_size.height());
     next_x += tab_size_info.pinned_tab_width - tab_size_info.tab_overlap;
   }
-  if (index > 0 && index < num_tabs) {
-    (*tabs_bounds)[index].set_x(next_x + tab_size_info.pinned_to_normal_offset);
-  }
+  if (num_pinned_tabs)
+    next_x += tab_size_info.pinned_to_normal_offset;
+  return next_x;
 }
 
 std::vector<gfx::Rect> CalculateBounds(const TabSizeInfo& tab_size_info,
                                        int num_pinned_tabs,
                                        int num_tabs,
                                        int active_index,
+                                       int start_x,
                                        int width,
                                        int* active_width,
                                        int* inactive_width) {
@@ -82,18 +84,11 @@ std::vector<gfx::Rect> CalculateBounds(const TabSizeInfo& tab_size_info,
 
   *active_width = *inactive_width = tab_size_info.max_size.width();
 
-  int next_x = 0;
-  if (num_pinned_tabs) {
-    CalculateBoundsForPinnedTabs(tab_size_info, num_pinned_tabs, num_tabs,
-                                 &tabs_bounds);
-    if (num_pinned_tabs == num_tabs)
-      return tabs_bounds;
-
-    // CalculateBoundsForPinnedTabs() sets the x location of the first normal
-    // tab.
-    width -= tabs_bounds[num_pinned_tabs].x();
-    next_x = tabs_bounds[num_pinned_tabs].x();
-  }
+  int next_x = CalculateBoundsForPinnedTabs(tab_size_info, num_pinned_tabs,
+                                            num_tabs, start_x, &tabs_bounds);
+  if (num_pinned_tabs == num_tabs)
+    return tabs_bounds;
+  width -= next_x - start_x;
 
   const bool is_active_tab_normal = active_index >= num_pinned_tabs;
   const int num_normal_tabs = num_tabs - num_pinned_tabs;

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

@@ -35,13 +35,13 @@ struct TabSizeInfo {
 };
 
 // Calculates the bounds of the pinned tabs. This assumes |tabs_bounds| is the
-// same size as |num_tabs|. In addition to setting the bounds of the pinned
-// tabs this sets the x-coordinate of the first normal tab (as long as there is
-// a normal tab).
-void CalculateBoundsForPinnedTabs(const TabSizeInfo& tab_size_info,
-                                  int num_pinned_tabs,
-                                  int num_tabs,
-                                  std::vector<gfx::Rect>* tabs_bounds);
+// same size as |num_tabs|. Returns the x-coordinate to use for the first
+// non-pinned tab, if any.
+int CalculateBoundsForPinnedTabs(const TabSizeInfo& tab_size_info,
+                                 int num_pinned_tabs,
+                                 int num_tabs,
+                                 int start_x,
+                                 std::vector<gfx::Rect>* tabs_bounds);
 
 // Calculates and returns the bounds of the tabs. |width| is the available
 // width to use for tab layout. This never sizes the tabs smaller then the
@@ -51,6 +51,7 @@ std::vector<gfx::Rect> CalculateBounds(const TabSizeInfo& tab_size_info,
                                        int num_pinned_tabs,
                                        int num_tabs,
                                        int active_index,
+                                       int start_x,
                                        int width,
                                        int* active_width,
                                        int* inactive_width);

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

@@ -41,65 +41,69 @@ TEST(TabStripLayoutTest, Tests) {
     int num_pinned_tabs;
     int num_tabs;
     int active_index;
+    int start_x;
     int width;
     const char* expected_sizes;
     int expected_active_width;
     int expected_inactive_width;
   } test_cases[] = {
       // Ample space, all normal tabs.
-      {0, 3, 0, 1000, "0 100, 96 100, 192 100", 100, 100},
+      {0, 3, 0, 0, 1000, "0 100, 96 100, 192 100", 100, 100},
+
+      // Ample space, all normal tabs, starting at a nonzero value.
+      {0, 3, 0, 100, 1000, "100 100, 196 100, 292 100", 100, 100},
 
       // Ample space, all pinned.
-      {3, 3, 0, 1000, "0 10, 6 10, 12 10", 100, 100},
+      {3, 3, 0, 0, 1000, "0 10, 6 10, 12 10", 100, 100},
 
       // Ample space, one pinned and two normal tabs.
-      {1, 3, 0, 1000, "0 10, 12 100, 108 100", 100, 100},
+      {1, 3, 0, 0, 1000, "0 10, 12 100, 108 100", 100, 100},
 
       // Resize between min and max, no pinned and no rounding.
-      {0, 4, 0, 100, "0 28, 24 28, 48 28, 72 28", 28, 28},
+      {0, 4, 0, 0, 100, "0 28, 24 28, 48 28, 72 28", 28, 28},
 
       // Resize between min and max, pinned and no rounding.
-      {1, 3, 0, 100, "0 10, 12 46, 54 46", 46, 46},
+      {1, 3, 0, 0, 100, "0 10, 12 46, 54 46", 46, 46},
 
       // Resize between min and max, no pinned, rounding.
-      {0, 4, 0, 102, "0 29, 25 29, 50 28, 74 28", 28, 28},
+      {0, 4, 0, 0, 102, "0 29, 25 29, 50 28, 74 28", 28, 28},
 
       // Resize between min and max, pinned and rounding.
-      {1, 3, 0, 101, "0 10, 12 47, 55 46", 46, 46},
+      {1, 3, 0, 0, 101, "0 10, 12 47, 55 46", 46, 46},
 
       // Resize between active/inactive width, only one tab.
-      {0, 1, 0, 15, "0 20", 20, 15},
+      {0, 1, 0, 0, 15, "0 20", 20, 15},
 
       // Resize between active/inactive width and no rounding.
-      {0, 6, 0, 90, "0 20, 16 18, 30 18, 44 18, 58 18, 72 18", 20, 18},
+      {0, 6, 0, 0, 90, "0 20, 16 18, 30 18, 44 18, 58 18, 72 18", 20, 18},
 
       // Resize between active/inactive width, rounding.
-      {0, 6, 0, 93, "0 20, 16 19, 31 19, 46 19, 61 18, 75 18", 20, 18},
+      {0, 6, 0, 0, 93, "0 20, 16 19, 31 19, 46 19, 61 18, 75 18", 20, 18},
 
       // Resize between active/inactive width, one pinned and active, no
       // rounding.
-      {1, 6, 0, 91, "0 10, 12 19, 27 19, 42 19, 57 19, 72 19", 20, 19},
+      {1, 6, 0, 0, 91, "0 10, 12 19, 27 19, 42 19, 57 19, 72 19", 20, 19},
 
       // Resize between active/inactive width, one pinned and active, rounding.
-      {1, 6, 0, 92, "0 10, 12 20, 28 19, 43 19, 58 19, 73 19", 20, 19},
+      {1, 6, 0, 0, 92, "0 10, 12 20, 28 19, 43 19, 58 19, 73 19", 20, 19},
 
       // Not enough space, all pinned.
-      {3, 3, 0, 10, "0 10, 6 10, 12 10", 100, 100},
+      {3, 3, 0, 0, 10, "0 10, 6 10, 12 10", 100, 100},
 
       // Not enough space (for minimum widths), all normal.
-      {0, 3, 0, 10, "0 20, 16 14, 26 14", 20, 14},
+      {0, 3, 0, 0, 10, "0 20, 16 14, 26 14", 20, 14},
 
       // Not enough space (for minimum widths), one pinned and two normal.
-      {1, 3, 0, 10, "0 10, 12 14, 22 14", 20, 14},
+      {1, 3, 0, 0, 10, "0 10, 12 14, 22 14", 20, 14},
   };
 
   for (size_t i = 0; i < arraysize(test_cases); ++i) {
     int active_width;
     int inactive_width;
-    std::vector<gfx::Rect> tabs_bounds =
-        CalculateBounds(tab_size_info, test_cases[i].num_pinned_tabs,
-                        test_cases[i].num_tabs, test_cases[i].active_index,
-                        test_cases[i].width, &active_width, &inactive_width);
+    std::vector<gfx::Rect> tabs_bounds = CalculateBounds(
+        tab_size_info, test_cases[i].num_pinned_tabs, test_cases[i].num_tabs,
+        test_cases[i].active_index, test_cases[i].start_x, test_cases[i].width,
+        &active_width, &inactive_width);
     EXPECT_EQ(test_cases[i].expected_sizes, TabsBoundsToString(tabs_bounds))
         << i;
     EXPECT_EQ(test_cases[i].expected_active_width, active_width) << i;