Remove layout during paint in Tab

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

Index: chrome/browser/ui/views/tabs/tab_strip.cc
diff --git a/chrome/browser/ui/views/tabs/tab_strip.cc b/chrome/browser/ui/views/tabs/tab_strip.cc
index 151d264d72d7cd81fb921b09fc78001886907fbe..88356d54d2ef74821ef69b4d39c942f6e0af287d 100644
--- a/chrome/browser/ui/views/tabs/tab_strip.cc
+++ b/chrome/browser/ui/views/tabs/tab_strip.cc
@@ -728,6 +728,9 @@ void TabStrip::SetStackedLayout(bool stacked_layout) {
         active_center - ideal_bounds(active_index).width() / 2);
     AnimateToIdealBounds();
   }
+
+  for (int i = 0; i < tab_count(); ++i)
+    tab_at(i)->Layout();

[sky, 2016/01/12 22:29:11]

Can you elaborate as to why this is necessary (and add a comment too). In theory
if we do something interesting here, then SwapLayoutIfNecessary will trigger an
animation that will cause each tabs bounds to change and trigger a layout.

[enne (OOO), 2016/01/12 22:36:28]

Changing stacked layout can change whether or not buttons are visible. 
Animations seem somewhat of an implementation detail, but I don't know this
code.  Should the test just call layout, similar to how it was calling paint
before?  I'm not sure that makes the test very valuable, but maybe that's the
best approach.

[sky, 2016/01/12 22:41:02]

Yes, because the animation triggers layout.

 }
 
 gfx::Rect TabStrip::GetNewTabButtonBounds() {
@@ -1143,9 +1146,17 @@ bool TabStrip::ShouldHideCloseButtonForInactiveTabs() {
 }
 
 void TabStrip::SelectTab(Tab* tab) {
+  int old_active = controller_->GetActiveIndex();

[sky, 2016/01/12 22:29:11]

How come TabStrip::SetSelection doesn't cover this?

[enne (OOO), 2016/01/12 22:36:29]

SelectTab and SetSelection are both public functions that do not call each
other.  The test calls SelectTab, and so SetSelection is not relevant here.

[sky, 2016/01/12 22:41:02]

SelectTab calls to the TabStripController, which should result in a call back to
SetSelection. Are you seeing otherwise? If it's from a test, then the test may
not be calling SetSelection as real code does.

[enne (OOO), 2016/01/12 22:54:54]

It does not appear to call SetSelection from the test.

[sky, 2016/01/13 00:41:09]

Please update the caller to do that then as that is the expectation of
Controller::SelectTab.

+
   int model_index = GetModelIndexOfTab(tab);
   if (IsValidModelIndex(model_index))
     controller_->SelectTab(model_index);
+
+  int new_active = controller_->GetActiveIndex();
+  if (new_active != old_active) {
+    tab_at(old_active)->ActiveStateChanged();
+    tab_at(new_active)->ActiveStateChanged();
+  }
 }
 
 void TabStrip::ExtendSelectionTo(Tab* tab) {