Implement basic TabGroupUnderline

The underline is composed of two parts:
1) A straight underline, painted by TabStrip, extending from the group header to the last time (minus some inset).
2) A stroke above the active tab, painted by TabStyleViews, which (along with the active tab background) is layered over the straight underline.

The underline is still missing the following polish:
- Half-rounded edges.
- Potential redesign of the stroke above the active tab.

Screenshots are in the attached bug.

Bug: 905491
Change-Id: I4a99a67f9637b8a879bba7589feb848edb30deee
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1809546
Commit-Queue: Connie Wan <connily@chromium.org>
Reviewed-by: Taylor Bergquist <tbergquist@chromium.org>
Cr-Commit-Position: refs/heads/master@{#697805}

chrome/browser/ui/BUILD.gn

@@ -3102,6 +3102,8 @@ jumbo_split_static_library("ui") {
       "views/tabs/tab_group_editor_bubble_view.h",
       "views/tabs/tab_group_header.cc",
       "views/tabs/tab_group_header.h",
+      "views/tabs/tab_group_underline.cc",
+      "views/tabs/tab_group_underline.h",
       "views/tabs/tab_hover_card_bubble_view.cc",
       "views/tabs/tab_hover_card_bubble_view.h",
       "views/tabs/tab_icon.cc",

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

@@ -685,7 +685,7 @@ void Tab::SetGroup(base::Optional<TabGroupId> group) {
   if (group_ == group)
     return;
   group_ = group;
-  GroupColorChanged();
+  SchedulePaint();
 }
 
 base::Optional<SkColor> Tab::GetGroupColor() const {
@@ -695,11 +695,6 @@ base::Optional<SkColor> Tab::GetGroupColor() const {
              : base::nullopt;
 }
 
-void Tab::GroupColorChanged() {
-  UpdateForegroundColors();
-  SchedulePaint();
-}
-
 SkColor Tab::GetAlertIndicatorColor(TabAlertState state) const {
   // If theme provider is not yet available, return the default button
   // color.

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

@@ -127,10 +127,6 @@ class Tab : public gfx::AnimationDelegate,
   // Returns the color for the tab's group, if any.
   base::Optional<SkColor> GetGroupColor() const;
 
-  // Should be called when the result of
-  // |TabController::GetVisualDataForGroup()| changes.
-  void GroupColorChanged();
-
   // Returns the color used for the alert indicator icon.
   SkColor GetAlertIndicatorColor(TabAlertState state) const;
 

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

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/ui/views/tabs/tab_group_underline.h"
+
+#include <memory>
+#include <utility>
+
+#include "chrome/browser/ui/tabs/tab_group_visual_data.h"
+#include "chrome/browser/ui/views/tabs/tab.h"
+#include "chrome/browser/ui/views/tabs/tab_group_header.h"
+#include "chrome/browser/ui/views/tabs/tab_strip.h"
+#include "chrome/browser/ui/views/tabs/tab_strip_controller.h"
+#include "third_party/skia/include/core/SkColor.h"
+#include "ui/gfx/canvas.h"
+#include "ui/views/background.h"
+#include "ui/views/view.h"
+
+TabGroupUnderline::TabGroupUnderline(TabStrip* tab_strip, TabGroupId group)
+    : tab_strip_(tab_strip), group_(group) {
+  UpdateVisuals();
+}
+
+void TabGroupUnderline::OnPaint(gfx::Canvas* canvas) {
+  UpdateVisuals();
+  OnPaintBackground(canvas);
+}
+
+void TabGroupUnderline::UpdateVisuals() {
+  SkColor color = GetColor();
+
+  int start_x = GetStart();
+  int end_x = GetEnd();
+
+  int start_y = tab_strip_->bounds().height() - 1;
+  constexpr int kStrokeWidth = 2;
+
+  SetBounds(start_x, start_y - kStrokeWidth, end_x - start_x, kStrokeWidth);
+  SetBackground(views::CreateSolidBackground(color));
+}
+
+SkColor TabGroupUnderline::GetColor() {
+  const TabGroupVisualData* data =
+      tab_strip_->controller()->GetVisualDataForGroup(group_);
+
+  return data->color();
+}
+
+int TabGroupUnderline::GetStart() {
+  const gfx::Rect group_header_bounds =
+      tab_strip_->group_header(group_)->bounds();
+
+  constexpr int kInset = 20;
+  return group_header_bounds.x() + kInset;
+}
+
+int TabGroupUnderline::GetEnd() {
+  const std::vector<int> tabs_in_group =
+      tab_strip_->controller()->ListTabsInGroup(group_);
+  const int last_tab_index = tabs_in_group[tabs_in_group.size() - 1];
+  const gfx::Rect& last_tab_bounds =
+      tab_strip_->tab_at(last_tab_index)->bounds();
+
+  constexpr int kInset = 20;
+  return last_tab_bounds.right() - kInset;
+}

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

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_UI_VIEWS_TABS_TAB_GROUP_UNDERLINE_H_
+#define CHROME_BROWSER_UI_VIEWS_TABS_TAB_GROUP_UNDERLINE_H_
+
+#include "chrome/browser/ui/tabs/tab_group_id.h"
+#include "chrome/browser/ui/views/tabs/tab_strip.h"
+#include "chrome/browser/ui/views/tabs/tab_strip_controller.h"
+#include "ui/views/view.h"
+
+// View for tab group underlines in the tab strip, which are markers of group
+// members. There is one underline for each group, which is included in the tab
+// strip flow and positioned across all tabs in the group.
+class TabGroupUnderline : public views::View {
+ public:
+  TabGroupUnderline(TabStrip* tab_strip, TabGroupId group);
+
+  TabGroupId group() const { return group_; }
+
+  // Updates the bounds and color of the underline for painting.
+  void UpdateVisuals();
+
+  // views::View:
+  void OnPaint(gfx::Canvas* canvas) override;
+
+ private:
+  TabStrip* const tab_strip_;
+  const TabGroupId group_;
+
+  // The underline color is the group color.
+  SkColor GetColor();
+
+  // The underline starts at the left edge of the header chip.
+  int GetStart();
+
+  // The underline ends at the right edge of the last grouped tab's close
+  // button.
+  int GetEnd();
+
+  DISALLOW_COPY_AND_ASSIGN(TabGroupUnderline);
+};
+
+#endif  // CHROME_BROWSER_UI_VIEWS_TABS_TAB_GROUP_UNDERLINE_H_

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

@@ -42,6 +42,7 @@
 #include "chrome/browser/ui/views/tabs/tab.h"
 #include "chrome/browser/ui/views/tabs/tab_drag_controller.h"
 #include "chrome/browser/ui/views/tabs/tab_group_header.h"
+#include "chrome/browser/ui/views/tabs/tab_group_underline.h"
 #include "chrome/browser/ui/views/tabs/tab_hover_card_bubble_view.h"
 #include "chrome/browser/ui/views/tabs/tab_strip_controller.h"
 #include "chrome/browser/ui/views/tabs/tab_strip_layout_helper.h"
@@ -1143,6 +1144,12 @@ void TabStrip::ChangeTabGroup(int model_index,
         base::BindOnce(&TabStrip::OnGroupCloseAnimationCompleted,
                        base::Unretained(this), new_group.value()));
     group_headers_[new_group.value()] = std::move(header);
+
+    auto underline =
+        std::make_unique<TabGroupUnderline>(this, new_group.value());
+    underline->set_owned_by_client();
+    AddChildView(underline.get());
+    group_underlines_[new_group.value()] = std::move(underline);
   }
   if (old_group.has_value()) {
     if (controller_->ListTabsInGroup(old_group.value()).size() == 0) {
@@ -1159,8 +1166,6 @@ void TabStrip::ChangeTabGroup(int model_index,
 
 void TabStrip::GroupVisualsChanged(TabGroupId group) {
   group_headers_[group]->VisualsChanged();
-  for (int i : controller_->ListTabsInGroup(group))
-    tabs_.view_at(i)->GroupColorChanged();
   // The group title may have changed size.
   UpdateIdealBounds();
   AnimateToIdealBounds();
@@ -1832,6 +1837,10 @@ void TabStrip::PaintChildren(const views::PaintInfo& paint_info) {
   for (const auto& header_pair : group_headers_)
     header_pair.second->Paint(paint_info);
 
+  // Paint group underlines.
+  for (const auto& underline_pair : group_underlines_)
+    underline_pair.second->Paint(paint_info);
+
   // Always paint the active tab over all the inactive tabs.
   if (active_tab && !is_dragging)
     active_tab->Paint(paint_info);
@@ -2410,6 +2419,7 @@ void TabStrip::OnTabCloseAnimationCompleted(Tab* tab) {
 
 void TabStrip::OnGroupCloseAnimationCompleted(TabGroupId group) {
   group_headers_.erase(group);
+  group_underlines_.erase(group);
   // TODO(crbug.com/905491): We might want to simulate a mouse move here, like
   // we do in OnTabCloseAnimationCompleted.
 }

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

@@ -44,6 +44,7 @@ class NewTabButton;
 class StackedTabStripLayout;
 class Tab;
 class TabGroupHeader;
+class TabGroupUnderline;
 class TabGroupId;
 class TabHoverCardBubbleView;
 class TabStripController;
@@ -197,6 +198,9 @@ class TabStrip : public views::AccessiblePaneView,
   // TODO(pkasting): Make const correct
   Tab* tab_at(int index) const { return tabs_.view_at(index); }
 
+  // Returns the TabGroupHeader with ID |id|.
+  TabGroupHeader* group_header(TabGroupId id) { return GetGroupHeaders()[id]; }
+
   // Returns the NewTabButton.
   NewTabButton* new_tab_button() { return new_tab_button_; }
 
@@ -583,6 +587,9 @@ class TabStrip : public views::AccessiblePaneView,
   // Map associating each group to its TabGroupHeader instance.
   std::map<TabGroupId, std::unique_ptr<TabGroupHeader>> group_headers_;
 
+  // Map associating each group to its TabGroupUnderline instance.
+  std::map<TabGroupId, std::unique_ptr<TabGroupUnderline>> group_underlines_;
+
   // The view tracker is used to keep track of if the hover card has been
   // destroyed by its widget.
   TabHoverCardBubbleView* hover_card_ = nullptr;

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

@@ -18,6 +18,7 @@
 #include "chrome/browser/ui/views/tabs/tab.h"
 #include "chrome/browser/ui/views/tabs/tab_animation.h"
 #include "chrome/browser/ui/views/tabs/tab_group_header.h"
+#include "chrome/browser/ui/views/tabs/tab_group_underline.h"
 #include "chrome/browser/ui/views/tabs/tab_icon.h"
 #include "chrome/browser/ui/views/tabs/tab_renderer_data.h"
 #include "chrome/browser/ui/views/tabs/tab_strip_controller.h"
@@ -223,6 +224,13 @@ class TabStripTest : public ChromeViewsTestBase,
     return result;
   }
 
+  std::vector<TabGroupUnderline*> ListGroupUnderlines() const {
+    std::vector<TabGroupUnderline*> result;
+    for (auto const& underline_pair : tab_strip_->group_underlines_)
+      result.push_back(underline_pair.second.get());
+    return result;
+  }
+
   // Owned by TabStrip.
   FakeBaseTabStripController* controller_ = nullptr;
   TabStrip* tab_strip_ = nullptr;
@@ -1171,6 +1179,43 @@ TEST_P(TabStripTest, DeleteTabGroupHeaderWhenEmpty) {
   EXPECT_EQ(0u, ListGroupHeaders().size());
 }
 
+TEST_P(TabStripTest, GroupUnderlineBasics) {
+  tab_strip_->SetBounds(0, 0, 1000, 100);
+  bounds_animator()->SetAnimationDuration(base::TimeDelta());
+  tab_strip_->AddTabAt(0, TabRendererData(), false);
+  tab_strip_->AddTabAt(1, TabRendererData(), false);
+
+  base::Optional<TabGroupId> group = TabGroupId::GenerateNew();
+  controller_->MoveTabIntoGroup(0, group);
+  CompleteAnimationAndLayout();
+
+  std::vector<TabGroupUnderline*> underlines = ListGroupUnderlines();
+  EXPECT_EQ(1u, underlines.size());
+  TabGroupUnderline* underline = underlines[0];
+  // Update underline manually in the absence of a real Paint cycle.
+  underline->UpdateVisuals();
+  constexpr int kInset = 20;
+  constexpr int kStrokeWidth = 2;
+  EXPECT_EQ(underline->x(), kInset);
+  EXPECT_GT(underline->width(), 0);
+  EXPECT_EQ(underline->bounds().right(),
+            tab_strip_->tab_at(0)->bounds().right() - kInset);
+  EXPECT_EQ(underline->height(), kStrokeWidth);
+}
+
+TEST_P(TabStripTest, DeleteTabGroupUnderlineWhenEmpty) {
+  tab_strip_->AddTabAt(0, TabRendererData(), false);
+  tab_strip_->AddTabAt(1, TabRendererData(), false);
+  base::Optional<TabGroupId> group = TabGroupId::GenerateNew();
+  controller_->MoveTabIntoGroup(0, group);
+  controller_->MoveTabIntoGroup(1, group);
+  controller_->MoveTabIntoGroup(0, base::nullopt);
+
+  EXPECT_EQ(1u, ListGroupUnderlines().size());
+  controller_->MoveTabIntoGroup(1, base::nullopt);
+  EXPECT_EQ(0u, ListGroupUnderlines().size());
+}
+
 TEST_P(TabStripTest, ChangingLayoutTypeResizesTabs) {
   tab_strip_->SetBounds(0, 0, 1000, 100);
 

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

@@ -653,9 +653,14 @@ float GM2TabStyle::GetThrobValue() const {
 }
 
 int GM2TabStyle::GetStrokeThickness(bool should_paint_as_active) const {
-  return (tab_->IsActive() || should_paint_as_active)
-             ? tab_->controller()->GetStrokeThickness()
-             : 0;
+  base::Optional<SkColor> group_color = tab_->GetGroupColor();
+  if (group_color.has_value() && tab_->IsActive())
+    return 2;
+
+  if (tab_->IsActive() || should_paint_as_active)
+    return tab_->controller()->GetStrokeThickness();
+
+  return 0;
 }
 
 bool GM2TabStyle::ShouldPaintTabBackgroundColor(
@@ -679,20 +684,6 @@ SkColor GM2TabStyle::GetTabBackgroundColor(TabActive active) const {
   SkColor color = tab_->controller()->GetTabBackgroundColor(
       active, BrowserNonClientFrameView::kUseCurrent);
 
-  base::Optional<SkColor> group_color = tab_->GetGroupColor();
-  if (group_color.has_value()) {
-    if (tab_->IsActive()) {
-      color = group_color.value();
-    } else {
-      // Tint with group color. With a dark scheme, the tint needs a higher
-      // contrast to stand out effectively.
-      const float target_contrast = color_utils::IsDark(color) ? 1.8f : 1.2f;
-      color = color_utils::BlendForMinContrast(
-                  color, color, group_color.value(), target_contrast)
-                  .color;
-    }
-  }
-
   return color;
 }
 
@@ -728,11 +719,14 @@ void GM2TabStyle::PaintTabBackground(gfx::Canvas* canvas,
   // |y_inset| is only set when |fill_id| is being used.
   DCHECK(!y_inset || fill_id.has_value());
 
+  base::Optional<SkColor> group_color = tab_->GetGroupColor();
+
   PaintTabBackgroundFill(canvas, active,
                          active == TabActive::kInactive && IsHoverActive(),
                          fill_id, y_inset);
-  PaintBackgroundStroke(canvas, active,
-                        tab_->controller()->GetToolbarTopSeparatorColor());
+  PaintBackgroundStroke(
+      canvas, active,
+      group_color.value_or(tab_->controller()->GetToolbarTopSeparatorColor()));
   PaintSeparators(canvas);
 }