[stacked tabs] Fix stacking logic with 'open link in new tab'

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

Index: chrome/browser/ui/views/tabs/stacked_tab_strip_layout.cc
diff --git a/chrome/browser/ui/views/tabs/stacked_tab_strip_layout.cc b/chrome/browser/ui/views/tabs/stacked_tab_strip_layout.cc
index aaf637e65afff7838d62365a17506c8b26fcd600..1524cdbcc61ac0229d157859604476837e702ac0 100644
--- a/chrome/browser/ui/views/tabs/stacked_tab_strip_layout.cc
+++ b/chrome/browser/ui/views/tabs/stacked_tab_strip_layout.cc
@@ -171,8 +171,13 @@ void StackedTabStripLayout::AddTab(int index, int add_types, int start_x) {
     ResetToIdealState();
     return;
   }
-  int active_x = (index + 1 == tab_count()) ?
+
+  int active_x = ideal_x(active_index());
+  if (add_types & kAddTypeActive) {
+    active_x = (index + 1 == tab_count()) ?
       width_ - size_.width() : ideal_x(index + 1);
+  }
+
   SetIdealBoundsAt(active_index(), ConstrainActiveX(active_x));
   LayoutByTabOffsetAfter(active_index());
   LayoutByTabOffsetBefore(active_index());
@@ -336,24 +341,27 @@ void StackedTabStripLayout::MakeVisible(int index) {
   if (index <= active_index() || !requires_stacking() || !IsStacked(index))
     return;
 
-  int ideal_delta = width_for_count(index - active_index()) - overlap_;
+  const int ideal_delta = width_for_count(index - active_index()) - overlap_;
   if (ideal_x(index) - ideal_x(active_index()) == ideal_delta)
     return;
 
-  // First push active index as far to the left as it'll go.
-  int active_x = std::max(GetMinX(active_index()),
-                          std::min(ideal_x(index) - ideal_delta,
-                                   ideal_x(active_index())));
+  // Move the active tab to the left so that all tabs between the active tab
+  // and |index| (inclusive) can be made visible.
+  const int active_x = std::max(GetMinX(active_index()),
+                                std::min(ideal_x(index) - ideal_delta,
+                                         ideal_x(active_index())));
   SetIdealBoundsAt(active_index(), active_x);
-  LayoutUsingCurrentBefore(active_index());
-  LayoutUsingCurrentAfter(active_index());
+  LayoutByTabOffsetBefore(active_index());
+  LayoutByTabOffsetAfter(active_index());

[Peter Kasting, 2017/06/28 22:04:33]

This now leaves only one caller of LayoutUsingCurrentXXX(), which makes me
suspicious about whether that caller really wants to be updated similarly.

[tdanderson, 2017/06/29 16:01:04]

There are actually still two: SetActiveBoundsAndLayoutFromActiveTab() via
RemoveTab(), and line 371. I am going to ask UX if I can change the logic used
in the former (eliminating the call site), and I suspect the latter is a no-op
but had planned to look at both at the same time.

   AdjustStackedTabs();
+
   if (ideal_x(index) - ideal_x(active_index()) == ideal_delta)
     return;
 
   // If we get here active_index() is left aligned. Push |index| as far to
-  // the right as possible.
-  int x = std::min(GetMaxX(index), active_x + ideal_delta);
+  // the right as possible, forming a stack immediately to the right of the
+  // active tab if necessary.
+  const int x = std::min(GetMaxX(index), active_x + ideal_delta);
   SetIdealBoundsAt(index, x);
   LayoutByTabOffsetAfter(index);
   for (int next_x = x, i = index - 1; i > active_index(); --i) {