Commit eeeb3fd8 authored by Taylor Bergquist's avatar Taylor Bergquist Committed by Commit Bot

Fix tab visibility when the last inactive tab is in a collapsed group.

Tab widths are cached during UpdateIdealBounds for subsequent calls
to GetActiveWidth and GetInactiveWidth. If the last inactive tab in
any invocation of UpdateIdealBounds() is in a collapsed group, its
width of kTabOverlap will end up being recorded as the inactive tab
width. This most prominently manifested as a bug in calculating tab
visibility.

Bug: 1140388
Change-Id: Ie9ecdbacf49ffdb829f1ff208d4f22e077457b94
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2542769
Commit-Queue: Taylor Bergquist <tbergquist@chromium.org>
Reviewed-by: default avatarElly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#828414}
parent cce107ca
...@@ -313,17 +313,8 @@ std::vector<gfx::Rect> TabStripLayoutHelper::CalculateIdealBounds( ...@@ -313,17 +313,8 @@ std::vector<gfx::Rect> TabStripLayoutHelper::CalculateIdealBounds(
auto pinned = i <= last_pinned_tab_slot_index ? TabPinned::kPinned auto pinned = i <= last_pinned_tab_slot_index ? TabPinned::kPinned
: TabPinned::kUnpinned; : TabPinned::kUnpinned;
// The slot can only be collapsed if it is a tab and in a collapsed group.
// If the slot is indeed a tab and in a group, check the collapsed state of
// the group to determine if it is collapsed.
// A collapsed tab animates close like a closed tab. // A collapsed tab animates close like a closed tab.
base::Optional<tab_groups::TabGroupId> id = slots_[i].view->group(); auto open = (slots_[i].animation->IsClosing() || SlotIsCollapsedTab(i))
bool slot_is_collapsed_tab =
(slots_[i].type == ViewType::kTab && id.has_value())
? controller_->IsGroupCollapsed(id.value())
: false;
auto open = (slots_[i].animation->IsClosing() || slot_is_collapsed_tab)
? TabOpen::kClosed ? TabOpen::kClosed
: TabOpen::kOpen; : TabOpen::kOpen;
TabAnimationState ideal_animation_state = TabAnimationState ideal_animation_state =
...@@ -448,6 +439,10 @@ TabStripLayoutHelper::GetCurrentTabWidthConstraints() const { ...@@ -448,6 +439,10 @@ TabStripLayoutHelper::GetCurrentTabWidthConstraints() const {
void TabStripLayoutHelper::UpdateCachedTabWidth(int tab_index, void TabStripLayoutHelper::UpdateCachedTabWidth(int tab_index,
int tab_width, int tab_width,
bool active) { bool active) {
// If the slot is collapsed, its width should never be reported as the
// current active or inactive tab width - it's not even visible.
if (SlotIsCollapsedTab(tab_index))
return;
if (active) if (active)
active_tab_width_ = tab_width; active_tab_width_ = tab_width;
else else
...@@ -458,3 +453,13 @@ bool TabStripLayoutHelper::WidthsConstrainedForClosingMode() { ...@@ -458,3 +453,13 @@ bool TabStripLayoutHelper::WidthsConstrainedForClosingMode() {
return tab_width_override_.has_value() || return tab_width_override_.has_value() ||
tabstrip_width_override_.has_value(); tabstrip_width_override_.has_value();
} }
bool TabStripLayoutHelper::SlotIsCollapsedTab(int i) const {
// The slot can only be collapsed if it is a tab and in a collapsed group.
// If the slot is indeed a tab and in a group, check the collapsed state of
// the group to determine if it is collapsed.
const base::Optional<tab_groups::TabGroupId> id = slots_[i].view->group();
return (slots_[i].type == ViewType::kTab && id.has_value())
? controller_->IsGroupCollapsed(id.value())
: false;
}
...@@ -171,6 +171,9 @@ class TabStripLayoutHelper { ...@@ -171,6 +171,9 @@ class TabStripLayoutHelper {
// Returns true if any width constraint is currently being enforced. // Returns true if any width constraint is currently being enforced.
bool WidthsConstrainedForClosingMode(); bool WidthsConstrainedForClosingMode();
// True iff the slot at index |i| is a tab that is in a collapsed group.
bool SlotIsCollapsedTab(int i) const;
// The owning tabstrip's controller. // The owning tabstrip's controller.
const TabStripController* const controller_; const TabStripController* const controller_;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment