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

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

 // Copyright (c) 2012 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/views/tabs/stacked_tab_strip_layout.h"
 
 #include <stdio.h>
 
 #include "base/logging.h"
 #include "base/metrics/user_metrics.h"
@@ skipping 153 matching lines @@
     active_index_ = index;
   else if (active_index_ >= index)
     active_index_++;
   if (add_types & kAddTypePinned)
     pinned_tab_count_++;
   x_ = start_x;
   if (!requires_stacking() || normal_tab_count() <= 1) {
     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());
   AdjustStackedTabs();
 
   if ((add_types & kAddTypeActive) == 0)
     MakeVisible(index);
 }
 
 void StackedTabStripLayout::RemoveTab(int index, int start_x, int old_x) {
@@ skipping 143 matching lines @@
              ideal_x(tab_count() - 1) != width_ - size_.width());
   }
   AdjustStackedTabs();
 }
 
 void StackedTabStripLayout::MakeVisible(int index) {
   // Currently no need to support tabs opening before |index| visible.
   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) {
     next_x = std::max(GetMinXCompressed(i), next_x - tab_offset());
     SetIdealBoundsAt(i, next_x);
   }
   LayoutUsingCurrentAfter(active_index());
   AdjustStackedTabs();
 }
 
@@ skipping 210 matching lines @@
   return width_ - trailing_offset - size_.width();
 }
 
 int StackedTabStripLayout::GetMinXCompressed(int index) const {
   DCHECK_GT(index, active_index());
   return std::max(
       width_ - width_for_count(tab_count() - index),
       ideal_x(active_index()) +
           stacked_padding_for_count(index - active_index()));
 }