Paint tab-loading throbbers into a ui::Layer.

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

Index: chrome/browser/ui/views/tabs/tab.cc
diff --git a/chrome/browser/ui/views/tabs/tab.cc b/chrome/browser/ui/views/tabs/tab.cc
index 6d0d78ae342129de7cfe6e2eb8a11b18c20d701b..dc2ae467324e77ce7a22cedbd7d4535f917a6a46 100644
--- a/chrome/browser/ui/views/tabs/tab.cc
+++ b/chrome/browser/ui/views/tabs/tab.cc
@@ -382,6 +382,80 @@ class Tab::TabCloseButton : public views::ImageButton,
 };
 
 ////////////////////////////////////////////////////////////////////////////////
+// ThrobberView
+//
+// A Layer-backed view for updating a waiting or loading tab throbber.
+class Tab::ThrobberView : public views::View {
+ public:
+  explicit ThrobberView(Tab* owner);
+
+  // Resets the times tracking when the throbber changes state.
+  void ResetStartTimes();
+
+ private:
+  // views::View:
+  bool CanProcessEventsWithinSubtree() const override;
+  void OnPaint(gfx::Canvas* canvas) override;
+
+  Tab* owner_;  // Weak. Owns |this|.
+
+  // 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 throbber after the most recent waiting paint.
+  gfx::ThrobberWaitingState waiting_state_;
+
+  DISALLOW_COPY_AND_ASSIGN(ThrobberView);
+};
+
+Tab::ThrobberView::ThrobberView(Tab* owner) : owner_(owner) {
+  // Since the throbber can animate for a long time, paint to a separate layer
+  // to reduce repaint overhead.
+  SetPaintToLayer(true);
+  SetFillsBoundsOpaquely(false);
+}
+
+void Tab::ThrobberView::ResetStartTimes() {
+  waiting_start_time_ = base::TimeTicks();
+  loading_start_time_ = base::TimeTicks();
+  waiting_state_ = gfx::ThrobberWaitingState();
+}
+
+bool Tab::ThrobberView::CanProcessEventsWithinSubtree() const {
+  return false;
+}
+
+void Tab::ThrobberView::OnPaint(gfx::Canvas* canvas) {
+  const TabRendererData::NetworkState state = owner_->data().network_state;
+  if (state == TabRendererData::NETWORK_STATE_NONE)
+    return;
+
+  ui::ThemeProvider* tp = GetThemeProvider();
+  const gfx::Rect bounds = GetLocalBounds();
+  if (state == TabRendererData::NETWORK_STATE_WAITING) {
+    if (waiting_start_time_ == base::TimeTicks())
+      waiting_start_time_ = base::TimeTicks::Now();
+
+    waiting_state_.elapsed_time = base::TimeTicks::Now() - waiting_start_time_;
+    gfx::PaintThrobberWaiting(
+        canvas, bounds, tp->GetColor(ThemeProperties::COLOR_THROBBER_WAITING),
+        waiting_state_.elapsed_time);
+  } else {
+    if (loading_start_time_ == base::TimeTicks())
+      loading_start_time_ = base::TimeTicks::Now();
+
+    waiting_state_.color =
+        tp->GetColor(ThemeProperties::COLOR_THROBBER_WAITING);
+    gfx::PaintThrobberSpinningAfterWaiting(
+        canvas, bounds, tp->GetColor(ThemeProperties::COLOR_THROBBER_SPINNING),
+        base::TimeTicks::Now() - loading_start_time_, &waiting_state_);
+  }
+}
+
+////////////////////////////////////////////////////////////////////////////////
 // ImageCacheEntry
 
 Tab::ImageCacheEntry::ImageCacheEntry()
@@ -412,6 +486,7 @@ Tab::Tab(TabController* controller)
       favicon_hiding_offset_(0),
       immersive_loading_step_(0),
       should_display_crashed_favicon_(false),
+      throbber_(nullptr),
       media_indicator_button_(nullptr),
       close_button_(nullptr),
       title_(new views::Label()),
@@ -440,6 +515,9 @@ Tab::Tab(TabController* controller)
   SetEventTargeter(
       scoped_ptr<views::ViewTargeter>(new views::ViewTargeter(this)));
 
+  throbber_ = new ThrobberView(this);
+  AddChildView(throbber_);
+
   media_indicator_button_ = new MediaIndicatorButton(this);
   AddChildView(media_indicator_button_);
 
