Fix the attention indicator.

It wasn't showing up for dialogs on background tabs (broken in
r520759). This fixes that and adds a test.

BUG=860185
Signed-off-by: Avi Drissman <avi@chromium.org>
Change-Id: I70ddec77e987dff727a5189e810aad5c3fc589de
Reviewed-on: https://chromium-review.googlesource.com/1147705Reviewed-by: Robert Liao <robliao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577966}

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

@@ -1033,10 +1033,7 @@ bool Tab::IsActive() const {
 }
 
 void Tab::ActiveStateChanged() {
-  if (IsActive()) {
-    // Clear the blocked WebContents for active tabs because it's distracting.
-    icon_->SetAttention(TabIcon::AttentionType::kBlockedWebContents, false);
-  }
+  UpdateTabIconNeedsAttentionBlocked();
   OnButtonColorMaybeChanged();
   alert_indicator_button_->UpdateEnabledForMuteToggle();
   Layout();
@@ -1070,14 +1067,7 @@ void Tab::SetData(TabRendererData data) {
   icon_->SetNetworkState(data_.network_state, data_.should_hide_throbber);
   icon_->SetCanPaintToLayer(controller_->CanPaintThrobberToLayer());
   icon_->SetIsCrashed(data_.IsCrashed());
-  if (IsActive()) {
-    icon_->SetAttention(TabIcon::AttentionType::kBlockedWebContents, false);
-  } else {
-    // Only non-active WebContents get the blocked attention type because it's
-    // confusing on the active tab.
-    icon_->SetAttention(TabIcon::AttentionType::kBlockedWebContents,
-                        data_.blocked);
-  }
+  UpdateTabIconNeedsAttentionBlocked();
 
   base::string16 title = data_.title;
   if (title.empty()) {
@@ -1699,5 +1689,17 @@ void Tab::OnButtonColorMaybeChanged() {
   close_button_->SetIconColors(icon_color);
 }
 
+void Tab::UpdateTabIconNeedsAttentionBlocked() {
+  // Only show the blocked attention indicator on non-active tabs. For active
+  // tabs, the user sees the dialog blocking the tab, so there's no point to it
+  // and it would be distracting.
+  if (IsActive()) {
+    icon_->SetAttention(TabIcon::AttentionType::kBlockedWebContents, false);
+  } else {
+    icon_->SetAttention(TabIcon::AttentionType::kBlockedWebContents,
+                        data_.blocked);
+  }
+}
+
 Tab::BackgroundCache::BackgroundCache() = default;
 Tab::BackgroundCache::~BackgroundCache() = default;

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

@@ -301,6 +301,10 @@ class Tab : public gfx::AnimationDelegate,
   // time the theme or active state may have changed.
   void OnButtonColorMaybeChanged();
 
+  // Updates the blocked attention state of the |icon_|. This only updates
+  // state; it is the responsibility of the caller to request a paint.
+  void UpdateTabIconNeedsAttentionBlocked();
+
   // The controller, never NULL.
   TabController* const controller_;
 

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

@@ -204,7 +204,7 @@ void TabIcon::OnPaint(gfx::Canvas* canvas) {
       icon_to_paint = &themed_favicon_;
   }
 
-  if (attention_types_ != 0 && !should_display_crashed_favicon_) {
+  if (ShowingAttentionIndicator() && !should_display_crashed_favicon_) {
     PaintAttentionIndicatorAndIcon(canvas, *icon_to_paint, icon_bounds);
   } else if (!icon_to_paint->isNull()) {
     canvas->DrawImageInt(*icon_to_paint, 0, 0, icon_bounds.width(),

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

@@ -195,8 +195,8 @@ class TabStripTest : public ChromeViewsTestBase,
     return tab->showing_close_button_;
   }
 
-  bool IsShowingAttentionIndicator(int model_index) {
-    return tab_strip_->tab_at(model_index)->icon_->ShowingAttentionIndicator();
+  bool IsShowingAttentionIndicator(Tab* tab) {
+    return tab->icon_->ShowingAttentionIndicator();
   }
 
   // Checks whether |tab| contains |point_in_tabstrip_coords|, where the point
@@ -819,6 +819,41 @@ TEST_P(TabStripTest, ResetBoundsForDraggedTabs) {
   EXPECT_LT(dragged_tab->bounds().width(), min_active_width);
 }
 
+// The "blocked" attention indicator should only show for background tabs.
+TEST_P(TabStripTest, TabNeedsAttentionBlocked) {
+  controller_->AddTab(0, false);
+  controller_->AddTab(1, true);
+
+  Tab* tab1 = tab_strip_->tab_at(1);
+
+  // Block tab1.
+  TabRendererData data;
+  data.blocked = true;
+  tab1->SetData(data);
+
+  EXPECT_FALSE(IsShowingAttentionIndicator(tab1));
+  controller_->SelectTab(0);
+  EXPECT_TRUE(IsShowingAttentionIndicator(tab1));
+  controller_->SelectTab(1);
+  EXPECT_FALSE(IsShowingAttentionIndicator(tab1));
+}
+
+// The generic "wants attention" version should always show.
+TEST_P(TabStripTest, TabNeedsAttentionGeneric) {
+  controller_->AddTab(0, false);
+  controller_->AddTab(1, true);
+
+  Tab* tab1 = tab_strip_->tab_at(1);
+
+  tab1->SetTabNeedsAttention(true);
+
+  EXPECT_TRUE(IsShowingAttentionIndicator(tab1));
+  controller_->SelectTab(0);
+  EXPECT_TRUE(IsShowingAttentionIndicator(tab1));
+  controller_->SelectTab(1);
+  EXPECT_TRUE(IsShowingAttentionIndicator(tab1));
+}
+
 // Defines an alias to be used for tests that are only relevant to the touch-
 // optimized UI mode.
 using TabStripTouchOptimizedUiOnlyTest = TabStripTest;