Skip to content

Commit

Permalink
[TLDD] Use instanceId to differentiate dragsource. Moved shared state…
Browse files Browse the repository at this point in the history
…s to static class and made TabDragSource non-static.

* The current tab drag-drop implementation uses a hash of toolbarView to detect if this instance is same as that of drag source. Updated this to use instanceId from MultiWindowManager instead since drag could be invoked from tab switcher where we wont have toolbarView.
* Moved variables shared between instanced to DragDropSharedState static instance
* Cleanup tests and related source code
* Cleanup and simplify TabDropTarget

Moved this logic to common /dragdrop/ package.

Change-Id: Ic036847699f2b22987429d6ff2cad7d4cfc2812b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4943197
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Reviewed-by: Wenyu Fu <wenyufu@chromium.org>
Commit-Queue: Sirisha Kavuluru <skavuluru@google.com>
Reviewed-by: Calder Kitagawa <ckitagawa@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1217099}
  • Loading branch information
Sirisha Kavuluru authored and Chromium LUCI CQ committed Oct 30, 2023
1 parent f517843 commit 6bf60e0
Show file tree
Hide file tree
Showing 14 changed files with 633 additions and 1,044 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 @@ -609,6 +609,7 @@ chrome_java_sources = [
"java/src/org/chromium/chrome/browser/download/service/DownloadTaskScheduler.java",
"java/src/org/chromium/chrome/browser/dragdrop/ChromeDragAndDropBrowserDelegate.java",
"java/src/org/chromium/chrome/browser/dragdrop/ChromeDropDataAndroid.java",
"java/src/org/chromium/chrome/browser/dragdrop/DragDropGlobalState.java",
"java/src/org/chromium/chrome/browser/dragdrop/toolbar/TargetViewBinder.java",
"java/src/org/chromium/chrome/browser/dragdrop/toolbar/TargetViewDragListener.java",
"java/src/org/chromium/chrome/browser/dragdrop/toolbar/TargetViewProperties.java",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@
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;
Expand All @@ -65,7 +64,6 @@
import org.chromium.components.browser_ui.styles.ChromeColors;
import org.chromium.components.browser_ui.styles.SemanticColorUtils;
import org.chromium.ui.base.LocalizationUtils;
import org.chromium.ui.dragdrop.DragAndDropDelegate;
import org.chromium.ui.interpolators.Interpolators;
import org.chromium.ui.util.ColorUtils;

Expand Down Expand Up @@ -189,7 +187,6 @@ public Float get(StripLayoutHelper object) {
private final LayoutUpdateHost mUpdateHost;
private final LayoutRenderHost mRenderHost;
private final LayoutManagerHost mManagerHost;
private final DragAndDropDelegate mDragAndDropDelegate;
private TabModel mModel;
private TabGroupModelFilter mTabGroupModelFilter;
private TabCreator mTabCreator;
Expand Down Expand Up @@ -284,8 +281,8 @@ public Float get(StripLayoutHelper object) {
private int mPlaceholdersNeededDuringRestore;

// Tab Drag and Drop state to hold clicked tab being dragged.
private MultiInstanceManager mMultiInstanceManager;
private View mToolbarContainerView;
@Nullable private final TabDragSource mTabDragSource;
private StripLayoutTab mActiveClickedTab;
private TabDropTarget mTabDropTarget;

Expand All @@ -305,12 +302,7 @@ 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 dragDropDelegate The @{@link DragAndDropDelegate} passed to @{@link TabDragSource} to
* initiate drag and drop.
* @param browserControlsStateProvider The @BrowserControlsStateProvider passed for drag and
* drop.
* @param tabDragSource The @{@link TabDragSource} instance to initiate drag and drop.
* @param toolbarContainerView The @{link View} passed to @{link TabDragSource} for drag and
* drop.
*/
Expand All @@ -321,9 +313,7 @@ public StripLayoutHelper(
LayoutRenderHost renderHost,
boolean incognito,
CompositorButton modelSelectorButton,
MultiInstanceManager multiInstanceManager,
DragAndDropDelegate dragDropDelegate,
BrowserControlsStateProvider browserControlsStateProvider,
@Nullable TabDragSource tabDragSource,
View toolbarContainerView) {
mTabOverlapWidth = ChromeFeatureList.sTabStripRedesign.isEnabled()
? TAB_OVERLAP_WIDTH_LARGE_DP
Expand All @@ -334,10 +324,8 @@ public StripLayoutHelper(
mNewTabButtonWidth = NEW_TAB_BUTTON_WIDTH_DP;
}
mModelSelectorButton = modelSelectorButton;
mMultiInstanceManager = multiInstanceManager;
mToolbarContainerView = toolbarContainerView;
mDragAndDropDelegate = dragDropDelegate;
mBrowserControlStateProvider = browserControlsStateProvider;
mTabDragSource = tabDragSource;

if (ChromeFeatureList.sTabStripRedesign.isEnabled()) {
// Use toolbar menu button padding to align NTB with menu button.
Expand Down Expand Up @@ -3436,27 +3424,9 @@ StripLayoutTab getActiveClickedTabForTesting() {
return mActiveClickedTab;
}

TabDropTarget getTabDropTargetForTesting() {
return mTabDropTarget;
}

protected void prepareForDragDrop() {
if (!MultiWindowUtils.isMultiInstanceApi31Enabled()) return;
if (!TabUiFeatureUtilities.isTabDragEnabled()) return;

mTabDropTarget = new TabDropTarget(this);
TabDragSource.getInstance()
.prepareForDragDrop(
mToolbarContainerView,
mMultiInstanceManager,
mDragAndDropDelegate,
mTabDropTarget,
mBrowserControlStateProvider);
}

@VisibleForTesting
void allowMovingTabOutOfStripLayout(StripLayoutTab clickedTab, PointF dragStartPointF) {
if (!MultiWindowUtils.isMultiInstanceApi31Enabled()) return;
if (mTabDragSource == null) return;
if (!TabUiFeatureUtilities.isTabDragEnabled()) 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
Expand All @@ -3466,18 +3436,15 @@ void allowMovingTabOutOfStripLayout(StripLayoutTab clickedTab, PointF dragStartP
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, dragStartPointF)) {
boolean dragStarted =
mTabDragSource.startTabDragAction(
mToolbarContainerView, this, tabBeingDragged, dragStartPointF);
if (dragStarted) {
mActiveClickedTab = clickedTab;
}
}
}
}

View getToolbarContainerView() {
return mToolbarContainerView;
}

void selectTabAtIndex(int atIndex) {
if (!MultiWindowUtils.isMultiInstanceApi31Enabled()) return;
if (!TabUiFeatureUtilities.isTabDragEnabled()) return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import androidx.annotation.ColorInt;
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import androidx.annotation.VisibleForTesting;
import androidx.appcompat.content.res.AppCompatResources;
import androidx.core.graphics.ColorUtils;
Expand Down Expand Up @@ -47,6 +48,7 @@
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.multiwindow.MultiWindowUtils;
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 @@ -78,6 +80,7 @@
* all input and model events to the proper destination.
*/
public class StripLayoutHelperManager implements SceneOverlay, PauseResumeWithNativeObserver {

/**
* POD type that contains the necessary tab model info on startup. Used in the startup flicker
* fix experiment where we create a placeholder tab strip on startup to mitigate jank as tabs
Expand Down Expand Up @@ -146,21 +149,16 @@ public TabModelStartupInfo(int standardCount, int incognitoCount, int standardAc
private final float mHeight; // in dp units
private int mOrientation;
private CompositorButton mModelSelectorButton;

private Context mContext;
private boolean mBrowserScrimShowing;
private int mTabStripFadeShort;
private int mTabStripFadeLong;
private float mTabStripFadeShortWidth;
private float mTabStripFadeLongWidth;

private TabStripSceneLayer mTabStripTreeProvider;

private TabStripEventHandler mTabStripEventHandler;
private TabSwitcherLayoutObserver mTabSwitcherLayoutObserver;

private final ViewStub mTabHoverCardViewStub;

private float mModelSelectorWidth;
// 3-dots menu button with tab strip end padding
private float mMenuButtonPadding;
Expand All @@ -176,10 +174,13 @@ public void onTabModelSelected(TabModel newModel, TabModel oldModel) {

private TabModelObserver mTabModelObserver;
private final ActivityLifecycleDispatcher mLifecycleDispatcher;

private final String mDefaultTitle;
private final Supplier<LayerTitleCache> mLayerTitleCacheSupplier;

// Drag-Drop
@Nullable private TabDropTarget mTabDropTarget;
@Nullable private TabDragSource mTabDragSource;

private class TabStripEventHandler implements MotionEventHandler {
@Override
public void onDown(float x, float y, boolean fromMouse, int buttons) {
Expand Down Expand Up @@ -333,7 +334,6 @@ public StripLayoutHelperManager(
mDefaultTitle = context.getString(R.string.tab_loading_default_title);
mEventFilter =
new AreaMotionEventFilter(context, mTabStripEventHandler, null, false, false);

CompositorOnClickHandler selectorClickHandler = new CompositorOnClickHandler() {
@Override
public void onClick(long time) {
Expand All @@ -342,7 +342,6 @@ public void onClick(long time) {
};
if (ChromeFeatureList.sTabStripRedesign.isEnabled()) {
createModelSelectorButtonForTsr(context, selectorClickHandler);

// Model selector button background color.
// Default bg color is surface inverse.
int tsrBackgroundDefaultColor =
Expand Down Expand Up @@ -443,6 +442,17 @@ public void onClick(long time) {
mBrowserScrimShowing = false;

mTabHoverCardViewStub = tabHoverCardViewStub;
if (MultiWindowUtils.isMultiInstanceApi31Enabled()
&& TabUiFeatureUtilities.isTabDragEnabled()) {
mTabDragSource =
new TabDragSource(
toolbarContainerView,
multiInstanceManager,
dragDropDelegate,
managerHost.getBrowserControlsManager());
mTabDropTarget = new TabDropTarget(this, multiInstanceManager, toolbarContainerView);
}

mNormalHelper =
new StripLayoutHelper(
context,
Expand All @@ -451,9 +461,7 @@ public void onClick(long time) {
renderHost,
false,
mModelSelectorButton,
multiInstanceManager,
dragDropDelegate,
managerHost.getBrowserControlsManager(),
mTabDragSource,
toolbarContainerView);
mIncognitoHelper =
new StripLayoutHelper(
Expand All @@ -463,9 +471,7 @@ public void onClick(long time) {
renderHost,
true,
mModelSelectorButton,
multiInstanceManager,
dragDropDelegate,
managerHost.getBrowserControlsManager(),
mTabDragSource,
toolbarContainerView);

tabHoverCardViewStub.setOnInflateListener((viewStub, view) -> {
Expand All @@ -483,8 +489,6 @@ public void onClick(long time) {
}
}

mNormalHelper.prepareForDragDrop();

onContextChanged(context);
}

Expand Down Expand Up @@ -527,6 +531,8 @@ public void destroy() {
mTabModelSelectorTabModelObserver.destroy();
mTabModelSelectorTabObserver.destroy();
}
mTabDropTarget = null;
mTabDragSource = null;
}

@Override
Expand Down Expand Up @@ -1031,4 +1037,12 @@ void setTabStripTreeProviderForTesting(TabStripSceneLayer tabStripTreeProvider)
ViewStub getTabHoverCardViewStubForTesting() {
return mTabHoverCardViewStub;
}

public TabDragSource getTabDragSourceForTesting() {
return mTabDragSource;
}

public TabDropTarget getTabDropTargetForTesting() {
return mTabDropTarget;
}
}

0 comments on commit 6bf60e0

Please sign in to comment.