Add basic grouing features.

Adds simple opener-based grouping to the experimental tab controller.

Clicks are not handled for activation. Tabs are rendered overlapped
properly.

Bug: 778461
Change-Id: Ic93cb109875d815018a6197e35b10b358959ac02
Reviewed-on: https://chromium-review.googlesource.com/777735Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Brett Wilson <brettw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517913}

chrome/browser/ui/tabs/tab_data_experimental.cc

@@ -58,10 +58,11 @@ TabDataExperimental::~TabDataExperimental() = default;
 TabDataExperimental& TabDataExperimental::operator=(TabDataExperimental&&) =
     default;
 
-const base::string16& TabDataExperimental::GetTitle() const {
-  DCHECK(type() == Type::kSingle);
+base::string16 TabDataExperimental::GetTitle() const {
   // TODO(brettw) this will need to use TabUIHelper.
-  return contents_->GetTitle();
+  if (contents_)
+    return contents_->GetTitle();
+  return base::string16();
 }
 
 bool TabDataExperimental::CountsAsViewIndex() const {

chrome/browser/ui/tabs/tab_data_experimental.h

@@ -49,12 +49,13 @@ class TabDataExperimental {
   TabDataExperimental* parent() { return parent_; }
 
   Type type() const { return type_; }
+  void set_type(Type t) { type_ = t; }
+
   bool expanded() const { return expanded_; }
 
   content::WebContents* contents() { return contents_; }
 
-  // Valid when type() == kSingle or kHubAndSpoke.
-  const base::string16& GetTitle() const;
+  base::string16 GetTitle() const;
 
   // Returns true if this tab data itself is counted as a enumerable item when
   // going through the view.

chrome/browser/ui/tabs/tab_strip_model_experimental.cc

@@ -379,12 +379,30 @@ void TabStripModelExperimental::InsertWebContentsAt(
   delegate_->WillAddWebContents(contents);
 
   bool active = (add_types & ADD_ACTIVE) != 0;
+  TabDataExperimental* data = nullptr;
+
+  if ((add_types & ADD_INHERIT_GROUP) && active_index() >= 0 &&
+      active_index() < count()) {
+    // Add as a child following opener.
+    TabDataExperimental* parent = GetDataForViewIndex(active_index());
+    if (parent->type() == TabDataExperimental::Type::kSingle) {
+      // Promote parent to hub-and-spoke.
+      parent->set_type(TabDataExperimental::Type::kHubAndSpoke);
+      for (auto& observer : exp_observers_)
+        observer.TabChanged(parent);
+    }
+
+    parent->children_.push_back(std::make_unique<TabDataExperimental>(
+        parent, TabDataExperimental::Type::kSingle, contents, this));
+    data = parent->children_.back().get();
+
+  } else {
+    // Add at toplevel.
+    tabs_.push_back(std::make_unique<TabDataExperimental>(
+        nullptr, TabDataExperimental::Type::kSingle, contents, this));
+    data = tabs_.back().get();
+  }
 
-  // Always insert tabs at the end for now (so parent is always null).
-  TabDataExperimental* parent = nullptr;
-  tabs_.emplace_back(std::make_unique<TabDataExperimental>(
-      parent, TabDataExperimental::Type::kSingle, contents, this));
-  const TabDataExperimental* data = tabs_.back().get();
   UpdateViewCount();
   index = tab_view_count_ - 1;
 
@@ -407,7 +425,6 @@ bool TabStripModelExperimental::CloseWebContentsAt(int view_index,
                                                    uint32_t close_types) {
   ViewIterator found = FindViewIndex(view_index);
   DCHECK(found != end());
-  DCHECK(found->type() == TabDataExperimental::Type::kSingle);
   content::WebContents* closing = found->contents_;
   if (closing)
     InternalCloseTabs(base::span<content::WebContents*>(&closing, 1));
@@ -527,8 +544,21 @@ TabDataExperimental* TabStripModelExperimental::GetDataForViewIndex(
   return &*found;
 }
 
+int TabStripModelExperimental::GetViewIndexForData(
+    const TabDataExperimental* data) const {
+  int view_index = 0;
+  for (const auto& cur : *this) {
+    if (&cur == data)
+      return view_index;
+    ++view_index;
+  }
+  return kNoTab;
+}
+
 void TabStripModelExperimental::ActivateTabAt(int index, bool user_gesture) {
-  DCHECK(ContainsIndex(index));
+  if (!ContainsIndex(index))
+    return;
+
   ui::ListSelectionModel new_model = selection_model_;
   new_model.SetSelectedIndex(index);
   SetSelection(std::move(new_model),
@@ -649,7 +679,6 @@ bool TabStripModelExperimental::IsTabPinned(int index) const {
 }
 
 bool TabStripModelExperimental::IsTabBlocked(int index) const {
-  NOTIMPLEMENTED();
   return false;
 }
 
@@ -689,6 +718,10 @@ void TabStripModelExperimental::AddWebContents(content::WebContents* contents,
                                                int index,
                                                ui::PageTransition transition,
                                                int add_types) {
+  // Force group inheritance for link click transitions.
+  if (ui::PageTransitionTypeIncludingQualifiersIs(transition,
+                                                  ui::PAGE_TRANSITION_LINK))
+    add_types |= ADD_INHERIT_GROUP;
   InsertWebContentsAt(index, contents, add_types);
 }
 
@@ -769,6 +802,7 @@ void TabStripModelExperimental::DetachWebContents(
     NOTREACHED();  // WebContents not found in this model.
     return;
   }
+  TabDataExperimental* data = &*found;
 
   bool was_selected;
   if (view_index == kNoTab)
@@ -777,12 +811,31 @@ void TabStripModelExperimental::DetachWebContents(
     was_selected = IsTabSelected(view_index);
   int next_selected_index = view_index;
 
-  if (found->parent_) {
-    // Erase in parent.
-    found->parent_->children_.erase(found->parent_->children_.begin() +
-                                    found.inner_index_);
+  if (data->parent_) {
+    TabDataExperimental* parent = data->parent_;
+    // Erase out of the parent.
+    parent->children_.erase(parent->children_.begin() + found.inner_index_);
+
+    if (parent->children_.empty()) {
+      if (parent->type() == TabDataExperimental::Type::kHubAndSpoke) {
+        // Erasing the last child of a hub and spoke one converts it back to
+        // a single.
+        parent->set_type(TabDataExperimental::Type::kSingle);
+        for (auto& observer : exp_observers_)
+          observer.TabChanged(parent);
+      } else {
+        DCHECK(parent->type() == TabDataExperimental::Type::kGroup);
+        // TODO(brettw) remove group. Notifications might be tricky.
+      }
+    }
+  } else if (data->type() == TabDataExperimental::Type::kHubAndSpoke) {
+    // Removing the "hub" from a hub and spoke converts to a group.
 
-    // TODO(brettw) remove the parent if it's empty!
+    data->set_type(TabDataExperimental::Type::kGroup);
+    data->contents_ =
+        nullptr;  // TODO(brettw) does this delete things properly?
+    for (auto& observer : exp_observers_)
+      observer.TabChanged(data);
   } else {
     // Just remove from tabs.
     tabs_.erase(tabs_.begin() + found.toplevel_index_);

chrome/browser/ui/tabs/tab_strip_model_experimental.h

@@ -196,6 +196,9 @@ class TabStripModelExperimental : public TabStripModel {
   const TabDataExperimental* GetDataForViewIndex(int view_index) const;
   TabDataExperimental* GetDataForViewIndex(int view_index);
 
+  // Returns kNoTab if there isn't a view index for this data.
+  int GetViewIndexForData(const TabDataExperimental* data) const;
+
   void AddExperimentalObserver(TabStripModelExperimentalObserver* observer);
   void RemoveExperimentalObserver(TabStripModelExperimentalObserver* observer);
 

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

@@ -264,8 +264,7 @@ class Tab::TabCloseButton : public views::ImageButton,
   explicit TabCloseButton(Tab* tab)
       : views::ImageButton(tab),
         tab_(tab) {
-    SetEventTargeter(
-        std::unique_ptr<views::ViewTargeter>(new views::ViewTargeter(this)));
+    SetEventTargeter(std::make_unique<views::ViewTargeter>(this));
   }
 
   ~TabCloseButton() override {}
@@ -461,8 +460,7 @@ Tab::Tab(TabController* controller, gfx::AnimationContainer* container)
   title_->SetText(CoreTabHelper::GetDefaultTitle());
   AddChildView(title_);
 
-  SetEventTargeter(
-      std::unique_ptr<views::ViewTargeter>(new views::ViewTargeter(this)));
+  SetEventTargeter(std::make_unique<views::ViewTargeter>(this));
 
   throbber_ = new ThrobberView(this);
   throbber_->SetVisible(false);

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

@@ -6,11 +6,13 @@
 
 #include "chrome/browser/ui/layout_constants.h"
 #include "chrome/browser/ui/tab_contents/core_tab_helper.h"
-#include "chrome/browser/ui/tabs/tab_data_experimental.h"
+#include "chrome/browser/ui/tabs/tab_strip_model_experimental.h"
 #include "chrome/browser/ui/view_ids.h"
 #include "components/grit/components_scaled_resources.h"
 #include "ui/views/border.h"
 #include "ui/views/controls/label.h"
+#include "ui/views/masked_targeter_delegate.h"
+#include "ui/views/widget/widget.h"
 
 namespace {
 
@@ -27,9 +29,12 @@ float GetTabEndcapWidth() {
 
 }  // namespace
 
-TabExperimental::TabExperimental(const TabDataExperimental* data)
+TabExperimental::TabExperimental(TabStripModelExperimental* model,
+                                 const TabDataExperimental* data)
     : views::View(),
+      model_(model),
       data_(data),
+      type_(data->type()),
       title_(new views::Label),
       hover_controller_(this),
       paint_(this) {
@@ -40,6 +45,8 @@ TabExperimental::TabExperimental(const TabDataExperimental* data)
   title_->SetText(CoreTabHelper::GetDefaultTitle());
   AddChildView(title_);
 
+  SetEventTargeter(std::make_unique<views::ViewTargeter>(this));
+
   // So we get don't get enter/exit on children and don't prematurely stop the
   // hover.
   set_notify_enter_exit_on_child(true);
@@ -73,23 +80,84 @@ void TabExperimental::SetSelected(bool selected) {
 }
 
 void TabExperimental::DataUpdated() {
+  type_ = data_->type();
   title_->SetText(data_->GetTitle());
 }
 
+void TabExperimental::SetGroupLayoutParams(int first_child_begin_x) {
+  first_child_begin_x_ = first_child_begin_x;
+}
+
 int TabExperimental::GetOverlap() {
   // We want to overlap the endcap portions entirely.
   return gfx::ToCeiledInt(GetTabEndcapWidth());
 }
 
+bool TabExperimental::GetHitTestMask(gfx::Path* mask) const {
+  // When the window is maximized we don't want to shave off the edges or top
+  // shadow of the tab, such that the user can click anywhere along the top
+  // edge of the screen to select a tab. Ditto for immersive fullscreen.
+  const views::Widget* widget = GetWidget();
+  *mask = paint_.GetBorderPath(
+      GetWidget()->GetCompositor()->device_scale_factor(), true,
+      widget && (widget->IsMaximized() || widget->IsFullscreen()),
+      GetTabEndcapWidth(), size());
+  return true;
+}
+
 void TabExperimental::OnPaint(gfx::Canvas* canvas) {
-  paint_.PaintTabBackground(canvas, active_, 0, 0, nullptr);
+  if (type_ == TabDataExperimental::Type::kSingle)
+    paint_.PaintTabBackground(canvas, active_, 0, 0, nullptr);
+  else
+    paint_.PaintGroupBackground(canvas, active_);
 }
 
 void TabExperimental::Layout() {
+  // Space between the favicon and title.
   constexpr int kTitleSpacing = 6;
   const gfx::Rect bounds = GetContentsBounds();
 
-  title_->SetBoundsRect(gfx::Rect(bounds.x() + kTitleSpacing, bounds.y(),
-                                  bounds.width() - (kTitleSpacing * 2),
-                                  bounds.height()));
+  int title_left = bounds.x() + kTitleSpacing;
+  int title_right;
+  if (first_child_begin_x_ >= 0)
+    title_right = first_child_begin_x_;
+  else
+    title_right = bounds.width() - kTitleSpacing;
+
+  title_->SetBoundsRect(gfx::Rect(title_left, bounds.y(),
+                                  title_right - title_left, bounds.height()));
+}
+
+bool TabExperimental::OnMousePressed(const ui::MouseEvent& event) {
+  // TODO(brettw) the non-experimental one has some stuff about touch and
+  // multi-selection here.
+  if (event.IsOnlyLeftMouseButton())
+    model_->ActivateTabAt(model_->GetViewIndexForData(data_), true);
+  return true;
+}
+
+void TabExperimental::OnMouseReleased(const ui::MouseEvent& event) {
+  // Close tab on middle click, but only if the button is released over the tab
+  // (normal windows behavior is to discard presses of a UI element where the
+  // releases happen off the element).
+  if (event.IsMiddleMouseButton()) {
+    if (HitTestPoint(event.location())) {
+      // TODO(brettw) old one did PrepareForCloseAt which does some animation
+      // stuff.
+      model_->CloseWebContentsAt(
+          model_->GetViewIndexForData(data_),
+          TabStripModel::CLOSE_USER_GESTURE |
+              TabStripModel::CLOSE_CREATE_HISTORICAL_TAB);
+    } else if (closing_) {
+      // We're animating closed and a middle mouse button was pushed on us but
+      // we don't contain the mouse anymore. We assume the user is clicking
+      // quicker than the animation and we should close the tab that falls under
+      // the mouse.
+      /* TODO(brettw) fast closing.
+      Tab* closest_tab = controller_->GetTabAt(this, event.location());
+      if (closest_tab)
+        controller_->CloseTab(closest_tab, CLOSE_TAB_FROM_MOUSE);
+      */
+    }
+  }
 }

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

@@ -6,24 +6,33 @@
 #define CHROME_BROWSER_UI_VIEWS_TABS_TAB_EXPERIMENTAL_H_
 
 #include "base/macros.h"
+#include "chrome/browser/ui/tabs/tab_data_experimental.h"
 #include "chrome/browser/ui/views/tabs/tab_experimental_paint.h"
 #include "ui/views/controls/glow_hover_controller.h"
+#include "ui/views/masked_targeter_delegate.h"
 #include "ui/views/view.h"
 
 class TabDataExperimental;
+class TabStripModelExperimental;
 
 namespace views {
 class Label;
 }
 
-class TabExperimental : public views::View {
+class TabExperimental : public views::MaskedTargeterDelegate,
+                        public views::View {
  public:
-  explicit TabExperimental(const TabDataExperimental* data);
+  explicit TabExperimental(TabStripModelExperimental* model,
+                           const TabDataExperimental* data);
   ~TabExperimental() override;
 
   // Will be null when closing.
   const TabDataExperimental* data() const { return data_; }
 
+  // Cached stuff from data_ that can be used regardless of whether the data
+  // pointer is null or not.
+  TabDataExperimental::Type type() const { return type_; }
+
   // The view order is stored on a tab for the use of TabStrip layout and
   // computatations. It is not used directly by this class. It represents this
   // tab's position in the current layout.
@@ -52,17 +61,30 @@ class TabExperimental : public views::View {
   // to redraw everything.
   void DataUpdated();
 
+  // Called for group types when layout is done to set the bounds of the
+  // first tab. This is used to determine some painting parameters.
+  void SetGroupLayoutParams(int first_child_begin_x);
+
   // Returns the overlap between adjacent tabs.
   static int GetOverlap();
 
  private:
+  // views::MaskedTargeterDelegate:
+  bool GetHitTestMask(gfx::Path* mask) const override;
+
   // views::View:
   void OnPaint(gfx::Canvas* canvas) override;
   void Layout() override;
+  bool OnMousePressed(const ui::MouseEvent& event) override;
+  void OnMouseReleased(const ui::MouseEvent& event) override;
+
+  TabStripModelExperimental* model_;
 
   // Will be null when closing.
   const TabDataExperimental* data_;
 
+  TabDataExperimental::Type type_;
+
   size_t view_order_ = static_cast<size_t>(-1);
   gfx::Rect ideal_bounds_;
 
@@ -72,6 +94,9 @@ class TabExperimental : public views::View {
 
   views::Label* title_;  // Non-owning (owned by View hierarchy).
 
+  // Location of the first child tab. Negative indicates unused.
+  int first_child_begin_x_ = -1;
+
   views::GlowHoverController hover_controller_;
 
   TabExperimentalPaint paint_;

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

@@ -7,6 +7,7 @@
 #include "cc/paint/paint_recorder.h"
 #include "chrome/browser/themes/theme_properties.h"
 #include "chrome/browser/ui/layout_constants.h"
+#include "chrome/browser/ui/views/frame/browser_view.h"
 #include "chrome/grit/generated_resources.h"
 #include "chrome/grit/theme_resources.h"
 #include "third_party/skia/include/pathops/SkPathOps.h"
@@ -52,7 +53,9 @@ float GetInverseDiagonalSlope() {
 
 // Returns a path corresponding to the tab's content region inside the outer
 // stroke.
-gfx::Path GetFillPath(float scale, const gfx::Size& size, float endcap_width) {
+gfx::Path GetTabFillPath(float scale,
+                         const gfx::Size& size,
+                         float endcap_width) {
   const float right = size.width() * scale;
   // The bottom of the tab needs to be pixel-aligned or else when we call
   // ClipPath with anti-aliasing enabled it can cause artifacts.
@@ -90,11 +93,11 @@ gfx::Path GetFillPath(float scale, const gfx::Size& size, float endcap_width) {
 // |extend_to_top| is true, the path is extended vertically to the top of the
 // tab bounds.  The caller uses this for Fitts' Law purposes in
 // maximized/fullscreen mode.
-gfx::Path GetBorderPath(float scale,
-                        bool unscale_at_end,
-                        bool extend_to_top,
-                        float endcap_width,
-                        const gfx::Size& size) {
+gfx::Path GetTabBorderPath(float scale,
+                           bool unscale_at_end,
+                           bool extend_to_top,
+                           float endcap_width,
+                           const gfx::Size& size) {
   const float top = scale - 1;
   const float right = size.width() * scale;
   const float bottom = size.height() * scale;
@@ -132,6 +135,62 @@ gfx::Path GetBorderPath(float scale,
   return path;
 }
 
+// Returns a path corresponding to the tab's content region inside the outer
+// stroke.
+gfx::Path GetHubAndSpokeFillPath(float scale,
+                                 const gfx::Size& size,
+                                 float endcap_width) {
+  const float right = size.width() * scale;
+  // The bottom of the tab needs to be pixel-aligned or else when we call
+  // ClipPath with anti-aliasing enabled it can cause artifacts.
+  const float bottom = std::ceil(size.height() * scale);
+
+  gfx::Path fill;
+  fill.moveTo(right - 1, bottom);
+  fill.rCubicTo(-0.75 * scale, 0, -1.625 * scale, -0.5 * scale, -2 * scale,
+                -1.5 * scale);
+  fill.lineTo(right - 1 - (endcap_width - 2) * scale, 2.5 * scale);
+  // Prevent overdraw in the center near minimum width (only happens if
+  // scale < 2).  We could instead avoid this by increasing the tab inset
+  // values, but that would shift all the content inward as well, unless we
+  // then overlapped the content on the endcaps, by which point we'd have a
+  // huge mess.
+  const float scaled_endcap_width = 1 + endcap_width * scale;
+  const float overlap = scaled_endcap_width * 2 - right;
+  const float offset = (overlap > 0) ? (overlap / 2) : 0;
+  fill.rCubicTo(-0.375 * scale, -1 * scale, -1.25 * scale + offset,
+                -1.5 * scale, -2 * scale + offset, -1.5 * scale);
+  if (overlap < 0)
+    fill.lineTo(scaled_endcap_width, scale);
+  fill.rCubicTo(-0.75 * scale, 0, -1.625 * scale - offset, 0.5 * scale,
+                -2 * scale - offset, 1.5 * scale);
+  fill.lineTo(1 + 2 * scale, bottom - 1.5 * scale);
+  fill.rCubicTo(-0.375 * scale, scale, -1.25 * scale, 1.5 * scale, -2 * scale,
+                1.5 * scale);
+  fill.close();
+  return fill;
+}
+
+gfx::Path GetHubAndSpokeBorderPath(float scale,
+                                   float endcap_width,
+                                   const gfx::Size& size) {
+  const float top = scale - 1;
+  const float right = size.width() * scale;
+  const float bottom = size.height() * scale;
+
+  gfx::Path path;
+  path.moveTo(0, bottom);
+  path.rLineTo(0, -1);
+  path.rCubicTo(0.75 * scale, 0, 1.625 * scale, -0.5 * scale, 2 * scale,
+                -1.5 * scale);
+  path.lineTo((endcap_width - 2) * scale, top + 1.5 * scale);
+  path.rCubicTo(0.375 * scale, -scale, 1.25 * scale, -1.5 * scale, 2 * scale,
+                -1.5 * scale);
+  path.lineTo(right, top);
+
+  return path;
+}
+
 }  // namespace
 
 // TabExperimentalPaint::BackgroundCache ---------------------------------------
@@ -166,6 +225,48 @@ void TabExperimentalPaint::BackgroundCache::SetCacheKey(float scale,
 TabExperimentalPaint::TabExperimentalPaint(views::View* view) : view_(view) {}
 TabExperimentalPaint::~TabExperimentalPaint() {}
 
+gfx::Path TabExperimentalPaint::GetBorderPath(float scale,
+                                              bool unscale_at_end,
+                                              bool extend_to_top,
+                                              float endcap_width,
+                                              const gfx::Size& size) const {
+  const float top = scale - 1;
+  const float right = size.width() * scale;
+  const float bottom = size.height() * scale;
+
+  gfx::Path path;
+  path.moveTo(0, bottom);
+  path.rLineTo(0, -1);
+  path.rCubicTo(0.75 * scale, 0, 1.625 * scale, -0.5 * scale, 2 * scale,
+                -1.5 * scale);
+  path.lineTo((endcap_width - 2) * scale, top + 1.5 * scale);
+  if (extend_to_top) {
+    // Create the vertical extension by extending the side diagonals until
+    // they reach the top of the bounds.
+    const float dy = 2.5 * scale - 1;
+    const float dx = Tab::GetInverseDiagonalSlope() * dy;
+    path.rLineTo(dx, -dy);
+    path.lineTo(right - (endcap_width - 2) * scale - dx, 0);
+    path.rLineTo(dx, dy);
+  } else {
+    path.rCubicTo(0.375 * scale, -scale, 1.25 * scale, -1.5 * scale, 2 * scale,
+                  -1.5 * scale);
+    path.lineTo(right - endcap_width * scale, top);
+    path.rCubicTo(0.75 * scale, 0, 1.625 * scale, 0.5 * scale, 2 * scale,
+                  1.5 * scale);
+  }
+  path.lineTo(right - 2 * scale, bottom - 1 - 1.5 * scale);
+  path.rCubicTo(0.375 * scale, scale, 1.25 * scale, 1.5 * scale, 2 * scale,
+                1.5 * scale);
+  path.rLineTo(0, 1);
+  path.close();
+
+  if (unscale_at_end && (scale != 1))
+    path.transform(SkMatrix::MakeScale(1.f / scale));
+
+  return path;
+}
+
 void TabExperimentalPaint::PaintTabBackground(gfx::Canvas* canvas,
                                               bool active,
                                               int fill_id,
@@ -179,8 +280,12 @@ void TabExperimentalPaint::PaintTabBackground(gfx::Canvas* canvas,
   const SkColor active_color = tp->GetColor(ThemeProperties::COLOR_TOOLBAR);
   const SkColor inactive_color =
       tp->GetColor(ThemeProperties::COLOR_BACKGROUND_TAB);
-  const SkColor stroke_color =
-      0xFFFF;  // controller_->GetToolbarTopSeparatorColor();
+
+  // TODO(brettw) in the old code this was stroke_color =
+  // controller_->GetToolbarTopSeparatorColor() which handles theming,
+  // activation, and incognito.
+  const SkColor stroke_color = ThemeProperties::GetDefaultColor(
+      ThemeProperties::COLOR_TOOLBAR_TOP_SEPARATOR, false);
   const bool paint_hover_effect =
       false;  //! active && hover_controller_.ShouldDraw();
 
@@ -193,9 +298,9 @@ void TabExperimentalPaint::PaintTabBackground(gfx::Canvas* canvas,
   // cache based on the hover states.
   if (fill_id || paint_hover_effect) {
     gfx::Path fill_path =
-        GetFillPath(canvas->image_scale(), view_->size(), endcap_width);
-    gfx::Path stroke_path = GetBorderPath(canvas->image_scale(), false, false,
-                                          endcap_width, view_->size());
+        GetTabFillPath(canvas->image_scale(), view_->size(), endcap_width);
+    gfx::Path stroke_path = GetTabBorderPath(
+        canvas->image_scale(), false, false, endcap_width, view_->size());
     PaintTabBackgroundFill(canvas, fill_path, active, paint_hover_effect,
                            active_color, inactive_color, fill_id, y_offset);
     gfx::ScopedCanvas scoped_canvas(clip ? canvas : nullptr);
@@ -211,9 +316,9 @@ void TabExperimentalPaint::PaintTabBackground(gfx::Canvas* canvas,
   if (!cache.CacheKeyMatches(canvas->image_scale(), view_->size(), active_color,
                              inactive_color, stroke_color)) {
     gfx::Path fill_path =
-        GetFillPath(canvas->image_scale(), view_->size(), endcap_width);
-    gfx::Path stroke_path = GetBorderPath(canvas->image_scale(), false, false,
-                                          endcap_width, view_->size());
+        GetTabFillPath(canvas->image_scale(), view_->size(), endcap_width);
+    gfx::Path stroke_path = GetTabBorderPath(
+        canvas->image_scale(), false, false, endcap_width, view_->size());
     cc::PaintRecorder recorder;
 
     {
@@ -243,6 +348,64 @@ void TabExperimentalPaint::PaintTabBackground(gfx::Canvas* canvas,
   if (clip)
     canvas->sk_canvas()->clipPath(*clip, SkClipOp::kDifference, true);
   canvas->sk_canvas()->drawPicture(cache.stroke_record);
+
+  // Bottom border.
+  if (!active) {
+    BrowserView::Paint1pxHorizontalLine(
+        canvas,
+        ThemeProperties::GetDefaultColor(
+            ThemeProperties::COLOR_TOOLBAR_TOP_SEPARATOR, false),
+        view_->GetLocalBounds(), true);
+  }
+}
+
+void TabExperimentalPaint::PaintGroupBackground(gfx::Canvas* canvas,
+                                                bool active) {
+  // const ui::ThemeProvider* tp = view_->GetThemeProvider();
+  // const SkColor active_color = tp->GetColor(ThemeProperties::COLOR_TOOLBAR);
+  // const SkColor inactive_color =
+  //    tp->GetColor(ThemeProperties::COLOR_BACKGROUND_TAB);
+
+  // TODO(brettw) in the old code this was stroke_color =
+  // controller_->GetToolbarTopSeparatorColor() which handles theming,
+  // activation, and incognito.
+  SkColor stroke_color = ThemeProperties::GetDefaultColor(
+      ThemeProperties::COLOR_TOOLBAR_TOP_SEPARATOR, false);
+
+  const ui::ThemeProvider* tp = view_->GetThemeProvider();
+  const SkColor active_color = tp->GetColor(ThemeProperties::COLOR_TOOLBAR);
+  const SkColor inactive_color =
+      tp->GetColor(ThemeProperties::COLOR_BACKGROUND_TAB);
+  float endcap_width = GetTabEndcapWidth();
+  const bool paint_hover_effect =
+      false;  //! active && hover_controller_.ShouldDraw();
+
+  gfx::Path fill_path = GetHubAndSpokeFillPath(canvas->image_scale(),
+                                               view_->size(), endcap_width);
+  PaintTabBackgroundFill(canvas, fill_path, active, paint_hover_effect,
+                         active_color, inactive_color, 0, 0);
+
+  {
+    gfx::ScopedCanvas scoped_canvas(canvas);
+    float scale = canvas->UndoDeviceScaleFactor();
+    gfx::Path path =
+        GetHubAndSpokeBorderPath(scale, endcap_width, view_->size());
+    cc::PaintFlags flags;
+    flags.setAntiAlias(true);
+    flags.setColor(stroke_color);
+    flags.setStyle(cc::PaintFlags::kStroke_Style);
+
+    canvas->DrawPath(path, flags);
+  }
+
+  // Bottom border.
+  if (!active) {
+    BrowserView::Paint1pxHorizontalLine(
+        canvas,
+        ThemeProperties::GetDefaultColor(
+            ThemeProperties::COLOR_TOOLBAR_TOP_SEPARATOR, false),
+        view_->GetLocalBounds(), true);
+  }
 }
 
 void TabExperimentalPaint::PaintTabBackgroundFill(gfx::Canvas* canvas,
@@ -254,7 +417,7 @@ void TabExperimentalPaint::PaintTabBackgroundFill(gfx::Canvas* canvas,
                                                   int fill_id,
                                                   int y_offset) {
   gfx::ScopedCanvas scoped_canvas(canvas);
-  const float scale = canvas->UndoDeviceScaleFactor();
+  float scale = canvas->UndoDeviceScaleFactor();
 
   canvas->ClipPath(fill_path, true);
   if (fill_id) {

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

@@ -26,12 +26,26 @@ class TabExperimentalPaint {
   explicit TabExperimentalPaint(views::View* view);
   ~TabExperimentalPaint();
 
+  // Returns a path corresponding to the tab's outer border for a given tab
+  // |size|, |scale|, and |endcap_width|. If |unscale_at_end| is true, this
+  // path will be normalized to a 1x scale by scaling by 1/scale before
+  // returning. If |extend_to_top| is true, the path is extended vertically to
+  // the top of the tab bounds.  The caller uses this for Fitts' Law purposes
+  // in maximized/fullscreen mode.
+  gfx::Path GetBorderPath(float scale,
+                          bool unscale_at_end,
+                          bool extend_to_top,
+                          float endcap_width,
+                          const gfx::Size& size) const;
+
   void PaintTabBackground(gfx::Canvas* canvas,
                           bool active,
                           int fill_id,
                           int y_offset,
                           gfx::Path* clip);
 
+  void PaintGroupBackground(gfx::Canvas* canvas, bool active);
+
  private:
   class BackgroundCache {
    public:

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

@@ -97,6 +97,9 @@ const int kPinnedToNonPinnedOffset = 2;
 const int kPinnedToNonPinnedOffset = 3;
 #endif
 
+// Additional size above the tabs used for group bounds.
+const int kTopPaddingForGroups = 2;
+
 // Returns the width needed for the new tab button (and padding).
 int GetNewTabButtonWidth() {
   return GetLayoutSize(NEW_TAB_BUTTON).width() -
@@ -168,6 +171,23 @@ void ResetDraggingStateDelegate::AnimationCanceled(
   AnimationEnded(animation);
 }
 
+// Returns how much spacing should go after the tab with the given index. The
+// return value will be negative for overlapped tabs.
+int TabSpacingAfter(
+    const std::vector<std::unique_ptr<TabDataExperimental>>& tabs,
+    size_t index) {
+  if (index == tabs.size() - 1)
+    return 0;  // No space after the last tab.
+
+  // All non-single-tabs have no overlap.
+  if (tabs[index]->type() != TabDataExperimental::Type::kSingle ||
+      tabs[index + 1]->type() != TabDataExperimental::Type::kSingle)
+    return 0;
+
+  // Everything else gets the standard overlap.
+  return -TabExperimental::GetOverlap();
+}
+
 }  // namespace
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -502,7 +522,7 @@ void TabStripExperimental::TabInserted(const TabDataExperimental* data,
                                        bool is_active) {
   InvalidateViewOrder();
 
-  TabExperimental* tab = new TabExperimental(data);
+  TabExperimental* tab = new TabExperimental(model_, data);
   AddChildView(tab);
   tab->SetVisible(true);
   tabs_.emplace(data, tab);
@@ -546,6 +566,9 @@ void TabStripExperimental::TabClosing(const TabDataExperimental* data) {
   if (!tab)
     return;
 
+  // TODO(brettw) this uses view order to tell if it's the last tab, but this
+  // is not correct. It should look only at the tabs structure and we should
+  // be able to delte view_order() on the TabExperimental.
   if (in_tab_close_ && tab->view_order() != view_order_.size() - 1)
     StartMouseInitiatedRemoveTabAnimation(tab);
   else
@@ -583,6 +606,8 @@ void TabStripExperimental::TabChanged(const TabDataExperimental* data) {
 void TabStripExperimental::TabSelectionChanged(
     const TabDataExperimental* old_data,
     const TabDataExperimental* new_data) {
+  InvalidateViewOrder();
+
   TabExperimental* old_tab = TabForData(old_data);
   if (old_tab) {
     old_tab->SetActive(false);
@@ -623,8 +648,26 @@ void TabStripExperimental::Layout() {
 void TabStripExperimental::PaintChildren(const views::PaintInfo& paint_info) {
   EnsureViewOrderUpToDate();
 
-  // The order stored by the view itself doesn't match the paint order. The
-  // paint order is the reverse of the view_order_ vector (to paint
+  // TODO(brettw) The color should be
+  // controller_->GetToolbarTopSeparatorColor() which handles theming,
+  // activation, and incognito.
+  // TODO(brettw) only paint this where there are no tabs, as the tabs also
+  // paint their own bottom border and this will be overpainting.
+  {
+    ui::PaintRecorder recorder(paint_info.context(),
+                               paint_info.paint_recording_size(),
+                               paint_info.paint_recording_scale_x(),
+                               paint_info.paint_recording_scale_y(), nullptr);
+    gfx::Canvas* canvas = recorder.canvas();
+    BrowserView::Paint1pxHorizontalLine(
+        canvas,
+        ThemeProperties::GetDefaultColor(
+            ThemeProperties::COLOR_TOOLBAR_TOP_SEPARATOR, false),
+        GetLocalBounds(), true);
+  }
+
+  // The order stored by the view::View itself doesn't match the paint order.
+  // The paint order is the reverse of the view_order_ vector (to paint
   // back-to-front).
   for (TabExperimental* tab : base::Reversed(view_order_))
     tab->Paint(paint_info);
@@ -645,10 +688,6 @@ void TabStripExperimental::PaintChildren(const views::PaintInfo& paint_info) {
         SkClipOp::kDifference);
   }
   */
-  /* TODO(brettw) top of toolbar.
-  BrowserView::Paint1pxHorizontalLine(canvas, GetToolbarTopSeparatorColor(),
-                                      GetLocalBounds(), true);
-  */
 }
 
 const char* TabStripExperimental::GetClassName() const {
@@ -684,8 +723,9 @@ gfx::Size TabStripExperimental::CalculatePreferredSize() const {
   needed_tab_width = std::min(std::max(needed_tab_width, min_selected_width),
                               largest_min_tab_width);
 
-  return gfx::Size(needed_tab_width + GetNewTabButtonWidth(),
-                   Tab::GetMinimumInactiveSize().height());
+  return gfx::Size(
+      needed_tab_width + GetNewTabButtonWidth(),
+      Tab::GetMinimumInactiveSize().height() + kTopPaddingForGroups);
 }
 
 void TabStripExperimental::OnDragEntered(const DropTargetEvent& event) {
@@ -824,14 +864,8 @@ void TabStripExperimental::StartInsertTabAnimation(
   GenerateIdealBounds();
 
   // Set the current bounds to be the correct place but 0 width.
-  DCHECK(view_order_[tab->view_order()] == tab);
-  if (tab->view_order() == 0) {
-    tab->SetBounds(0, tab->ideal_bounds().y(), 0, tab->ideal_bounds().height());
-  } else {
-    TabExperimental* prev_tab = view_order_[tab->view_order() - 1];
-    tab->SetBounds(prev_tab->bounds().right() - TabExperimental::GetOverlap(),
-                   tab->ideal_bounds().y(), 0, tab->ideal_bounds().height());
-  }
+  tab->SetBounds(tab->ideal_bounds().x(), tab->ideal_bounds().y(), 0,
+                 tab->ideal_bounds().height());
 
   // Animate in to the full width.
   AnimateToIdealBounds();
@@ -845,10 +879,10 @@ void TabStripExperimental::StartMoveTabAnimation() {
 
 void TabStripExperimental::StartRemoveTabAnimation(TabExperimental* tab) {
   PrepareForAnimation();
-  tab->SetClosing();
-
   RemoveTabFromViewModel(tab);
   ScheduleRemoveTabAnimation(tab);
+
+  tab->SetClosing();
 }
 
 void TabStripExperimental::ScheduleRemoveTabAnimation(TabExperimental* tab) {
@@ -876,7 +910,11 @@ void TabStripExperimental::ScheduleRemoveTabAnimation(TabExperimental* tab) {
 
 void TabStripExperimental::AnimateToIdealBounds() {
   EnsureViewOrderUpToDate();
-  for (TabExperimental* tab : view_order_) {
+  // Don't iterate over view_order since that will contain tabs that are
+  // in the process of being deleted. Asking them to animate again will
+  // cancel the previous animation which will cause them to be deleted.
+  for (const auto& pair : tabs_) {
+    TabExperimental* tab = pair.second;
     bounds_animator_.AnimateViewTo(tab, tab->ideal_bounds());
     bounds_animator_.SetAnimationDelegate(
         tab, std::make_unique<TabAnimationDelegate>(this, tab));
@@ -1107,10 +1145,10 @@ void TabStripExperimental::SetTabBoundsForDrag(
 
 void TabStripExperimental::AddMessageLoopObserver() {
   if (!mouse_watcher_.get()) {
-    mouse_watcher_.reset(new views::MouseWatcher(
+    mouse_watcher_ = std::make_unique<views::MouseWatcher>(
         new views::MouseWatcherViewHost(
             this, gfx::Insets(0, 0, kTabStripAnimationVSlop, 0)),
-        this));
+        this);
   }
   mouse_watcher_->Start();
 }
@@ -1286,16 +1324,70 @@ void TabStripExperimental::GenerateIdealBounds() {
                             ? GetTabAreaWidth()
                             : available_width_for_tabs_;
 
+  auto& tabs = model_->top_level_tabs();
+  int overlap = TabExperimental::GetOverlap();
+
+  // Precompute the spacing to know how much space can be distributed to
+  // the tabs. Thia is usually negative since the tabs overlap slightly.
+  int spacing_accumulator = 0;
+  for (size_t i = 0; i < tabs.size() - 1; i++) {
+    switch (tabs[i]->type()) {
+      case TabDataExperimental::Type::kSingle:
+        spacing_accumulator += TabSpacingAfter(tabs, i);
+        break;
+      case TabDataExperimental::Type::kHubAndSpoke:
+        // Hub and spoke gets overlap between the hub and first child.
+        spacing_accumulator -= overlap;
+      // Fall through to group for counting the children.
+      case TabDataExperimental::Type::kGroup:
+        for (size_t inner_i = 0; inner_i < tabs[i]->children().size();
+             inner_i++)
+          spacing_accumulator += TabSpacingAfter(tabs[i]->children(), inner_i);
+        break;
+    }
+  }
+
   gfx::Size standard_size = Tab::GetStandardSize();
-  int tab_width = available_width / view_order_.size();
+  int tab_width = (available_width - spacing_accumulator) / view_order_.size();
   if (tab_width > standard_size.width())
     tab_width = standard_size.width();
 
   int x = 0;
-  for (size_t i = 0; i < view_order_.size(); i++) {
-    view_order_[i]->set_ideal_bounds(
-        gfx::Rect(x, 0, tab_width, standard_size.height()));
-    x += tab_width;
+  for (size_t i = 0; i < tabs.size(); i++) {
+    const TabDataExperimental* top_data = tabs[i].get();
+    const auto top_found = tabs_.find(top_data);
+    if (top_found == tabs_.end())
+      continue;  // Can happen during tab close.
+
+    if (top_data->type() == TabDataExperimental::Type::kSingle) {
+      top_found->second->set_ideal_bounds(gfx::Rect(
+          x, kTopPaddingForGroups, tab_width, standard_size.height()));
+      x += tab_width + TabSpacingAfter(tabs, i);
+    } else {
+      int top_x = x;
+      x += tab_width - overlap;  // Will increment for each child.
+
+      top_found->second->SetGroupLayoutParams(x - top_x);
+
+      // Layout children inside.
+      for (size_t inner_i = 0; inner_i < top_data->children().size();
+           inner_i++) {
+        const TabDataExperimental* inner_data =
+            top_data->children()[inner_i].get();
+        const auto inner_found = tabs_.find(inner_data);
+        if (inner_found == tabs_.end()) {
+          NOTREACHED();
+        } else {
+          inner_found->second->set_ideal_bounds(gfx::Rect(
+              x, kTopPaddingForGroups, tab_width, standard_size.height()));
+        }
+        x += tab_width + TabSpacingAfter(top_data->children(), inner_i);
+      }
+
+      // Layout group.
+      top_found->second->set_ideal_bounds(gfx::Rect(
+          top_x, 0, x - top_x, standard_size.height() + kTopPaddingForGroups));
+    }
   }
 
   /* TODO(brettw) need to notify of max X.
@@ -1418,30 +1510,12 @@ int TabStripExperimental::GetStartXForNormalTabs() const {
 
 TabExperimental* TabStripExperimental::FindTabHitByPoint(
     const gfx::Point& point) {
-  /* TODO(brettw) hit testing.
   // The display order doesn't necessarily match the child order, so we iterate
   // in display order.
-  for (int i = 0; i < tab_count(); ++i) {
-    // If we don't first exclude points outside the current tab, the code below
-    // will return the wrong tab if the next tab is selected, the following tab
-    // is active, and |point| is in the overlap region between the two.
-    TabExperimental* tab = tab_at(i);
-    if (!IsPointInTab(tab, point))
-      continue;
-
-    // Selected tabs render atop unselected ones, and active tabs render atop
-    // everything.  Check whether the next tab renders atop this one and |point|
-    // is in the overlap region.
-    TabExperimental* next_tab = i < (tab_count() - 1) ? tab_at(i + 1) : nullptr;
-    if (next_tab &&
-        (next_tab->active() || (next_tab->selected() && !tab->selected())) &&
-        IsPointInTab(next_tab, point))
-      return next_tab;
-
-    // This is the topmost tab for this point.
-    return tab;
+  for (TabExperimental* tab : view_order_) {
+    if (IsPointInTab(tab, point))
+      return tab;
   }
-  */
   return nullptr;
 }
 
@@ -1632,29 +1706,9 @@ void TabStripExperimental::EnsureViewOrderUpToDate() const {
     return;
 
   view_order_.reserve(tabs_.size());
-
-  for (const auto& top_data : model_->top_level_tabs()) {
-    // Add children. This assumes a two-level hierarchy. If more nesting is
-    // required, this will need to be recursive.
-    for (const auto& inner_data : top_data->children()) {
-      const auto inner_found = tabs_.find(inner_data.get());
-      if (inner_found == tabs_.end()) {
-        NOTREACHED();
-      } else {
-        inner_found->second->set_view_order(view_order_.size());
-        view_order_.push_back(inner_found->second);
-      }
-    }
-
-    // The group itself goes below the child tabs.
-    const auto top_found = tabs_.find(top_data.get());
-    if (top_found == tabs_.end()) {
-      NOTREACHED();
-    } else {
-      top_found->second->set_view_order(view_order_.size());
-      view_order_.push_back(top_found->second);
-    }
-  }
+  const TabDataExperimental* selected_data =
+      model_->GetDataForViewIndex(model_->active_index());
+  ComputeViewOrder(model_->top_level_tabs(), selected_data, &view_order_);
 
   // Put the closing tabs at the back.
   // TODO(brettw) this isn't right, they should go in the order they were
@@ -1665,3 +1719,43 @@ void TabStripExperimental::EnsureViewOrderUpToDate() const {
     view_order_.push_back(tab);
   }
 }
+
+void TabStripExperimental::ComputeViewOrder(
+    base::span<const std::unique_ptr<TabDataExperimental>> tabs,
+    const TabDataExperimental* selected,
+    std::vector<TabExperimental*>* output) const {
+  // Stores the selected one if we encounter it.
+  const std::unique_ptr<TabDataExperimental>* top = nullptr;
+
+  size_t begin_index = output->size();
+
+  for (const auto& data : tabs) {
+    const auto found_tab = tabs_.find(data.get());
+    if (found_tab == tabs_.end())
+      continue;  // Can happen during tab close.
+    if (data.get() == selected) {
+      // Save the selected one for last.
+      top = &data;
+      continue;
+    }
+
+    // Recursively order children.
+    if (!data->children().empty())
+      ComputeViewOrder(data->children(), selected, output);
+
+    found_tab->second->set_view_order(output->size());
+    output->push_back(found_tab->second);
+  }
+
+  // Add the selected one (and any children) to the front.
+  if (top) {
+    // Recursively call ComputeViewOrder since the selected tab may be a group
+    // that needs its children laid out.
+    std::vector<TabExperimental*> temp_output;
+    ComputeViewOrder(
+        base::span<const std::unique_ptr<TabDataExperimental>>(top, 1), nullptr,
+        &temp_output);
+    output->insert(output->begin() + begin_index, temp_output.begin(),
+                   temp_output.end());
+  }
+}

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

@@ -11,6 +11,7 @@
 #include "base/compiler_specific.h"
 #include "base/containers/flat_map.h"
 #include "base/containers/flat_set.h"
+#include "base/containers/span.h"
 #include "base/gtest_prod_util.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
@@ -423,6 +424,12 @@ class TabStripExperimental : public TabStrip,
   // Ensures that view_order_ corresponds to the latest tabs_ structure.
   void EnsureViewOrderUpToDate() const;
 
+  // Recursive backend for EnsureViewOrderUpToDate.
+  void ComputeViewOrder(
+      base::span<const std::unique_ptr<TabDataExperimental>> tabs,
+      const TabDataExperimental* selected,
+      std::vector<TabExperimental*>* output) const;
+
   // -- Member Variables ------------------------------------------------------
 
   TabStripModelExperimental* model_;
@@ -437,8 +444,9 @@ class TabStripExperimental : public TabStrip,
   // this will cause them to be painted in order of their memory address!
   base::flat_set<TabExperimental*> closing_tabs_;
 
-  // Cached ordered vector for painting the tabs. This will include everything
-  // in the tabs_ map and also the items in closing_tabs_.
+  // Cached ordered vector for painting the tabs. The topmost view is at the
+  // front(). This will include everything in the tabs_ map and also the items
+  // in closing_tabs_.
   //
   // This is computed by EnsureViewOrderUpToDate().
   mutable std::vector<TabExperimental*> view_order_;

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

@@ -1934,10 +1934,10 @@ void TabStripImpl::SetTabBoundsForDrag(
 
 void TabStripImpl::AddMessageLoopObserver() {
   if (!mouse_watcher_.get()) {
-    mouse_watcher_.reset(new views::MouseWatcher(
+    mouse_watcher_ = std::make_unique<views::MouseWatcher>(
         new views::MouseWatcherViewHost(
             this, gfx::Insets(0, 0, kTabStripAnimationVSlop, 0)),
-        this));
+        this);
   }
   mouse_watcher_->Start();
 }