Skip to content

Commit

Permalink
Refocus on previously selected tab for GTS
Browse files Browse the repository at this point in the history
This CL does the following:
1. Fixes the issue of GTS hiding when undoing a single and all tab closure action.
2. Adds functionality of refocusing on previously selected tab when tab closure is undone from GTS.
3. Refactors the existing refocus code by extracting the logic specific to it into its own helper file.
4. Refocus on previously selected tab when undoing a single and all tab closure action on mobile.

Demo:
Phone: https://drive.google.com/file/d/13cn1WD07yhHNlqF7LNcNQKIU8aJUQql0/view?usp=sharing
Tablet: https://drive.google.com/file/d/1CRaeNfpAkglxI_yfVxVUzPH1Atv3jDmN/view?usp=sharing

Bug: 1316413
Change-Id: I7a6c7da8c9d4bc081d32d5cf9ee8dacf0aac2c10
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3594345
Reviewed-by: Theresa Sullivan <twellington@chromium.org>
Commit-Queue: Gaurav Jayasawal <gauravjj@google.com>
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Reviewed-by: Sirisha Kavuluru <skavuluru@google.com>
Cr-Commit-Position: refs/heads/main@{#1002117}
  • Loading branch information
Gaurav Jayasawal authored and Chromium LUCI CQ committed May 11, 2022
1 parent f61ad36 commit d0e0946
Show file tree
Hide file tree
Showing 26 changed files with 770 additions and 368 deletions.
1 change: 1 addition & 0 deletions chrome/android/chrome_java_sources.gni
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ chrome_java_sources = [
"java/src/org/chromium/chrome/browser/SynchronousInitializationActivity.java",
"java/src/org/chromium/chrome/browser/TabUsageTracker.java",
"java/src/org/chromium/chrome/browser/TabbedModeTabDelegateFactory.java",
"java/src/org/chromium/chrome/browser/UndoRefocusHelper.java",
"java/src/org/chromium/chrome/browser/WarmupManager.java",
"java/src/org/chromium/chrome/browser/WebContentsFactory.java",
"java/src/org/chromium/chrome/browser/ZoomController.java",
Expand Down
2 changes: 2 additions & 0 deletions chrome/android/chrome_junit_test_java_sources.gni
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ chrome_junit_test_java_sources = [
"junit/src/org/chromium/chrome/browser/PowerBroadcastReceiverTest.java",
"junit/src/org/chromium/chrome/browser/ShadowIdleHandlerAwareMessageQueue.java",
"junit/src/org/chromium/chrome/browser/ShortcutHelperTest.java",
"junit/src/org/chromium/chrome/browser/UndoRefocusHelperTest.java",
"junit/src/org/chromium/chrome/browser/about_settings/AboutSettingsBridgeTest.java",
"junit/src/org/chromium/chrome/browser/app/appmenu/AppMenuPropertiesDelegateUnitTest.java",
"junit/src/org/chromium/chrome/browser/app/download/home/FileDeletionQueueTest.java",
Expand Down Expand Up @@ -65,6 +66,7 @@ chrome_junit_test_java_sources = [
"junit/src/org/chromium/chrome/browser/compositor/layouts/StaticLayoutUnitTest.java",
"junit/src/org/chromium/chrome/browser/compositor/overlays/strip/StripLayoutHelperTest.java",
"junit/src/org/chromium/chrome/browser/compositor/overlays/strip/TabUsageTrackerTest.java",
"junit/src/org/chromium/chrome/browser/compositor/overlays/strip/TestTabModel.java",
"junit/src/org/chromium/chrome/browser/content_capture/ContentCaptureHistoryDeletionObserverTest.java",
"junit/src/org/chromium/chrome/browser/contextmenu/ContextMenuCoordinatorTest.java",
"junit/src/org/chromium/chrome/browser/contextmenu/ContextMenuHeaderMediatorTest.java",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,8 @@ public void didSelectTab(Tab tab, int type, int lastId) {

mSelectedTabDidNotChangedAfterShown = false;
updateSelectedTab(tab);
if (type == TabSelectionType.FROM_CLOSE || mShouldIgnoreNextSelect) {
if (type == TabSelectionType.FROM_CLOSE || type == TabSelectionType.FROM_UNDO
|| mShouldIgnoreNextSelect) {
mShouldIgnoreNextSelect = false;
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,8 @@ public void didSelectTab(Tab tab, int type, int lastId) {
if (!mTabModelSelector.isTabStateInitialized()) {
return;
}
if (type == TabSelectionType.FROM_CLOSE || mShouldIgnoreNextSelect) {
if (type == TabSelectionType.FROM_CLOSE || mShouldIgnoreNextSelect
|| type == TabSelectionType.FROM_UNDO) {
mShouldIgnoreNextSelect = false;
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,21 @@ public void doesNotHideWhenSelectedTabChangedDueToTabClosure() {
verify(mLayout).onTabSelecting(anyLong(), eq(TAB1_ID));
}

@Test
public void doesNotHideWhenSelectedTabChangedDueToUndoTabClosure() {
doReturn(true).when(mTabModelSelector).isTabStateInitialized();
initAndAssertAllProperties();
mMediator.showOverview(true);
assertThat(mModel.get(TabListContainerProperties.IS_VISIBLE), equalTo(true));

doReturn(true).when(mTab3).isClosing();
mTabModelObserverCaptor.getValue().didSelectTab(mTab1, TabSelectionType.FROM_UNDO, TAB3_ID);
verify(mLayout, never()).onTabSelecting(anyLong(), anyInt());

mTabModelObserverCaptor.getValue().didSelectTab(mTab1, TabSelectionType.FROM_USER, TAB3_ID);
verify(mLayout).onTabSelecting(anyLong(), eq(TAB1_ID));
}

@Test
public void doesNotHideWhenSelectedTabChangedDueToModelChange() {
doReturn(true).when(mTabModelSelector).isTabStateInitialized();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1747,6 +1747,7 @@ public void performPostInflationStartup() {
mInactivityTracker = new ChromeInactivityTracker(
ChromePreferenceKeys.TABBED_ACTIVITY_LAST_BACKGROUNDED_TIME_MS_PREF);
TabUsageTracker.initialize(this.getLifecycleDispatcher(), tabModelSelector);
UndoRefocusHelper.initialize(tabModelSelector, getLayoutManagerSupplier(), isTablet());

assert getActivityTabStartupMetricsTracker() != null;
boolean shouldShowOverviewPageOnStart = shouldShowOverviewPageOnStart();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,246 @@
// Copyright 2022 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.

package org.chromium.chrome.browser;

import androidx.annotation.VisibleForTesting;

import org.chromium.base.Callback;
import org.chromium.base.metrics.RecordUserAction;
import org.chromium.base.supplier.ObservableSupplier;
import org.chromium.chrome.browser.compositor.layouts.LayoutManagerImpl;
import org.chromium.chrome.browser.flags.CachedFeatureFlags;
import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.layouts.LayoutStateProvider;
import org.chromium.chrome.browser.layouts.LayoutType;
import org.chromium.chrome.browser.lifecycle.DestroyObserver;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tab.TabSelectionType;
import org.chromium.chrome.browser.tabmodel.TabModel;
import org.chromium.chrome.browser.tabmodel.TabModelSelector;
import org.chromium.chrome.browser.tabmodel.TabModelSelectorTabModelObserver;
import org.chromium.chrome.browser.tabmodel.TabModelUtils;

import java.util.HashSet;
import java.util.Set;

/**
* Refocus on previously selected tab if the selected tab closure was undone.
*/
public class UndoRefocusHelper implements DestroyObserver {
private final Set<Integer> mTabsClosedFromTabStrip;
private final TabModelSelector mModelSelector;
private final ObservableSupplier<LayoutManagerImpl> mLayoutManagerObservableSupplier;

private LayoutManagerImpl mLayoutManager;
private LayoutStateProvider.LayoutStateObserver mLayoutStateObserver;
private TabModelSelectorTabModelObserver mTabModelSelectorTabModelObserver;
private Integer mSelectedTabIdWhenTabClosed;
private Boolean mClosedAllTabs;
private boolean mTabSwitcherActive;
private Callback<LayoutManagerImpl> mLayoutManagerSupplierCallback;
private boolean mIsTablet;

/**
* This method is used to create and initialize the UndoRefocusHelper.
* @param modelSelector TabModelSelector used to subscribe to TabModelSelectorTabModelObserver
* to capture when tabs are being closed or the closure is being undone.
* @param layoutManagerObservableSupplier This supplies the LayoutManager implementation to
* observe the layout state when it's available.
* @param isTablet Whether the current device is a tablet.
*/
public static void initialize(TabModelSelector modelSelector,
ObservableSupplier<LayoutManagerImpl> layoutManagerObservableSupplier,
boolean isTablet) {
if (!CachedFeatureFlags.isEnabled(ChromeFeatureList.TAB_STRIP_IMPROVEMENTS)) return;

new UndoRefocusHelper(modelSelector, layoutManagerObservableSupplier, isTablet);
}

@VisibleForTesting
protected UndoRefocusHelper(TabModelSelector modelSelector,
ObservableSupplier<LayoutManagerImpl> layoutManagerObservableSupplier,
boolean isTablet) {
mLayoutManagerObservableSupplier = layoutManagerObservableSupplier;
mModelSelector = modelSelector;
mTabsClosedFromTabStrip = new HashSet<>();
mTabSwitcherActive = false;
mIsTablet = isTablet;

observeTabModel();
observeLayoutState();
}

@Override
public void onDestroy() {
mTabModelSelectorTabModelObserver.destroy();
mLayoutManagerObservableSupplier.removeObserver(mLayoutManagerSupplierCallback);
mLayoutManager.removeObserver(mLayoutStateObserver);
}

private void observeTabModel() {
mTabModelSelectorTabModelObserver = new TabModelSelectorTabModelObserver(mModelSelector) {
@Override
public void willCloseTab(Tab tab, boolean animate) {
// TODO(crbug.com/1324405) Extract common logic between this method and
// willCloseAllTabs into a helper method.
if (mClosedAllTabs != null || tab.isIncognito()) return;

int tabId = tab.getId();
TabModel model = mModelSelector.getModel(false);

if (!mTabSwitcherActive && mIsTablet) {
mTabsClosedFromTabStrip.add(tabId);
}

int selTabIndex = model.index();
if (selTabIndex > -1 && selTabIndex < model.getCount()) {
Tab selectedTab = model.getTabAt(selTabIndex);
if (selectedTab != null && tabId == selectedTab.getId()) {
mSelectedTabIdWhenTabClosed = tabId;
}
}
}

@Override
public void didSelectTab(Tab tab, int type, int lastId) {
// Undoing a selected tab closure, after manually switching tabs shouldn't switch
// focus to the reopened tab.
if (type == TabSelectionType.FROM_USER || type == TabSelectionType.FROM_OMNIBOX
|| type == TabSelectionType.FROM_NEW) {
resetSelectionsForUndo();
}
}

@Override
public void willCloseAllTabs(boolean incognito) {
if (!incognito) {
TabModel model = mModelSelector.getCurrentModel();
int selTabIndex = model.index();
mClosedAllTabs = true;
if (selTabIndex > -1 && selTabIndex < model.getCount()) {
Tab tab = model.getTabAt(selTabIndex);
if (tab != null) {
mSelectedTabIdWhenTabClosed = tab.getId();
if (!mTabSwitcherActive && mIsTablet) {
mTabsClosedFromTabStrip.add(tab.getId());
}
}
}
}
}

@Override
public void tabClosureUndone(Tab tab) {
if (mClosedAllTabs != null) return;
int id = tab.getId();
recordClosureCancellation(id);
if (mSelectedTabIdWhenTabClosed != null && mSelectedTabIdWhenTabClosed == id) {
selectPreviouslySelectedTab();
}
}

@Override
public void allTabsClosureUndone() {
if (mSelectedTabIdWhenTabClosed != null) {
recordClosureCancellation(mSelectedTabIdWhenTabClosed);
selectPreviouslySelectedTab();
}

resetSelectionsForUndo();
mTabsClosedFromTabStrip.clear();
}

@Override
public void tabClosureCommitted(Tab tab) {
if (!tab.isIncognito()) {
resetSelectionsForUndo();
mTabsClosedFromTabStrip.clear();
}
}

@Override
public void allTabsClosureCommitted(boolean isIncognito) {
if (!isIncognito) {
resetSelectionsForUndo();
mTabsClosedFromTabStrip.clear();
}
}

private void recordClosureCancellation(int id) {
// Only record the action if the tab was previously closed from the tab strip.
if (mTabsClosedFromTabStrip.contains(id)) {
RecordUserAction.record("TabletTabStrip.UndoCloseTab");
mTabsClosedFromTabStrip.remove(id);
}
}
};
}

private void observeLayoutState() {
mLayoutManagerSupplierCallback = this::onLayoutManagerAvailable;
mLayoutManagerObservableSupplier.addObserver(mLayoutManagerSupplierCallback);
}

private void onLayoutManagerAvailable(LayoutManagerImpl layoutManager) {
mLayoutManager = layoutManager;
mLayoutStateObserver = new LayoutStateProvider.LayoutStateObserver() {
@Override
public void onFinishedShowing(int layoutType) {
if (layoutType != LayoutType.TAB_SWITCHER) return;
mTabSwitcherActive = true;
}

@Override
public void onFinishedHiding(int layoutType) {
if (layoutType != LayoutType.TAB_SWITCHER) return;
mTabSwitcherActive = false;
}
};

mLayoutManager.addObserver(mLayoutStateObserver);
}

/**
* If a tab closure is undone, this selects tab if it was previously selected.
*/
private void selectPreviouslySelectedTab() {
TabModel model = mModelSelector.getCurrentModel();
if (model == null || mSelectedTabIdWhenTabClosed == null) return;

int prevSelectedIndex = TabModelUtils.getTabIndexById(model, mSelectedTabIdWhenTabClosed);

TabModelUtils.setIndex(model, prevSelectedIndex, false, TabSelectionType.FROM_UNDO);
resetSelectionsForUndo();
}

/**
* After a tab closure has been committed or user manually selects a different tab, these values
* are reset so the next undo closure action does not reselect the reopened tab.
*/
private void resetSelectionsForUndo() {
mSelectedTabIdWhenTabClosed = null;
mClosedAllTabs = null;
}

@VisibleForTesting
public TabModelSelectorTabModelObserver getTabModelSelectorTabModelObserverForTests() {
return mTabModelSelectorTabModelObserver;
}

@VisibleForTesting
public Callback<LayoutManagerImpl> getLayoutManagerSupplierCallbackForTests() {
return mLayoutManagerSupplierCallback;
}

@VisibleForTesting
public void setTabSwitcherVisibilityForTests(boolean tabSwitcherActive) {
this.mTabSwitcherActive = tabSwitcherActive;
}

@VisibleForTesting
public void setLayoutManagerForTesting(LayoutManagerImpl layoutManager) {
this.mLayoutManager = layoutManager;
}
}

0 comments on commit d0e0946

Please sign in to comment.