Skip to content

Commit

Permalink
DnD: Add support for tab tearing to a new window using drag and drop
Browse files Browse the repository at this point in the history
The feature is behind a TAB_DRAG_DROP_ANDROID feature flag which is set to false by default.

Pipe the multi instance manager and toolbar_container view from the ChromeTabbedActivity to the TabDragSource.

Add method to MultiInstanceManagerApi31 to launch a new Chrome window and reparent the selected tab to it when it is dropped outside the strip layout area within the toolbar container view.

Low-Coverage-Reason: The code is only executed in environments not in the CQ. Integration test over MultiInstanceManager31 aren't counted.
Bug: 276804039
Change-Id: I53a9c9490c176cc340eaf7383cf0bb581dff0fce
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4596528
Reviewed-by: Yusuke Sato <yusukes@chromium.org>
Commit-Queue: Gurmeet Kalra <gurmeetk@google.com>
Reviewed-by: Theresa Sullivan <twellington@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1158436}
  • Loading branch information
gurmeetk authored and Chromium LUCI CQ committed Jun 15, 2023
1 parent e3a48cb commit 864c253
Show file tree
Hide file tree
Showing 11 changed files with 359 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -737,11 +737,13 @@ private void setupCompositorContentPreNativeForTablet() {

// clang-format off
ViewGroup tabSwitcherViewHolder = findViewById(R.id.tab_switcher_view_holder);
View toolbarContainerView = findViewById(R.id.toolbar_container);
mLayoutManager = new LayoutManagerChromeTablet(compositorViewHolder, mContentContainer,
mStartSurfaceSupplier, mTabSwitcherSupplier, getTabContentManagerSupplier(),
mRootUiCoordinator::getTopUiThemeColorProvider, mTabModelStartupInfoSupplier,
tabSwitcherViewHolder, mRootUiCoordinator.getScrimCoordinator(),
getLifecycleDispatcher(), () -> createAndSetStartSurfaceForTablet());
getLifecycleDispatcher(), () -> createAndSetStartSurfaceForTablet(),
mMultiInstanceManager, toolbarContainerView);
mLayoutStateProviderSupplier.set(mLayoutManager);
// clang-format on
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

package org.chromium.chrome.browser.compositor.layouts;

import android.view.View;
import android.view.ViewGroup;