@@ -494,6 +572,7 @@ void Tab::SetData(const TabRendererData& data) {
     return;
 
   TabRendererData old(data_);
+  UpdateLoadingAnimation(data.network_state);
   data_ = data;
 
   base::string16 title = data_.title;
@@ -552,9 +631,8 @@ void Tab::UpdateLoadingAnimation(TabRendererData::NetworkState state) {
     return;
   }
 
-  TabRendererData::NetworkState old_state = data_.network_state;
   data_.network_state = state;
-  AdvanceLoadingAnimation(old_state, state);
+  AdvanceLoadingAnimation(state);
 }
 
 void Tab::StartPulse() {
@@ -787,6 +865,7 @@ void Tab::Layout() {
     favicon_bounds_.set_y(lb.y() + (lb.height() - gfx::kFaviconSize + 1) / 2);
     MaybeAdjustLeftForPinnedTab(&favicon_bounds_);
   }
+  throbber_->SetBoundsRect(favicon_bounds_);
 
   showing_close_button_ = ShouldShowCloseBox();
   if (showing_close_button_) {
@@ -1347,28 +1426,7 @@ void Tab::PaintIcon(gfx::Canvas* canvas) {
   bounds.set_x(GetMirroredXForRect(bounds));
 
   if (data().network_state != TabRendererData::NETWORK_STATE_NONE) {
-    // Paint network activity (aka throbber) animation frame.
-    ui::ThemeProvider* tp = GetThemeProvider();
-    if (data().network_state == TabRendererData::NETWORK_STATE_WAITING) {
-      if (waiting_start_time_ == base::TimeTicks())
-        waiting_start_time_ = base::TimeTicks::Now();
-
-      waiting_state_.elapsed_time =
-          base::TimeTicks::Now() - waiting_start_time_;
-      gfx::PaintThrobberWaiting(
-          canvas, bounds, tp->GetColor(ThemeProperties::COLOR_THROBBER_WAITING),
-          waiting_state_.elapsed_time);
-    } else {
-      if (loading_start_time_ == base::TimeTicks())
-        loading_start_time_ = base::TimeTicks::Now();
-
-      waiting_state_.color =
-          tp->GetColor(ThemeProperties::COLOR_THROBBER_WAITING);
-      gfx::PaintThrobberSpinningAfterWaiting(
-          canvas, bounds,
-          tp->GetColor(ThemeProperties::COLOR_THROBBER_SPINNING),
-          base::TimeTicks::Now() - loading_start_time_, &waiting_state_);
-    }
+    // Throbber will do its own painting.
   } else if (should_display_crashed_favicon_) {
     // Paint crash favicon.
     ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
@@ -1387,27 +1445,39 @@ void Tab::PaintIcon(gfx::Canvas* canvas) {
   }
 }
 
-void Tab::AdvanceLoadingAnimation(TabRendererData::NetworkState old_state,
-                                  TabRendererData::NetworkState state) {
-  if (state == TabRendererData::NETWORK_STATE_WAITING) {
-    // Waiting steps backwards.
-    immersive_loading_step_ =
-        (immersive_loading_step_ - 1 + kImmersiveLoadingStepCount) %
-            kImmersiveLoadingStepCount;
-  } else if (state == TabRendererData::NETWORK_STATE_LOADING) {
-    immersive_loading_step_ = (immersive_loading_step_ + 1) %
-        kImmersiveLoadingStepCount;
-  } else {
-    waiting_start_time_ = base::TimeTicks();
-    loading_start_time_ = base::TimeTicks();
-    waiting_state_ = gfx::ThrobberWaitingState();
-    immersive_loading_step_ = 0;
-  }
+void Tab::AdvanceLoadingAnimation(TabRendererData::NetworkState state) {
   if (controller_->IsImmersiveStyle()) {
+    if (state == TabRendererData::NETWORK_STATE_WAITING) {
+      // Waiting steps backwards.
+      immersive_loading_step_ =
+          (immersive_loading_step_ - 1 + kImmersiveLoadingStepCount) %
+          kImmersiveLoadingStepCount;
+    } else if (state == TabRendererData::NETWORK_STATE_LOADING) {
+      immersive_loading_step_ =
+          (immersive_loading_step_ + 1) % kImmersiveLoadingStepCount;
+    } else {
+      immersive_loading_step_ = 0;
+    }
+
     SchedulePaintInRect(GetImmersiveBarRect());
-  } else {
-    ScheduleIconPaint();
+    return;
+  }
+
+  // To ensure the throbber layer does not paint over stacked or dragged tabs,
+  // hide it when the favicon would be clipped out. Check for any clipping on
+  // the left since the shoulder of a dragged tab can still paint over the icon.
+  gfx::Rect clip = GetLocalBounds();
+  const bool needs_throbber = state != TabRendererData::NETWORK_STATE_NONE &&
+                              controller_->ShouldPaintTab(this, &clip) &&
+                              clip.x() == 0 && clip.Contains(favicon_bounds_);

[sky, 2015/10/09 15:16:38]

I don't think this is enough. I can still easily get bleed through if a tab is
loading and I drag another tab on top of it.

[tapted, 2015/10/11 10:36:38]

Thanks for trying it! I added handling for two additional cases. The first I
didn't notice on a desktop-linux build, but on a ChromeOS build it showed in
some tab reorder animations. The fix is in TabStrip::ShouldPaintTab(..) - there
are two cases for when the tab being dragged overlaps the right side of a tab
animating to the left: the clip rect wasn't updated for these, now it is.

The other case was when a tab first attaches to a tab strip from another window.
In this case, the right-side of a throbber can get the dragged tab over it
(usually the tab reorders to the right before that can happen, but not when it
happens in TabDragController::Attach). So, that check replaces the
clip.Contains(..) here.

I need to stare at the logic in TabStrip::ShouldPaintTab some more -- I think
the clipping needs to be different for non-stacked tabs now that it updates the
clip rect for both types.

But let me know if you think this approach seems sound, and I'll fix that up and
work on some tests.

+
+  if (needs_throbber != throbber_->visible()) {
+    throbber_->SetVisible(needs_throbber);
+    ScheduleIconPaint();  // Repaint the icon area to update favicon visibility.
   }
+  if (state == TabRendererData::NETWORK_STATE_NONE)
+    throbber_->ResetStartTimes();
+  throbber_->SchedulePaint();
 }
 
 int Tab::IconCapacity() const {