Skip to content

Commit

Permalink
[Tab Management] Simplify M5 params
Browse files Browse the repository at this point in the history
Flatten launch params for M5 to make things a bit easier to reason
about. This CL flattens params for

* ENABLE_LAUNCH_POLISH
* ENABLE_LAUNCH_BUG_FIX
* ENABLE_DEFERRED_FAVICON

into TabUiFeatureUtilities#isTabGroupsAndroidContinuationEnabled(). This
flag has been turned off and will only be re-activated once we are ready
to re-attempt the M5 launch.

This CL also removes

* ENABLE_TAB_GROUP_SHARING

which is deprecated in favor of TabSelectionEditorV2.

The second, third, and fourth bugs are speculatively fixed by this.

Bug: 1420613, 1418922, 1420197, 1419842
Change-Id: I4ad5aa7e1e0467ef8fc47e28ad1548a02a47ea1f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4292154
Reviewed-by: Mei Liang <meiliang@chromium.org>
Commit-Queue: Calder Kitagawa <ckitagawa@chromium.org>
Code-Coverage: Findit <findit-for-me@appspot.gserviceaccount.com>
Reviewed-by: Sirisha Kavuluru <skavuluru@google.com>
Reviewed-by: Theresa Sullivan <twellington@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1115757}
  • Loading branch information
ckitagawa-work authored and Chromium LUCI CQ committed Mar 10, 2023
1 parent a81689c commit 2d18044
Show file tree
Hide file tree
Showing 38 changed files with 230 additions and 523 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -331,10 +331,9 @@ public StartSurfaceCoordinator(@NonNull Activity activity,
mGridTabSwitcher = TabManagementModuleProvider.getDelegate().createGridTabSwitcher(
activity, activityLifecycleDispatcher, tabModelSelector, tabContentManager,
browserControlsManager, tabCreatorManager, menuOrKeyboardActionController,
containerView, shareDelegateSupplier, multiWindowModeStateDispatcher,
scrimCoordinator, /* rootView= */ containerView, dynamicResourceLoaderSupplier,
snackbarManager, modalDialogManager, incognitoReauthControllerSupplier,
backPressManager);
containerView, multiWindowModeStateDispatcher, scrimCoordinator,
/* rootView= */ containerView, dynamicResourceLoaderSupplier, snackbarManager,
modalDialogManager, incognitoReauthControllerSupplier, backPressManager);
mTabSwitcherCustomViewManagerSupplier.set(
mGridTabSwitcher.getTabSwitcherCustomViewManager());
controller = mGridTabSwitcher.getController();
Expand Down Expand Up @@ -750,7 +749,6 @@ public TasksView getPrimarySurfaceView() {
* @param browserControlsStateProvider Gives access to the state of the browser controls.
* @param tabCreatorManager Manages creation of tabs.
* @param menuOrKeyboardActionController allows access to menu or keyboard actions.
* @param shareDelegateSupplier Supplies the current {@link ShareDelegate}.
* @param multiWindowModeStateDispatcher Gives access to the multi window mode state.
* @param rootView The root view of the app.
* @param incognitoReauthControllerSupplier {@link OneshotSupplier<IncognitoReauthController>}
Expand All @@ -769,7 +767,6 @@ TasksSurface createTasksSurface(@NonNull Activity activity,
@NonNull BrowserControlsStateProvider browserControlsStateProvider,
@NonNull TabCreatorManager tabCreatorManager,
@NonNull MenuOrKeyboardActionController menuOrKeyboardActionController,
@NonNull Supplier<ShareDelegate> shareDelegateSupplier,
@NonNull MultiWindowModeStateDispatcher multiWindowModeStateDispatcher,
@NonNull ViewGroup rootView,
@Nullable OneshotSupplier<IncognitoReauthController>
Expand All @@ -779,8 +776,7 @@ TasksSurface createTasksSurface(@NonNull Activity activity,
activityLifecycleDispatcher, tabModelSelector, snackbarManager,
dynamicResourceLoaderSupplier, tabContentManager, modalDialogManager,
browserControlsStateProvider, tabCreatorManager, menuOrKeyboardActionController,
shareDelegateSupplier, multiWindowModeStateDispatcher, rootView,
incognitoReauthControllerSupplier);
multiWindowModeStateDispatcher, rootView, incognitoReauthControllerSupplier);
}

