Separate tab icon and throbber into a separate view.

This class handles the favicon, loading throbber, attention indicator,
and sad tab animations. The separation makes the Tab class cleaner and
allows this complex view to be re-used in the experimental tab class
in the future.

The network state enum was moved out from TabRendererData into its
own class. It was updated to an enum class with new-style naming.

Throbber stepping was moved to be entirely within the tab strip rather
than round-tripping through the TabStripController.

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

chrome/browser/ui/BUILD.gn

@@ -907,6 +907,8 @@ split_static_library("ui") {
       "tabs/tab_metrics_logger.h",
       "tabs/tab_metrics_logger_impl.cc",
       "tabs/tab_metrics_logger_impl.h",
+      "tabs/tab_network_state.cc",
+      "tabs/tab_network_state.h",
       "tabs/tab_strip_model.cc",
       "tabs/tab_strip_model.h",
       "tabs/tab_strip_model_delegate.h",
@@ -3079,6 +3081,8 @@ split_static_library("ui") {
         "views/tabs/tab_controller.h",
         "views/tabs/tab_drag_controller.cc",
         "views/tabs/tab_drag_controller.h",
+        "views/tabs/tab_icon.cc",
+        "views/tabs/tab_icon.h",
         "views/tabs/tab_strip.cc",
         "views/tabs/tab_strip.h",
         "views/tabs/tab_strip_controller.h",

chrome/browser/ui/tabs/tab_change_type.h

@@ -13,7 +13,11 @@ enum class TabChangeType {
   // Only the loading state changed.
   kLoadingOnly,
 
-  // Only the title changed and page isn't loading.
+  // Only the title changed and page isn't loading. This is used to set the
+  // attention state for background pinned tabs. This has to be one
+  // notification since the title changed and loading state updates are
+  // normally broadcast asynchronously, making it hard to tell if the page was
+  // definitely done loading when the title changed.
   kTitleNotLoading
 };
 

chrome/browser/ui/tabs/tab_network_state.cc

+// Copyright 2017 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/tabs/tab_network_state.h"
+
+#include "content/public/browser/navigation_entry.h"
+#include "content/public/browser/web_contents.h"
+
+TabNetworkState TabNetworkStateForWebContents(content::WebContents* contents) {
+  DCHECK(contents);
+
+  if (!contents->IsLoadingToDifferentDocument()) {
+    content::NavigationEntry* entry =
+        contents->GetController().GetLastCommittedEntry();
+    if (entry && (entry->GetPageType() == content::PAGE_TYPE_ERROR))
+      return TabNetworkState::kError;
+    return TabNetworkState::kNone;
+  }
+
+  if (contents->IsWaitingForResponse())
+    return TabNetworkState::kWaiting;
+  return TabNetworkState::kLoading;
+}

chrome/browser/ui/tabs/tab_network_state.h

+// Copyright 2017 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_TABS_TAB_NETWORK_STATE_H_
+#define CHROME_BROWSER_UI_TABS_TAB_NETWORK_STATE_H_
+
+namespace content {
+class WebContents;
+}
+
+// The types of network activity for a tab. The network state of a tab may be
+// used to alter the UI (e.g. show different kinds of loading animations).
+enum class TabNetworkState {
+  kNone,     // No network activity.
+  kWaiting,  // Waiting for a connection.
+  kLoading,  // Connected, transferring data.
+  kError,    // Encountered a network error.
+};
+
+// Computes tthe TabNetworkState for the given WebContents.
+TabNetworkState TabNetworkStateForWebContents(content::WebContents* contents);
+
+#endif  // CHROME_BROWSER_UI_TABS_TAB_NETWORK_STATE_H_

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

@@ -272,16 +272,15 @@ IN_PROC_BROWSER_TEST_F(BrowserViewTest, TitleAndLoadState) {
           base::FilePath(FILE_PATH_LITERAL("title2.html"))),
       0, WindowOpenDisposition::CURRENT_TAB, ui_test_utils::BROWSER_TEST_NONE);
   EXPECT_TRUE(browser()->tab_strip_model()->TabsAreLoading());
-  EXPECT_EQ(TabRendererData::NETWORK_STATE_WAITING,
+  EXPECT_EQ(TabNetworkState::kWaiting,
             tab_strip->tab_at(0)->data().network_state);
   EXPECT_EQ(test_title, title_watcher.WaitAndGetTitle());
   EXPECT_TRUE(browser()->tab_strip_model()->TabsAreLoading());
-  EXPECT_EQ(TabRendererData::NETWORK_STATE_LOADING,
+  EXPECT_EQ(TabNetworkState::kLoading,
             tab_strip->tab_at(0)->data().network_state);
 
   // Now block for the navigation to complete.
   navigation_watcher.Wait();
   EXPECT_FALSE(browser()->tab_strip_model()->TabsAreLoading());
-  EXPECT_EQ(TabRendererData::NETWORK_STATE_NONE,
-            tab_strip->tab_at(0)->data().network_state);
+  EXPECT_EQ(TabNetworkState::kNone, tab_strip->tab_at(0)->data().network_state);
 }

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

@@ -24,6 +24,7 @@
 #include "chrome/browser/ui/browser_tabstrip.h"
 #include "chrome/browser/ui/tab_ui_helper.h"
 #include "chrome/browser/ui/tabs/tab_menu_model.h"
+#include "chrome/browser/ui/tabs/tab_network_state.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/tabs/tab_strip_model_delegate.h"
 #include "chrome/browser/ui/tabs/tab_utils.h"
@@ -64,23 +65,6 @@ using content::WebContents;
 
 namespace {
 
-TabRendererData::NetworkState TabContentsNetworkState(
-    WebContents* contents) {
-  DCHECK(contents);
-
-  if (!contents->IsLoadingToDifferentDocument()) {
-    content::NavigationEntry* entry =
-        contents->GetController().GetLastCommittedEntry();
-    if (entry && (entry->GetPageType() == content::PAGE_TYPE_ERROR))
-      return TabRendererData::NETWORK_STATE_ERROR;
-    return TabRendererData::NETWORK_STATE_NONE;
-  }
-
-  if (contents->IsWaitingForResponse())
-    return TabRendererData::NETWORK_STATE_WAITING;
-  return TabRendererData::NETWORK_STATE_LOADING;
-}
-
 bool DetermineTabStripLayoutStacked(PrefService* prefs, bool* adjust_layout) {
   *adjust_layout = false;
   // For ash, always allow entering stacked mode.
@@ -321,11 +305,6 @@ void BrowserTabStripController::ShowContextMenuForTab(
   context_menu_contents_->RunMenuAt(p, source_type);
 }
 
-void BrowserTabStripController::UpdateLoadingAnimations() {
-  for (int i = 0, tab_count = tabstrip_->tab_count(); i < tab_count; ++i)
-    tabstrip_->tab_at(i)->StepLoadingAnimation();
-}
-
 int BrowserTabStripController::HasAvailableDragActions() const {
   return model_->delegate()->GetDragActions();
 }
@@ -537,7 +516,7 @@ TabRendererData BrowserTabStripController::TabRendererDataFromModel(
   TabRendererData data;
   TabUIHelper* tab_ui_helper = TabUIHelper::FromWebContents(contents);
   data.favicon = tab_ui_helper->GetFavicon().AsImageSkia();
-  data.network_state = TabContentsNetworkState(contents);
+  data.network_state = TabNetworkStateForWebContents(contents);
   data.title = tab_ui_helper->GetTitle();
   data.url = contents->GetURL();
   data.crashed_status = contents->GetCrashedStatus();

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

@@ -64,7 +64,6 @@ class BrowserTabStripController : public TabStripController,
   void ShowContextMenuForTab(Tab* tab,
                              const gfx::Point& p,
                              ui::MenuSourceType source_type) override;
-  void UpdateLoadingAnimations() override;
   int HasAvailableDragActions() const override;
   void OnDropIndexUpdate(int index, bool drop_before) override;
   void PerformDrop(bool drop_before, int index, const GURL& url) override;

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

@@ -99,9 +99,6 @@ void FakeBaseTabStripController::ShowContextMenuForTab(
     ui::MenuSourceType source_type) {
 }
 
-void FakeBaseTabStripController::UpdateLoadingAnimations() {
-}
-
 int FakeBaseTabStripController::HasAvailableDragActions() const {
   return 0;
 }

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

@@ -42,7 +42,6 @@ class FakeBaseTabStripController : public TabStripController {
   void ShowContextMenuForTab(Tab* tab,
                              const gfx::Point& p,
                              ui::MenuSourceType source_type) override;
-  void UpdateLoadingAnimations() override;
   int HasAvailableDragActions() const override;
   void OnDropIndexUpdate(int index, bool drop_before) override;
   void PerformDrop(bool drop_before, int index, const GURL& url) override;

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

@@ -18,7 +18,6 @@
 #include "ui/gfx/animation/animation_delegate.h"
 #include "ui/gfx/animation/linear_animation.h"
 #include "ui/gfx/geometry/point.h"
-#include "ui/gfx/image/image_skia.h"
 #include "ui/gfx/paint_throbber.h"
 #include "ui/views/context_menu_controller.h"
 #include "ui/views/controls/button/button.h"
@@ -29,6 +28,7 @@
 class AlertIndicatorButton;
 class TabCloseButton;
 class TabController;
+class TabIcon;
 
 namespace gfx {
 class Animation;
@@ -90,8 +90,8 @@ class Tab : public gfx::AnimationDelegate,
   // Returns true if the tab is selected.
   bool IsSelected() const;
 
-  // Sets the data this tabs displays. Invokes DataChanged. Should only be
-  // called after Tab is added to widget hierarchy.
+  // Sets the data this tabs displays. Should only be called after Tab is added
+  // to widget hierarchy.
   void SetData(TabRendererData data);
   const TabRendererData& data() const { return data_; }
 
@@ -168,14 +168,8 @@ class Tab : public gfx::AnimationDelegate,
   friend class AlertIndicatorButtonTest;
   friend class TabTest;
   friend class TabStripTest;
-  friend class ThrobberView;
   FRIEND_TEST_ALL_PREFIXES(TabStripTest, TabCloseButtonVisibilityWhenStacked);
 
-  // The animation object used to swap the favicon with the sad tab icon.
-  class FaviconCrashAnimation;
-
-  class ThrobberView;
-
   // gfx::AnimationDelegate:
   void AnimationProgressed(const gfx::Animation* animation) override;
   void AnimationCanceled(const gfx::Animation* animation) override;
@@ -216,11 +210,9 @@ class Tab : public gfx::AnimationDelegate,
   void OnGestureEvent(ui::GestureEvent* event) override;
 
   // Invoked from Layout to adjust the position of the favicon or alert
-  // indicator for pinned tabs.
-  void MaybeAdjustLeftForPinnedTab(gfx::Rect* bounds) const;
-
-  // Invoked from SetData after |data_| has been updated to the new data.
-  void DataChanged(const TabRendererData& old);
+  // indicator for pinned tabs. The visual_width parameter is how wide the
+  // icon looks (rather than how wide the bounds are).
+  void MaybeAdjustLeftForPinnedTab(gfx::Rect* bounds, int visual_width) const;
 
   // Paints with the normal tab style.  If |clip| is non-empty, the tab border
   // should be clipped against it.
@@ -253,29 +245,12 @@ class Tab : public gfx::AnimationDelegate,
                                 bool active,
                                 SkColor color);
 
-  // Paints the attention indicator and |favicon_|. |favicon_| may be null.
-  // |favicon_draw_bounds| is |favicon_bounds_| adjusted for rtl and clipped to
-  // the bounds of the tab.
-  void PaintAttentionIndicatorAndIcon(gfx::Canvas* canvas,
-                                      const gfx::Rect& favicon_draw_bounds);
-
-  // Paints the favicon, mirrored for RTL if needed.
-  void PaintIcon(gfx::Canvas* canvas);
-
-  // Updates the throbber.
-  void UpdateThrobber(const TabRendererData& old);
-
-  // Sets the throbber visibility according to the state in |data_|.
-  void RefreshThrobber();
-
   // Returns the number of favicon-size elements that can fit in the tab's
   // current size.
   int IconCapacity() const;
 
-  // Returns whether the Tab should display a throbber.
-  bool ShouldShowThrobber() const;
-
-  // Returns whether the Tab should display a favicon.
+  // Returns whether the Tab should display the icon view, which includes the
+  // favicon and loading animation.
   bool ShouldShowIcon() const;
 
   // Returns whether the Tab should display the alert indicator.
@@ -293,20 +268,11 @@ class Tab : public gfx::AnimationDelegate,
   // mini tab title change and pulsing.
   double GetThrobValue();
 
-  // Set the temporary offset for the favicon. This is used during the crash
-  // animation.
-  void SetFaviconHidingOffset(int offset);
-
-  void SetShouldDisplayCrashedFavicon(bool value);
-
   // Recalculates the correct |button_color_| and resets the title, alert
   // indicator, and close button colors if necessary.  This should be called any
   // time the theme or active state may have changed.
   void OnButtonColorMaybeChanged();
 
-  // Schedules repaint task for icon.
-  void ScheduleIconPaint();
-
   // The controller, never NULL.
   TabController* const controller_;
 
@@ -321,28 +287,12 @@ class Tab : public gfx::AnimationDelegate,
   // True if the tab has been detached.
   bool detached_ = false;
 
-  // The offset used to animate the favicon location. This is used when the tab
-  // crashes.
-  int favicon_hiding_offset_ = 0;
-
-  bool should_display_crashed_favicon_ = false;
-
-  enum AttentionType : int {
-    kPinnedTabTitleChange = 1 << 0,     // The title of a pinned tab changed.
-    kBlockedWebContents = 1 << 1,       // The WebContents is marked as blocked.
-    kTabWantsAttentionStatus = 1 << 2,  // SetTabNeedsAttention() was called.
-  };
-  int current_attention_types_ = 0;
-
   // Whole-tab throbbing "pulse" animation.
   gfx::ThrobAnimation pulse_animation_;
 
-  // Crash icon animation (in place of favicon).
-  std::unique_ptr<FaviconCrashAnimation> crash_icon_animation_;
-
   scoped_refptr<gfx::AnimationContainer> animation_container_;
 
-  ThrobberView* throbber_ = nullptr;
+  TabIcon* icon_ = nullptr;
   AlertIndicatorButton* alert_indicator_button_ = nullptr;
   TabCloseButton* close_button_ = nullptr;
 
@@ -357,9 +307,6 @@ class Tab : public gfx::AnimationDelegate,
 
   views::GlowHoverController hover_controller_;
 
-  // The bounds of various sections of the display.
-  gfx::Rect favicon_bounds_;
-
   // The offset used to paint the inactive background image.
   gfx::Point background_offset_;
 
@@ -378,11 +325,6 @@ class Tab : public gfx::AnimationDelegate,
   // The current color of the alert indicator and close button icons.
   SkColor button_color_ = SK_ColorTRANSPARENT;
 
-  // The favicon for the tab. This might be the sad tab icon or a copy of
-  // data().favicon and may be modified for theming. It is created on demand
-  // and thus may be null.
-  gfx::ImageSkia favicon_;
-
   class BackgroundCache {
    public:
     BackgroundCache();

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

+// Copyright 2017 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_ICON_H_
+#define CHROME_BROWSER_UI_VIEWS_TABS_TAB_ICON_H_
+
+#include "base/macros.h"
+#include "base/time/time.h"
+#include "chrome/browser/ui/tabs/tab_network_state.h"
+#include "ui/gfx/image/image_skia.h"
+#include "ui/gfx/paint_throbber.h"
+#include "ui/views/view.h"
+
+class GURL;
+
+// View that displays the favicon, sad tab, throbber, and attention indicator
+// in a tab.
+//
+// The icon will be drawn in the upper left (upper right for RTL). Normally you
+// would lay this out so the top is where you want the icon to be positioned,
+// the width is TabIcon::GetIdealWidth(), and the height goes down to the
+// bottom of the enclosing view (this is so the crashed tab can animate out of
+// the bottom).
+class TabIcon : public views::View {
+ public:
+  // Attention indicator types (use as a bitmask). There is only one visual
+  // representation, but the state of each of these is tracked separately and
+  // the indicator is shown as long as one is enabled.
+  enum class AttentionType {
+    kPinnedTabTitleChange = 1 << 0,     // The title of a pinned tab changed.
+    kBlockedWebContents = 1 << 1,       // The WebContents is marked as blocked.
+    kTabWantsAttentionStatus = 1 << 2,  // Tab::SetTabNeedsAttention() called.
+  };
+
+  TabIcon();
+  ~TabIcon() override;
+
+  // NOTE The state setting functions below do not automatically scheule a
+  // repaint. They are normally updated all at once, and if each scheduled
+  // a paint it would be extra work. The caller should SchedulePaint() when the
+  // state changes.
+
+  // Sets the icon. Depending on the URL the icon may be automatically themed.
+  void SetIcon(const GURL& url, const gfx::ImageSkia& favicon);
+
+  // For certain types of tabs the loading animation is not desired so the
+  // caller can set inhibit_loading_animation to true. When false, the loading
+  // animation state will be derived from the network state.
+  void SetNetworkState(TabNetworkState network_state,
+                       bool inhibit_loading_animation);
+
+  void SetIsCrashed(bool is_crashed);
+
+  // Enables or disables the given attention type. The attention indicator
+  // will be shown as long as any of the types are enabled.
+  void SetAttention(AttentionType type, bool enabled);
+
+  bool ShowingLoadingAnimation() const;
+  bool ShowingAttentionIndicator() const;
+
+  // Sets whether this object can paint to a layer. When the loading animation
+  // is running, painting to a layer saves painting overhead. But if the tab is
+  // being painted to some other context than the window, the layered painting
+  // won't work.
+  void SetCanPaintToLayer(bool can_paint_to_layer);
+
+  // The loading animation only steps when this function is called. This
+  // ensures that all loading animations step at the same time and it keeps the
+  // number of timers down when lots of tabs are loading.
+  void StepLoadingAnimation();
+
+ private:
+  class CrashAnimation;
+  friend CrashAnimation;
+
+  // views::View:
+  void OnPaint(gfx::Canvas* canvas) override;
+  void OnThemeChanged() override;
+
+  // Paints the attention indicator and |favicon_| at the given location.
+  void PaintAttentionIndicatorAndIcon(gfx::Canvas* canvas,
+                                      const gfx::ImageSkia& icon,
+                                      const gfx::Rect& bounds);
+
+  void PaintLoadingAnimation(gfx::Canvas* canvas, const gfx::Rect& bounds);
+
+  // Creates or destroys the layer according to the current animation state and
+  // whether a layer can be used.
+  void RefreshLayer();
+
+  void UpdateThemedFavicon();
+
+  gfx::ImageSkia ThemeImage(const gfx::ImageSkia& source);
+
+  gfx::ImageSkia favicon_;
+  TabNetworkState network_state_ = TabNetworkState::kNone;
+  bool is_crashed_ = false;
+  int attention_types_ = 0;  // Bitmask of AttentionType.
+
+  // Value from last call to SetNetworkState. When true, the network loading
+  // animation will not be shown.
+  bool inhibit_loading_animation_ = false;
+
+  // The point in time when the tab icon was first painted in the waiting state.
+  base::TimeTicks waiting_start_time_;
+
+  // The point in time when the tab icon was first painted in the loading state.
+  base::TimeTicks loading_start_time_;
+
+  // Paint state for the loading animation after the most recent waiting paint.
+  gfx::ThrobberWaitingState waiting_state_;
+
+  // When the favicon_ has theming applied to it, the themed version will be
+  // cached here. If this isNull(), then there is no theming and favicon_
+  // should be used.
+  gfx::ImageSkia themed_favicon_;
+
+  // May be different than is_crashed when the crashed icon is animating in.
+  bool should_display_crashed_favicon_ = false;
+
+  // Drawn when should_display_crashed_favicon_ is set. Created lazily.
+  gfx::ImageSkia crashed_icon_;
+
+  // The fraction the icon is hidden by for the crashed tab animation.
+  // When this is 0 it will be drawn at the normal location, and when this is 1
+  // it will be drawn off the bottom.
+  double hiding_fraction_ = 0.0;
+
+  // Crash animation (in place of favicon). Lazily created since most of the
+  // time it will be unneeded.
+  std::unique_ptr<CrashAnimation> crash_animation_;
+
+  bool can_paint_to_layer_ = false;
+
+  DISALLOW_COPY_AND_ASSIGN(TabIcon);
+};
+
+#endif  // CHROME_BROWSER_UI_VIEWS_TABS_TAB_ICON_H_

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

@@ -6,6 +6,7 @@
 #define CHROME_BROWSER_UI_VIEWS_TABS_TAB_RENDERER_DATA_H_
 
 #include "base/strings/string16.h"
+#include "chrome/browser/ui/tabs/tab_network_state.h"
 #include "chrome/browser/ui/tabs/tab_utils.h"
 #include "chrome/browser/ui/views/chrome_views_export.h"
 #include "ui/gfx/image/image_skia.h"
@@ -13,16 +14,6 @@
 
 // Wraps the state needed by the renderers.
 struct CHROME_VIEWS_EXPORT TabRendererData {
-  // Different types of network activity for a tab. The NetworkState of a tab
-  // may be used to alter the UI (e.g. show different kinds of loading
-  // animations).
-  enum NetworkState {
-    NETWORK_STATE_NONE,     // no network activity.
-    NETWORK_STATE_WAITING,  // waiting for a connection.
-    NETWORK_STATE_LOADING,  // connected, transferring data.
-    NETWORK_STATE_ERROR,    // Encountered a network error.
-  };
-
   TabRendererData();
   TabRendererData(const TabRendererData& other);
   TabRendererData(TabRendererData&& other);
@@ -39,7 +30,7 @@ struct CHROME_VIEWS_EXPORT TabRendererData {
   bool IsCrashed() const;
 
   gfx::ImageSkia favicon;
-  NetworkState network_state = NETWORK_STATE_NONE;
+  TabNetworkState network_state = TabNetworkState::kNone;
   base::string16 title;
   GURL url;
   base::TerminationStatus crashed_status =

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

@@ -74,9 +74,6 @@ class TabStripController {
                                      const gfx::Point& p,
                                      ui::MenuSourceType source_type) = 0;
 
-  // Updates the loading animations of all the tabs.
-  virtual void UpdateLoadingAnimations() = 0;
-
   // Returns true if the associated TabStrip's delegate supports tab moving or
   // detaching. Used by the Frame to determine if dragging on the Tab
   // itself should move the window in cases where there's only one

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

@@ -340,8 +340,7 @@ bool TabStripExperimental::IsTabCrashed(int tab_index) const {
 
 bool TabStripExperimental::TabHasNetworkError(int tab_index) const {
   /* TODO(brettw) tab data.
-  return tab_at(tab_index)->data().network_state ==
-         TabRendererData::NETWORK_STATE_ERROR;
+  return tab_at(tab_index)->data().network_state == TabNetworkState::kError;
   */
   return false;
 }

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

@@ -361,8 +361,7 @@ bool TabStripImpl::IsTabCrashed(int tab_index) const {
 }
 
 bool TabStripImpl::TabHasNetworkError(int tab_index) const {
-  return tab_at(tab_index)->data().network_state ==
-         TabRendererData::NETWORK_STATE_ERROR;
+  return tab_at(tab_index)->data().network_state == TabNetworkState::kError;
 }
 
 TabAlertState TabStripImpl::GetTabAlertState(int tab_index) const {
@@ -370,7 +369,8 @@ TabAlertState TabStripImpl::GetTabAlertState(int tab_index) const {
 }
 
 void TabStripImpl::UpdateLoadingAnimations() {
-  controller_->UpdateLoadingAnimations();
+  for (int i = 0; i < tab_count(); i++)
+    tab_at(i)->StepLoadingAnimation();
 }
 
 void TabStripImpl::SetStackedLayout(bool stacked_layout) {
@@ -686,9 +686,7 @@ void TabStripImpl::SetSelection(const ui::ListSelectionModel& old_selection,
 }
 
 void TabStripImpl::TabTitleChangedNotLoading(int model_index) {
-  Tab* tab = tab_at(model_index);
-  if (tab->data().pinned && !tab->IsActive())
-    tab->TabTitleChangedNotLoading();
+  tab_at(model_index)->TabTitleChangedNotLoading();
 }
 
 void TabStripImpl::SetTabNeedsAttention(int model_index, bool attention) {

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

@@ -134,7 +134,8 @@ class TabStripImpl : public TabStrip,
   void SetSelection(const ui::ListSelectionModel& old_selection,
                     const ui::ListSelectionModel& new_selection);
 
-  // Invoked when the title of a tab changes and the tab isn't loading.
+  // Invoked when the title of a tab changes and the tab isn't loading. This is
+  // used by pinned tabs to indicate they need attention.
   void TabTitleChangedNotLoading(int model_index);
 
   // Invoked when a tab needs to show UI that it needs the user's attention.

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

@@ -9,6 +9,7 @@
 #include "chrome/browser/ui/layout_constants.h"
 #include "chrome/browser/ui/views/tabs/fake_base_tab_strip_controller.h"
 #include "chrome/browser/ui/views/tabs/tab.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"
 #include "chrome/browser/ui/views/tabs/tab_strip_impl.h"
@@ -122,7 +123,7 @@ class TabStripTest : public views::ViewsTestBase {
 
  protected:
   bool IsShowingAttentionIndicator(int model_index) {
-    return tab_strip_->tab_at(model_index)->current_attention_types_ > 0;
+    return tab_strip_->tab_at(model_index)->icon_->ShowingAttentionIndicator();
   }
 
   // Checks whether |tab| contains |point_in_tabstrip_coords|, where the point

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

@@ -13,9 +13,11 @@
 #include "chrome/browser/ui/views/tabs/alert_indicator_button.h"
 #include "chrome/browser/ui/views/tabs/tab_close_button.h"
 #include "chrome/browser/ui/views/tabs/tab_controller.h"
+#include "chrome/browser/ui/views/tabs/tab_icon.h"
 #include "chrome/grit/theme_resources.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/base/models/list_selection_model.h"
+#include "ui/gfx/favicon_size.h"
 #include "ui/views/controls/button/image_button.h"
 #include "ui/views/controls/label.h"
 #include "ui/views/test/views_test_base.h"
@@ -99,15 +101,7 @@ class TabTest : public views::ViewsTestBase {
     return tab.close_button_;
   }
 
-  static views::View* GetThrobberView(const Tab& tab) {
-    // Reinterpret to keep the definition encapsulated (which works so long as
-    // multiple inheritance isn't involved).
-    return reinterpret_cast<views::View*>(tab.throbber_);
-  }
-
-  static gfx::Rect GetFaviconBounds(const Tab& tab) {
-    return tab.favicon_bounds_;
-  }
+  static TabIcon* GetTabIcon(const Tab& tab) { return tab.icon_; }
 
   static int GetTitleWidth(const Tab& tab) {
     return tab.title_->bounds().width();
@@ -187,14 +181,21 @@ class TabTest : public views::ViewsTestBase {
     // are fully within the contents bounds.
     const gfx::Rect contents_bounds = tab.GetContentsBounds();
     if (tab.ShouldShowIcon()) {
-      EXPECT_LE(contents_bounds.x(), tab.favicon_bounds_.x());
+      EXPECT_LE(contents_bounds.x(), tab.icon_->x());
       if (tab.title_->visible())
-        EXPECT_LE(tab.favicon_bounds_.right(), tab.title_->x());
-      EXPECT_LE(contents_bounds.y(), tab.favicon_bounds_.y());
-      EXPECT_LE(tab.favicon_bounds_.bottom(), contents_bounds.bottom());
+        EXPECT_LE(tab.icon_->bounds().right(), tab.title_->x());
+      EXPECT_LE(contents_bounds.y(), tab.icon_->y());
+      EXPECT_LE(tab.icon_->bounds().bottom(), contents_bounds.bottom());
     }
-    if (tab.ShouldShowIcon() && tab.ShouldShowAlertIndicator())
-      EXPECT_LE(tab.favicon_bounds_.right(), GetAlertIndicatorBounds(tab).x());
+
+    if (tab.ShouldShowIcon() && tab.ShouldShowAlertIndicator()) {
+      // When checking for overlap, other views should not overlap the main
+      // favicon (covered by kFaviconSize) but can overlap the extra space
+      // reserved for the attention indicator.
+      int icon_visual_right = tab.icon_->bounds().x() + gfx::kFaviconSize;
+      EXPECT_LE(icon_visual_right, GetAlertIndicatorBounds(tab).x());
+    }
+
     if (tab.ShouldShowAlertIndicator()) {
       if (tab.title_->visible()) {
         EXPECT_LE(tab.title_->bounds().right(),
@@ -440,94 +441,93 @@ TEST_F(TabTest, LayeredThrobber) {
   widget.GetContentsView()->AddChildView(&tab);
   tab.SetBoundsRect(gfx::Rect(Tab::GetStandardSize()));
 
-  views::View* throbber = GetThrobberView(tab);
+  TabIcon* icon = GetTabIcon(tab);
   TabRendererData data;
   data.url = GURL("http://example.com");
-  EXPECT_FALSE(throbber->visible());
-  EXPECT_EQ(TabRendererData::NETWORK_STATE_NONE, tab.data().network_state);
-  EXPECT_EQ(throbber->bounds(), GetFaviconBounds(tab));
+  EXPECT_FALSE(icon->ShowingLoadingAnimation());
+  EXPECT_EQ(TabNetworkState::kNone, tab.data().network_state);
 
   // Simulate a "normal" tab load: should paint to a layer.
-  data.network_state = TabRendererData::NETWORK_STATE_WAITING;
+  data.network_state = TabNetworkState::kWaiting;
   tab.SetData(data);
   EXPECT_TRUE(tab_controller.CanPaintThrobberToLayer());
-  EXPECT_TRUE(throbber->visible());
-  EXPECT_TRUE(throbber->layer());
-  data.network_state = TabRendererData::NETWORK_STATE_LOADING;
+  EXPECT_TRUE(icon->ShowingLoadingAnimation());
+  EXPECT_TRUE(icon->layer());
+  data.network_state = TabNetworkState::kLoading;
   tab.SetData(data);
-  EXPECT_TRUE(throbber->visible());
-  EXPECT_TRUE(throbber->layer());
-  data.network_state = TabRendererData::NETWORK_STATE_NONE;
+  EXPECT_TRUE(icon->ShowingLoadingAnimation());
+  EXPECT_TRUE(icon->layer());
+  data.network_state = TabNetworkState::kNone;
   tab.SetData(data);
-  EXPECT_FALSE(throbber->visible());
+  EXPECT_FALSE(icon->ShowingLoadingAnimation());
 
-  // Simulate a tab that should hide throbber. It should never paint.
+  // Simulate a tab that should hide throbber.
   data.should_hide_throbber = true;
   tab.SetData(data);
-  EXPECT_FALSE(throbber->visible());
-  data.network_state = TabRendererData::NETWORK_STATE_WAITING;
+  EXPECT_FALSE(icon->ShowingLoadingAnimation());
+  data.network_state = TabNetworkState::kWaiting;
   tab.SetData(data);
-  EXPECT_FALSE(throbber->visible());
-  data.network_state = TabRendererData::NETWORK_STATE_LOADING;
+  EXPECT_FALSE(icon->ShowingLoadingAnimation());
+  data.network_state = TabNetworkState::kLoading;
   tab.SetData(data);
-  EXPECT_FALSE(throbber->visible());
-  data.network_state = TabRendererData::NETWORK_STATE_NONE;
+  EXPECT_FALSE(icon->ShowingLoadingAnimation());
+  data.network_state = TabNetworkState::kNone;
   tab.SetData(data);
-  EXPECT_FALSE(throbber->visible());
+  EXPECT_FALSE(icon->ShowingLoadingAnimation());
 
-  // Simulate a tab that should not hide throbber. It should paint.
+  // Simulate a tab that should not hide throbber.
   data.should_hide_throbber = false;
-  data.network_state = TabRendererData::NETWORK_STATE_WAITING;
+  data.network_state = TabNetworkState::kWaiting;
   tab.SetData(data);
   EXPECT_TRUE(tab_controller.CanPaintThrobberToLayer());
-  EXPECT_TRUE(throbber->visible());
-  EXPECT_TRUE(throbber->layer());
-  data.network_state = TabRendererData::NETWORK_STATE_LOADING;
+  EXPECT_TRUE(icon->ShowingLoadingAnimation());
+  EXPECT_TRUE(icon->layer());
+  data.network_state = TabNetworkState::kLoading;
   tab.SetData(data);
-  EXPECT_TRUE(throbber->visible());
-  EXPECT_TRUE(throbber->layer());
-  data.network_state = TabRendererData::NETWORK_STATE_NONE;
+  EXPECT_TRUE(icon->ShowingLoadingAnimation());
+  EXPECT_TRUE(icon->layer());
+  data.network_state = TabNetworkState::kNone;
   tab.SetData(data);
-  EXPECT_FALSE(throbber->visible());
+  EXPECT_FALSE(icon->ShowingLoadingAnimation());
 
   // After loading is done, simulate another resource starting to load.
-  data.network_state = TabRendererData::NETWORK_STATE_WAITING;
+  data.network_state = TabNetworkState::kWaiting;
   tab.SetData(data);
-  EXPECT_TRUE(throbber->visible());
+  EXPECT_TRUE(icon->ShowingLoadingAnimation());
 
   // Reset.
-  data.network_state = TabRendererData::NETWORK_STATE_NONE;
+  data.network_state = TabNetworkState::kNone;
   tab.SetData(data);
-  EXPECT_FALSE(throbber->visible());
+  EXPECT_FALSE(icon->ShowingLoadingAnimation());
 
   // Simulate a drag started and stopped during a load: layer painting stops
   // temporarily.
-  data.network_state = TabRendererData::NETWORK_STATE_WAITING;
+  data.network_state = TabNetworkState::kWaiting;
   tab.SetData(data);
-  EXPECT_TRUE(throbber->visible());
-  EXPECT_TRUE(throbber->layer());
+  EXPECT_TRUE(icon->ShowingLoadingAnimation());
+  EXPECT_TRUE(icon->layer());
   tab_controller.set_paint_throbber_to_layer(false);
   tab.StepLoadingAnimation();
-  EXPECT_TRUE(throbber->visible());
-  EXPECT_FALSE(throbber->layer());
+  EXPECT_TRUE(icon->ShowingLoadingAnimation());
+  EXPECT_FALSE(icon->layer());
   tab_controller.set_paint_throbber_to_layer(true);
   tab.StepLoadingAnimation();
-  EXPECT_TRUE(throbber->visible());
-  EXPECT_TRUE(throbber->layer());
-  data.network_state = TabRendererData::NETWORK_STATE_NONE;
+  EXPECT_TRUE(icon->ShowingLoadingAnimation());
+  EXPECT_TRUE(icon->layer());
+  data.network_state = TabNetworkState::kNone;
   tab.SetData(data);
-  EXPECT_FALSE(throbber->visible());
+  EXPECT_FALSE(icon->ShowingLoadingAnimation());
 
   // Simulate a tab load starting and stopping during tab dragging (or with
   // stacked tabs): no layer painting.
   tab_controller.set_paint_throbber_to_layer(false);
-  data.network_state = TabRendererData::NETWORK_STATE_WAITING;
+  data.network_state = TabNetworkState::kWaiting;
   tab.SetData(data);
-  EXPECT_TRUE(throbber->visible());
-  EXPECT_FALSE(throbber->layer());
-  data.network_state = TabRendererData::NETWORK_STATE_NONE;
+  EXPECT_TRUE(icon->ShowingLoadingAnimation());
+  EXPECT_FALSE(icon->layer());
+  data.network_state = TabNetworkState::kNone;
   tab.SetData(data);
-  EXPECT_FALSE(throbber->visible());
+  EXPECT_FALSE(icon->ShowingLoadingAnimation());
 }
 
 TEST_F(TabTest, TitleHiddenWhenSmall) {

ui/gfx/geometry/rect.h

@@ -61,12 +61,14 @@ class GFX_EXPORT Rect {
 #endif
 
   constexpr int x() const { return origin_.x(); }
+  // Sets the X position while preserving the width.
   void set_x(int x) {
     origin_.set_x(x);
     size_.set_width(GetClampedValue(x, width()));
   }
 
   constexpr int y() const { return origin_.y(); }
+  // Sets the Y position while preserving the height.
   void set_y(int y) {
     origin_.set_y(y);
     size_.set_height(GetClampedValue(y, height()));

ui/gfx/paint_throbber.h

@@ -22,8 +22,10 @@ class Rect;
 struct GFX_EXPORT ThrobberWaitingState {
   // The amount of time that was spent in the waiting state.
   base::TimeDelta elapsed_time;
+
   // The color of the arc in the waiting state.
-  SkColor color;
+  SkColor color = SK_ColorTRANSPARENT;
+
   // An opaque value used to cache calculations made by
   // PaintThrobberSpinningAfterWaiting.
   base::TimeDelta arc_time_offset;