import org.chromium.base.supplier.ObservableSupplier;
Expand All @@ -15,6 +16,7 @@
import org.chromium.chrome.browser.device.DeviceClassManager;
import org.chromium.chrome.browser.layouts.LayoutType;
import org.chromium.chrome.browser.lifecycle.ActivityLifecycleDispatcher;
import org.chromium.chrome.browser.multiwindow.MultiInstanceManager;
import org.chromium.chrome.browser.tab.TabLaunchType;
import org.chromium.chrome.browser.tabmodel.TabCreatorManager;
import org.chromium.chrome.browser.tabmodel.TabModelSelector;
Expand Down Expand Up @@ -70,6 +72,10 @@ public class LayoutManagerChromeTablet extends LayoutManagerChrome {
* @param lifecycleDispatcher @{@link ActivityLifecycleDispatcher} to be passed to TabStrip
* helper.
* @param delayedStartSurfaceCallable Callable to create StartSurface/GTS views.
* @param multiInstanceManager @{link MultiInstanceManager} passed to @{link StripLayoutHelper}
* to support tab drag and drop.
* @param toolbarContainerView @{link View} passed to @{link StripLayoutHelper} to support tab
* drag and drop.
*/
public LayoutManagerChromeTablet(LayoutManagerHost host, ViewGroup contentContainer,
Supplier<StartSurface> startSurfaceSupplier, Supplier<TabSwitcher> tabSwitcherSupplier,
Expand All @@ -78,15 +84,19 @@ public LayoutManagerChromeTablet(LayoutManagerHost host, ViewGroup contentContai
ObservableSupplier<TabModelStartupInfo> tabModelStartupInfoSupplier,
ViewGroup tabSwitcherViewHolder, ScrimCoordinator scrimCoordinator,
ActivityLifecycleDispatcher lifecycleDispatcher,
Callable<ViewGroup> delayedStartSurfaceCallable) {
Callable<ViewGroup> delayedStartSurfaceCallable,
MultiInstanceManager multiInstanceManager, View toolbarContainerView) {
super(host, contentContainer, startSurfaceSupplier, tabSwitcherSupplier,
tabContentManagerSupplier, topUiThemeColorProvider, tabSwitcherViewHolder,
scrimCoordinator);
mStartSurfaceSupplier = startSurfaceSupplier;
mTabSwitcherSupplier = tabSwitcherSupplier;
mTabStripLayoutHelperManager = new StripLayoutHelperManager(host.getContext(), host, this,
mHost.getLayoutRenderHost(),
() -> mLayerTitleCache, tabModelStartupInfoSupplier, lifecycleDispatcher);
()
-> mLayerTitleCache,
tabModelStartupInfoSupplier, lifecycleDispatcher, multiInstanceManager,
toolbarContainerView);
mScrimCoordinator = scrimCoordinator;
mCreateStartSurfaceCallable = delayedStartSurfaceCallable;
addSceneOverlay(mTabStripLayoutHelperManager);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@
import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.layouts.animation.CompositorAnimator;
import org.chromium.chrome.browser.layouts.components.VirtualView;
import org.chromium.chrome.browser.multiwindow.MultiInstanceManager;
import org.chromium.chrome.browser.multiwindow.MultiWindowUtils;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tabmodel.TabCreator;
import org.chromium.chrome.browser.tabmodel.TabModel;
Expand Down Expand Up @@ -250,6 +252,8 @@ public Float get(StripLayoutHelper object) {
private int mCurrentPlaceholderIndex;

// Tab Drag and Drop state to hold clicked tab being dragged.
private MultiInstanceManager mMultiInstanceManager;
private View mToolbarContainerView;
private StripLayoutTab mActiveClickedTab;

/**
Expand All @@ -261,10 +265,15 @@ public Float get(StripLayoutHelper object) {
* @param incognito Whether or not this tab strip is incognito.
* @param modelSelectorButton The {@link CompositorButton} used to toggle between regular and
* incognito models.
* @param multiInstanceManager The @{link MultiInstanceManager} passed to @{link TabDragSource}
* for drag and drop.
* @param toolbarContainerView The @{link View} passed to @{link TabDragSource} for drag and
* drop.
*/
public StripLayoutHelper(Context context, LayoutManagerHost managerHost,
LayoutUpdateHost updateHost, LayoutRenderHost renderHost, boolean incognito,
CompositorButton modelSelectorButton) {
CompositorButton modelSelectorButton, MultiInstanceManager multiInstanceManager,
View toolbarContainerView) {
mTabOverlapWidth = ChromeFeatureList.sTabStripRedesign.isEnabled()
? TAB_OVERLAP_WIDTH_LARGE_DP
: TAB_OVERLAP_WIDTH_DP;
Expand All @@ -274,6 +283,8 @@ public StripLayoutHelper(Context context, LayoutManagerHost managerHost,
mNewTabButtonWidth = NEW_TAB_BUTTON_WIDTH_DP;
}
mModelSelectorButton = modelSelectorButton;
mMultiInstanceManager = multiInstanceManager;
mToolbarContainerView = toolbarContainerView;

if (ChromeFeatureList.sTabStripRedesign.isEnabled()) {
// Use toolbar menu button padding to align NTB with menu button.
Expand Down Expand Up @@ -1327,6 +1338,11 @@ public void onLongPress(long time, float x, float y) {
} else {
resetResizeTimeout(false);
startReorderMode(time, x, x);

// Allow the user to drag the selected tab out of the tab toolbar.
if (clickedTab != null) {
allowMovingTabOutOfStripLayout(clickedTab);
}
}
}

Expand Down Expand Up @@ -3108,4 +3124,38 @@ private void performHapticFeedback(Tab tab) {
protected void clearActiveClickedTab() {
mActiveClickedTab = null;
}

@VisibleForTesting(otherwise = VisibleForTesting.NONE)
StripLayoutTab getActiveClickedTab() {
return mActiveClickedTab;
}

protected void prepareForDragDrop() {
if (!MultiWindowUtils.isMultiInstanceApi31Enabled()) return;
if (!ChromeFeatureList.sTabDragDropAndroid.isEnabled()) return;

TabDragSource.getInstance().prepareForDragDrop(
mToolbarContainerView, mMultiInstanceManager);
}

@VisibleForTesting
void allowMovingTabOutOfStripLayout(StripLayoutTab clickedTab) {
if (!MultiWindowUtils.isMultiInstanceApi31Enabled()) return;
if (!ChromeFeatureList.sTabDragDropAndroid.isEnabled()) return;

// In addition to reordering, one can drag and drop the tab beyond the strip layout view.
// Also start the tab drag only if there are more than one tabs and a tab has been selected
// with the long press.
if (clickedTab != null && mStripTabsVisuallyOrdered.length > 1) {
Tab tabBeingDragged = getTabById(clickedTab.getId());
if (tabBeingDragged != null) {
// TODO(b/285624813): Verify if setting onDragListener on toolbar container view
// causes any conflict with images drop work.
if (TabDragSource.getInstance().startTabDragAction(
mToolbarContainerView, this, tabBeingDragged)) {
mActiveClickedTab = clickedTab;
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import android.graphics.RectF;
import android.os.Handler;
import android.os.SystemClock;
import android.view.View;

import androidx.annotation.ColorInt;
import androidx.annotation.VisibleForTesting;
Expand Down Expand Up @@ -38,6 +39,7 @@
import org.chromium.chrome.browser.layouts.scene_layer.SceneOverlayLayer;
import org.chromium.chrome.browser.lifecycle.ActivityLifecycleDispatcher;
import org.chromium.chrome.browser.lifecycle.PauseResumeWithNativeObserver;
import org.chromium.chrome.browser.multiwindow.MultiInstanceManager;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tab.TabCreationState;
import org.chromium.chrome.browser.tab.TabLaunchType;
Expand Down Expand Up @@ -242,12 +244,16 @@ public TabSwitcherLayoutObserver getTabSwitcherObserver() {
* @param tabModelStartupInfoSupplier A supplier for the {@link TabModelStartupInfo}.
* @param lifecycleDispatcher The {@link ActivityLifecycleDispatcher} for registering this class
* to lifecycle events.
* @param multiInstanceManager @{link MultiInstanceManager} passed to @{link TabDragSource} for
* drag and drop.
* @param toolbarContainerView @{link View} passed to @{link TabDragSource} for drag and drop.
*/
public StripLayoutHelperManager(Context context, LayoutManagerHost managerHost,
LayoutUpdateHost updateHost, LayoutRenderHost renderHost,
Supplier<LayerTitleCache> layerTitleCacheSupplier,
ObservableSupplier<TabModelStartupInfo> tabModelStartupInfoSupplier,
ActivityLifecycleDispatcher lifecycleDispatcher) {
ActivityLifecycleDispatcher lifecycleDispatcher,
MultiInstanceManager multiInstanceManager, View toolbarContainerView) {
mUpdateHost = updateHost;
mLayerTitleCacheSupplier = layerTitleCacheSupplier;
mTabStripTreeProvider = new TabStripSceneLayer(context);
Expand Down Expand Up @@ -343,10 +349,10 @@ public void onClick(long time) {

mBrowserScrimShowing = false;

mNormalHelper = new StripLayoutHelper(
context, managerHost, updateHost, renderHost, false, mModelSelectorButton);
mIncognitoHelper = new StripLayoutHelper(
context, managerHost, updateHost, renderHost, true, mModelSelectorButton);
mNormalHelper = new StripLayoutHelper(context, managerHost, updateHost, renderHost, false,
mModelSelectorButton, multiInstanceManager, toolbarContainerView);
mIncognitoHelper = new StripLayoutHelper(context, managerHost, updateHost, renderHost, true,
mModelSelectorButton, multiInstanceManager, toolbarContainerView);

if (tabModelStartupInfoSupplier != null) {
if (tabModelStartupInfoSupplier.hasValue()) {
Expand All @@ -356,6 +362,8 @@ public void onClick(long time) {
}
}

mNormalHelper.prepareForDragDrop();

onContextChanged(context);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
public class TabDragSource {
private static final String TAG = "TabDragSource";

// TODO(b/285585036) Expand the ClipData definition to support dropping of the Tab info to be
// TODO(b/285585036): Expand the ClipData definition to support dropping of the Tab info to be
// used by SysUI that can parse this format.
public static final String MIMETYPE_CHROME_TAB = "cr_tab/*";
public static final String[] SUPPORTED_MIMETYPES = new String[] {MIMETYPE_CHROME_TAB};
Expand Down Expand Up @@ -89,7 +89,7 @@ public boolean onDrag(View view, DragEvent dragEvent) {
case DragEvent.ACTION_DRAG_LOCATION:
float curXPos = dragEvent.getX() * mPxToDp;
float curYPos = dragEvent.getY() * mPxToDp;
// TODO(b/285590087) Enter Android drag mode until tab is torn vertically to
// TODO(b/285590087): Enter Android drag mode until tab is torn vertically to
// prevent forwarding drag events back into SripLayoutHelper #drag,
// #onUpOrCancel, #onDownInternal, etc.
if (mPointerInView) {
Expand Down Expand Up @@ -151,8 +151,7 @@ void resetState() {
@VisibleForTesting
void openTabInNewWindow() {
if (mTabBeingDragged != null) {
// To be implemented.
// MultiInstanceManager api's will be used here.
mMultiInstanceManager.moveTabToNewWindow(mTabBeingDragged);
mTabBeingDragged = null;
}
}
Expand Down Expand Up @@ -201,7 +200,7 @@ public void onProvideShadowMetrics(Point size, Point touch) {

// Set the touch point's position to be in the middle of the drag
// shadow.
// TODO(b/285584145) Update to accurate x and y posiiton of user hold/touch relative
// TODO(b/285584145): Update to accurate x and y posiiton of user hold/touch relative
// to the Drag Shadow/Chrome Window.
touch.set(width / 2, 50);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -473,6 +473,14 @@ private boolean isMergedInstanceTaskRunning() {
return false;
}

public void moveTabToNewWindow(Tab tab) {
// Not implemented
}

public void moveTabToCurrentWindow(Tab tab) {
// Not implemented
}

protected void moveTabToOtherWindow(Tab tab) {
Intent intent = mMultiWindowModeStateDispatcher.getOpenInOtherWindowIntent();
if (intent == null) return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import org.chromium.chrome.browser.app.tabmodel.TabModelOrchestrator;
import org.chromium.chrome.browser.app.tabmodel.TabWindowManagerSingleton;
import org.chromium.chrome.browser.feature_engagement.TrackerFactory;
import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.lifecycle.ActivityLifecycleDispatcher;
import org.chromium.chrome.browser.preferences.ChromePreferenceKeys;
import org.chromium.chrome.browser.preferences.SharedPreferencesManager;
Expand Down Expand Up @@ -126,14 +127,21 @@ private void moveTabAction(InstanceInfo info, Tab tab) {
if (targetActivity != null) {
reparentTabToRunningActivity((ChromeTabbedActivity) targetActivity, tab);
} else {
onMultiInstanceModeStarted();
Intent intent = MultiWindowUtils.createNewWindowIntent(mActivity, info.instanceId,
/*preferNew=*/false, /*openAdjacently=*/true, /*addTrustedIntentExtras=*/true);
ReparentingTask.from(tab).begin(mActivity, intent,
mMultiWindowModeStateDispatcher.getOpenInOtherWindowActivityOptions(), null);
moveAndReparentTabToNewWindow(tab, info.instanceId, /*preferNew=*/false,
/*openAdjacently=*/true, /*addTrustedIntentExtras=*/true);
}
}

@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
void moveAndReparentTabToNewWindow(Tab tab, int instanceId, boolean preferNew,
boolean openAdjacently, boolean addTrustedIntentExtras) {
onMultiInstanceModeStarted();
Intent intent = MultiWindowUtils.createNewWindowIntent(
mActivity, instanceId, preferNew, openAdjacently, addTrustedIntentExtras);
ReparentingTask.from(tab).begin(mActivity, intent,
mMultiWindowModeStateDispatcher.getOpenInOtherWindowActivityOptions(), null);
}

private void reparentTabToRunningActivity(ChromeTabbedActivity targetActivity, Tab tab) {
assert targetActivity != null;
Intent intent = new Intent();
Expand Down Expand Up @@ -694,4 +702,17 @@ private void onMultiInstanceStateChanged(boolean inMultiInstanceMode) {
prefs.writeLong(ChromePreferenceKeys.MULTI_INSTANCE_START_TIME, 0);
}
}

/**
* Open a new instance of the ChromeTabbedActivity window and move the
* specified tab from existing instance to the new one.
* @param tab Tab that is to be moved to a new Chrome instance.
*/
@Override
public void moveTabToNewWindow(Tab tab) {
if (!ChromeFeatureList.sTabDragDropAndroid.isEnabled()) return;

moveAndReparentTabToNewWindow(tab, INVALID_INSTANCE_ID, /*preferNew=*/true,
/*openAdjacently=*/false, /*addTrustedIntentExtras=*/true);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

import android.content.Context;
import android.view.ContextThemeWrapper;
import android.view.View;

import androidx.appcompat.content.res.AppCompatResources;
import androidx.test.core.app.ApplicationProvider;
Expand Down Expand Up @@ -39,6 +40,7 @@
import org.chromium.chrome.browser.flags.BooleanCachedFieldTrialParameter;
import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.lifecycle.ActivityLifecycleDispatcher;
import org.chromium.chrome.browser.multiwindow.MultiInstanceManager;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tasks.tab_management.TabManagementFieldTrial;
import org.chromium.chrome.browser.tasks.tab_management.TabUiFeatureUtilities;
Expand Down Expand Up @@ -67,6 +69,10 @@ public class StripLayoutHelperManagerTest {
private Supplier<LayerTitleCache> mLayerTitleCacheSupplier;
@Mock
private ActivityLifecycleDispatcher mLifecycleDispatcher;
@Mock
private MultiInstanceManager mMultiInstanceManager;
@Mock
private View mToolbarContainerView;

private StripLayoutHelperManager mStripLayoutHelperManager;
private Context mContext;
Expand Down Expand Up @@ -97,7 +103,7 @@ private void initializeTest() {
mTabModelStartupInfoSupplier = new ObservableSupplierImpl<>();
mStripLayoutHelperManager = new StripLayoutHelperManager(mContext, mManagerHost,
mUpdateHost, mRenderHost, mLayerTitleCacheSupplier, mTabModelStartupInfoSupplier,
mLifecycleDispatcher);
mLifecycleDispatcher, mMultiInstanceManager, mToolbarContainerView);
}

private void initializeTestWithTsrArm(BooleanCachedFieldTrialParameter param) {
Expand Down

0 comments on commit 864c253

Please sign in to comment.