@VisibleForTesting
Expand Down Expand Up @@ -845,7 +841,7 @@ private void createAndSetStartSurface(boolean excludeQueryTiles) {
mActivityLifecycleDispatcher, mTabModelSelector, mSnackbarManager,
mDynamicResourceLoaderSupplier, mTabContentManager, mModalDialogManager,
mBrowserControlsManager, mTabCreatorManager, mMenuOrKeyboardActionController,
mShareDelegateSupplier, mMultiWindowModeStateDispatcher, mContainerView, null);
mMultiWindowModeStateDispatcher, mContainerView, null);
mTasksSurface.getView().setId(R.id.primary_tasks_surface_view);
initializeOffsetChangedListener();
addHeaderOffsetChangeListener(mOffsetChangedListenerToGenerateScrollEvents);
Expand Down Expand Up @@ -882,7 +878,7 @@ private void createStartSurfaceWithoutTasksSurface(boolean excludeQueryTiles) {
mActivityLifecycleDispatcher, mTabModelSelector, mTabContentManager,
mBrowserControlsManager, mTabCreatorManager,
mMenuOrKeyboardActionController,
mView.getCarouselTabSwitcherContainer(), mShareDelegateSupplier,
mView.getCarouselTabSwitcherContainer(),
mMultiWindowModeStateDispatcher, mScrimCoordinator, mView,
mDynamicResourceLoaderSupplier, mSnackbarManager, mModalDialogManager);
} else {
Expand Down Expand Up @@ -941,7 +937,7 @@ private TabSwitcher.Controller initializeSecondaryTasksSurface() {
mActivityLifecycleDispatcher, mTabModelSelector, mSnackbarManager,
mDynamicResourceLoaderSupplier, mTabContentManager, mModalDialogManager,
mBrowserControlsManager, mTabCreatorManager, mMenuOrKeyboardActionController,
mShareDelegateSupplier, mMultiWindowModeStateDispatcher, mContainerView,
mMultiWindowModeStateDispatcher, mContainerView,
mIncognitoReauthControllerSupplier);
if (mIsInitializedWithNative) {
mSecondaryTasksSurface.onFinishNativeInitialization(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -513,7 +513,7 @@ private void showOverviewWithTabShrink(boolean animate, Supplier<Rect> target,
if (skipSlowZooming) {
showShrinkingAnimation &= quick;
}
if (TabUiFeatureUtilities.isLaunchPolishEnabled()) {
if (TabUiFeatureUtilities.isTabGroupsAndroidContinuationEnabled(getContext())) {
// Intentionally disable the shrinking animation when accessibility is enabled.
// During the shrinking animation, since the ComponsitorViewHolder is not focusable,
// I think we are in a temporary no "valid" focus target state, so the focus shifts
Expand Down Expand Up @@ -864,7 +864,7 @@ public boolean onUpdateAnimation(long time, boolean jumpToEnd) {

@Override
public boolean canHostBeFocusable() {
if (TabUiFeatureUtilities.isLaunchPolishEnabled()
if (TabUiFeatureUtilities.isTabGroupsAndroidContinuationEnabled(getContext())
&& ChromeAccessibilityUtil.get().isAccessibilityEnabled()
&& !DeviceFormFactor.isNonMultiDisplayContextOnTablet(getContext())) {
// We don't allow this layout to gain focus when accessibility is enabled so that the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
import org.chromium.chrome.browser.profiles.ProfileManager;
import org.chromium.chrome.browser.query_tiles.QueryTileSection;
import org.chromium.chrome.browser.query_tiles.QueryTileUtils;
import org.chromium.chrome.browser.share.ShareDelegate;
import org.chromium.chrome.browser.suggestions.tile.MostVisitedTilesCoordinator;
import org.chromium.chrome.browser.suggestions.tile.TileGroupDelegateImpl;
import org.chromium.chrome.browser.tab.Tab;
Expand Down Expand Up @@ -104,7 +103,6 @@ public TasksSurfaceCoordinator(@NonNull Activity activity,
@NonNull BrowserControlsStateProvider browserControlsStateProvider,
@NonNull TabCreatorManager tabCreatorManager,
@NonNull MenuOrKeyboardActionController menuOrKeyboardActionController,
@NonNull Supplier<ShareDelegate> shareDelegateSupplier,
@NonNull MultiWindowModeStateDispatcher multiWindowModeStateDispatcher,
@NonNull ViewGroup rootView,
@Nullable OneshotSupplier<IncognitoReauthController>
Expand All @@ -127,18 +125,17 @@ public TasksSurfaceCoordinator(@NonNull Activity activity,
mTabSwitcher = TabManagementModuleProvider.getDelegate().createCarouselTabSwitcher(
activity, activityLifecycleDispatcher, tabModelSelector, tabContentManager,
browserControlsStateProvider, tabCreatorManager, menuOrKeyboardActionController,
mView.getCarouselTabSwitcherContainer(), shareDelegateSupplier,
multiWindowModeStateDispatcher, scrimCoordinator, rootView,
dynamicResourceLoaderSupplier, snackbarManager, modalDialogManager);
mView.getCarouselTabSwitcherContainer(), multiWindowModeStateDispatcher,
scrimCoordinator, rootView, dynamicResourceLoaderSupplier, snackbarManager,
modalDialogManager);
} else if (tabSwitcherType == TabSwitcherType.GRID) {
assert incognitoReauthControllerSupplier
!= null : "Valid Incognito re-auth controller supplier needed to create GTS.";
mTabSwitcher = TabManagementModuleProvider.getDelegate().createGridTabSwitcher(activity,
activityLifecycleDispatcher, tabModelSelector, tabContentManager,
browserControlsStateProvider, tabCreatorManager, menuOrKeyboardActionController,
mView.getBodyViewContainer(), shareDelegateSupplier,
multiWindowModeStateDispatcher, scrimCoordinator, rootView,
dynamicResourceLoaderSupplier, snackbarManager, modalDialogManager,
mView.getBodyViewContainer(), multiWindowModeStateDispatcher, scrimCoordinator,
rootView, dynamicResourceLoaderSupplier, snackbarManager, modalDialogManager,
incognitoReauthControllerSupplier, null /*BackPressManager*/);
} else if (tabSwitcherType == TabSwitcherType.SINGLE) {
mTabSwitcher = new SingleTabSwitcherCoordinator(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ public static void maybeShowIPH(@FeatureConstants String featureName, View view,
TextBubble textBubble = new TextBubble(view.getContext(), view, textId, accessibilityTextId,
true, rectProvider, ChromeAccessibilityUtil.get().isAccessibilityEnabled());
textBubble.setDismissOnTouchInteraction(true);
if (!TabUiFeatureUtilities.isLaunchBugFixEnabled()) {
if (!TabUiFeatureUtilities.isTabGroupsAndroidContinuationEnabled(view.getContext())) {
textBubble.addOnDismissListener(() -> tracker.dismissed(featureName));
textBubble.show();
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,7 @@
import org.chromium.base.metrics.RecordUserAction;
import org.chromium.base.supplier.ObservableSupplier;
import org.chromium.base.supplier.ObservableSupplierImpl;
import org.chromium.base.supplier.Supplier;
import org.chromium.chrome.browser.compositor.layouts.content.TabContentManager;
import org.chromium.chrome.browser.share.ShareDelegate;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tabmodel.TabCreatorManager;
import org.chromium.chrome.browser.tabmodel.TabModelSelector;
Expand Down Expand Up @@ -59,8 +57,8 @@ public class TabGridDialogCoordinator implements TabGridDialogMediator.DialogCon
ViewGroup containerView, TabSwitcherMediator.ResetHandler resetHandler,
TabListMediator.GridCardOnClickListenerProvider gridCardOnClickListenerProvider,
TabGridDialogMediator.AnimationSourceViewProvider animationSourceViewProvider,
Supplier<ShareDelegate> shareDelegateSupplier, ScrimCoordinator scrimCoordinator,
TabGroupTitleEditor tabGroupTitleEditor, ViewGroup rootView) {
ScrimCoordinator scrimCoordinator, TabGroupTitleEditor tabGroupTitleEditor,
ViewGroup rootView) {
try (TraceEvent e = TraceEvent.scoped("TabGridDialogCoordinator.constructor")) {
mActivity = activity;
mComponentName = animationSourceViewProvider == null ? "TabGridDialogFromStrip"
Expand All @@ -83,8 +81,7 @@ public class TabGridDialogCoordinator implements TabGridDialogMediator.DialogCon

mMediator = new TabGridDialogMediator(activity, this, mModel, tabModelSelector,
tabCreatorManager, resetHandler, this::getRecyclerViewPosition,
animationSourceViewProvider, shareDelegateSupplier, mSnackbarManager,
mComponentName);
animationSourceViewProvider, mSnackbarManager, mComponentName);

// TODO(crbug.com/1031349) : Remove the inline mode logic here, make the constructor to
// take in a mode parameter instead.
Expand Down

0 comments on commit 2d18044

Please sign in to comment.