Move logic in TabStrip::PrepareForCloseAt() to after tab removal |
|||
Issue descriptionPrepareForCloseAt() tries to adjust the width of the strip based on which tab was being closed. To do this accurately, it needs to know the next tab to be activated (see discussion on https://chromium-review.googlesource.com/c/chromium/src/+/1111763 ). However, that isn't yet known. sky@ proposed this refactoring to fix: "1. Remove TabStrip::PrepareForCloseAt(). 2. BrowserTabStripController::CloseTab() would gain a member to indicate it's closing a tab. 3. BrowserTabStripController::TabDetachedAt() would gain logic to recognize the detach is the result of the call from CloseTab and provide additional parameters to TabStrip::RemoveTabAt(). 4. TabStrip::RemoveTabAt() would get the logic here. As the selection model in the TabStripModel is updated, this logic could then definitively know the new selection. "I would not be surprised if there is some trickiness around timing with this change."
,
Jun 27 2018
,
Jul 4
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/362c80256baecfed86dfa88b758b4bf8a7a04f80 commit 362c80256baecfed86dfa88b758b4bf8a7a04f80 Author: sangwoo.ko <sangwoo108@chromium.org> Date: Wed Jul 04 14:45:50 2018 Remove TabStrip::PrepareForCloseAt() When removing tabs with mouse button, we'd like to know the next active tab so that we can tailor available_width_for_tabs_. But as PrepareForCloseAt() is called before the next tab is decided, we don't know that. So move this logic to RemoveTabAt() where we know it. And move other logic in PrepareCloseTabAt() is moved to CloseTab(), which triggered PrepareForCloseAt() via TabStripController::CloseTab(). As TabStripController::CloseTab() was invoked only by TabStrip::CloseTab(), it's okay to move them to TabStrip::CloseTab(). Bug: 856289 Change-Id: I5eb7036da4214e92a218f20e8c09891ab9a11955 Reviewed-on: https://chromium-review.googlesource.com/1119616 Commit-Queue: Sang Woo Ko <sangwoo108@chromium.org> Reviewed-by: Peter Kasting <pkasting@chromium.org> Cr-Commit-Position: refs/heads/master@{#572566} [modify] https://crrev.com/362c80256baecfed86dfa88b758b4bf8a7a04f80/chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc [modify] https://crrev.com/362c80256baecfed86dfa88b758b4bf8a7a04f80/chrome/browser/ui/views/tabs/fake_base_tab_strip_controller.cc [modify] https://crrev.com/362c80256baecfed86dfa88b758b4bf8a7a04f80/chrome/browser/ui/views/tabs/tab_strip.cc [modify] https://crrev.com/362c80256baecfed86dfa88b758b4bf8a7a04f80/chrome/browser/ui/views/tabs/tab_strip.h [modify] https://crrev.com/362c80256baecfed86dfa88b758b4bf8a7a04f80/chrome/browser/ui/views/tabs/tab_strip_unittest.cc
,
Jul 4
|
|||
►
Sign in to add a comment |
|||
Comment 1 by sangwoo108@chromium.org
, Jun 26 2018