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

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

Currently when in stacked tab mode, using 'open link in
new tab' can result in a stack forming to the left of
the active tab. Prevent this from happening, and also
do not adjust the position of the active tab to the left
unless necessary in order to show all tabs between the
active tab and the newly-opened tab (inclusive).

BUG=737639
TEST=StackedTabStripLayoutTest.AddTab

Review-Url: https://codereview.chromium.org/2963023002
Cr-Commit-Position: refs/heads/master@{#483375}
Committed: https://chromium.googlesource.com/chromium/src/+/7aa4e8bf36efda4db554567ffc5b13f14540601e

[tdanderson, 2017-06-28 21:38:09]

The CQ bit was checked by tdanderson@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-28 21:38:33]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tdanderson, 2017-06-28 21:54:01]

tdanderson@chromium.org changed reviewers:
+ pkasting@chromium.org

[tdanderson, 2017-06-28 21:54:02]

Peter, can you please take a look?

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

LGTM

https://codereview.chromium.org/2963023002/diff/1/chrome/browser/ui/views/tab...
File chrome/browser/ui/views/tabs/stacked_tab_strip_layout.cc (right):

https://codereview.chromium.org/2963023002/diff/1/chrome/browser/ui/views/tab...
chrome/browser/ui/views/tabs/stacked_tab_strip_layout.cc:355:
LayoutByTabOffsetAfter(active_index());
This now leaves only one caller of LayoutUsingCurrentXXX(), which makes me
suspicious about whether that caller really wants to be updated similarly.

[commit-bot: I haz the power, 2017-06-28 23:03:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-28 23:03:46]

Dry run: This issue passed the CQ dry run.

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

https://codereview.chromium.org/2963023002/diff/1/chrome/browser/ui/views/tab...
File chrome/browser/ui/views/tabs/stacked_tab_strip_layout.cc (right):

https://codereview.chromium.org/2963023002/diff/1/chrome/browser/ui/views/tab...
chrome/browser/ui/views/tabs/stacked_tab_strip_layout.cc:355:
LayoutByTabOffsetAfter(active_index());
On 2017/06/28 22:04:33, Peter Kasting wrote:
> This now leaves only one caller of LayoutUsingCurrentXXX(), which makes me
> suspicious about whether that caller really wants to be updated similarly.

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.

[tdanderson, 2017-06-29 16:01:10]

The CQ bit was checked by tdanderson@chromium.org

[commit-bot: I haz the power, 2017-06-29 16:01:29]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-29 16:05:18]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1498752070925940, "parent_rev":
"bbf85c4795b79cbe3285167e3a0d18127ef747a0", "commit_rev":
"7aa4e8bf36efda4db554567ffc5b13f14540601e"}

[commit-bot: I haz the power, 2017-06-29 16:05:37]

Description was changed from

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

Currently when in stacked tab mode, using 'open link in
new tab' can result in a stack forming to the left of
the active tab. Prevent this from happening, and also
do not adjust the position of the active tab to the left
unless necessary in order to show all tabs between the
active tab and the newly-opened tab (inclusive).

BUG=737639
TEST=StackedTabStripLayoutTest.AddTab
==========

to

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

Currently when in stacked tab mode, using 'open link in
new tab' can result in a stack forming to the left of
the active tab. Prevent this from happening, and also
do not adjust the position of the active tab to the left
unless necessary in order to show all tabs between the
active tab and the newly-opened tab (inclusive).

BUG=737639
TEST=StackedTabStripLayoutTest.AddTab

Review-Url: https://codereview.chromium.org/2963023002
Cr-Commit-Position: refs/heads/master@{#483375}
Committed:
https://chromium.googlesource.com/chromium/src/+/7aa4e8bf36efda4db554567ffc5b...
==========

[commit-bot: I haz the power, 2017-06-29 16:05:39]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/7aa4e8bf36efda4db554567ffc5b...