Skip to content

Commit

Permalink
Tab DnD: Set the drag shadow touch point to where user selected the t…
Browse files Browse the repository at this point in the history
…ab to drag.

Used a fast method to check the existing instances count.

(cherry picked from commit de2a6b9)

Bug: 1462762
Bug: b/285584145
Change-Id: Icce07018e8b63e8e0e03d96049c3ee82b92b5158
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4668133
Code-Coverage: Findit <findit-for-me@appspot.gserviceaccount.com>
Reviewed-by: Theresa Sullivan <twellington@chromium.org>
Commit-Queue: Gurmeet Kalra <gurmeetk@google.com>
Reviewed-by: Yusuke Sato <yusukes@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1166797}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4670633
Cr-Commit-Position: refs/branch-heads/5845@{#366}
Cr-Branched-From: 5a5dff6-refs/heads/main@{#1160321}
  • Loading branch information
gurmeetk authored and Chromium LUCI CQ committed Jul 7, 2023
1 parent 7a21861 commit 32be0de
Show file tree
Hide file tree
Showing 5 changed files with 89 additions and 31 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import android.annotation.SuppressLint;
import android.content.Context;
import android.content.res.Resources;
import android.graphics.PointF;
import android.os.Handler;
import android.os.Message;
import android.os.SystemClock;
Expand Down Expand Up @@ -1343,7 +1344,7 @@ public void onLongPress(long time, float x, float y) {

// Allow the user to drag the selected tab out of the tab toolbar.
if (clickedTab != null) {
allowMovingTabOutOfStripLayout(clickedTab);
allowMovingTabOutOfStripLayout(clickedTab, new PointF(x, y));
}
}
}
Expand Down Expand Up @@ -3145,7 +3146,7 @@ protected void prepareForDragDrop() {
}

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

Expand All @@ -3158,7 +3159,7 @@ void allowMovingTabOutOfStripLayout(StripLayoutTab clickedTab) {
// 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)) {
mToolbarContainerView, this, tabBeingDragged, dragStartPointF)) {
mActiveClickedTab = clickedTab;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import android.content.Intent;
import android.graphics.Color;
import android.graphics.Point;
import android.graphics.PointF;
import android.graphics.drawable.ColorDrawable;
import android.graphics.drawable.Drawable;
import android.view.DragEvent;
Expand Down Expand Up @@ -46,6 +47,7 @@ public class TabDragSource {
private Tab mTabBeingDragged;
private boolean mAcceptNextDrop;
private OnDragListenerImpl mOnDragListenerImpl;
private View.DragShadowBuilder mTabDragShadowBuilder;

private float mPxToDp;

Expand Down Expand Up @@ -106,6 +108,11 @@ void setMultiInstanceManager(MultiInstanceManager multiInstanceManager) {
mMultiInstanceManager = multiInstanceManager;
}

@VisibleForTesting(otherwise = VisibleForTesting.NONE)
View.DragShadowBuilder getTabDragShadowBuilder() {
return mTabDragShadowBuilder;
}

@VisibleForTesting
class OnDragListenerImpl implements View.OnDragListener {
private int mLastAction;
Expand Down Expand Up @@ -164,6 +171,7 @@ public boolean onDrag(View view, DragEvent dragEvent) {
// Clear the source view handle as DragNDrop is completed.
clearDragSourceTabsToolbarHashCode();
mSourceStripLayoutHelper.clearActiveClickedTab();
mTabDragShadowBuilder = null;
break;
case DragEvent.ACTION_DRAG_ENTERED:
mSourceStripLayoutHelper.onDownInternal(
Expand Down Expand Up @@ -203,22 +211,26 @@ void openTabInNewWindow() {
}
}

private void initiateTabDragAndDrop(View view, ClipData dragData) {
private void initiateTabDragAndDrop(View view, ClipData dragData, PointF startPointInView) {
// Instantiate the drag shadow builder.
View.DragShadowBuilder tabShadow = new TabDragShadowBuilder(getDragShadowView(view));
mTabDragShadowBuilder = new TabDragShadowBuilder(
getDragShadowView(view), getPositionOnScreen(view, startPointInView));

// Start the drag.
int flags = View.DRAG_FLAG_GLOBAL;
view.startDragAndDrop(dragData, // The data to be dragged.
tabShadow, // The drag shadow builder.
mTabDragShadowBuilder, // The drag shadow builder.
null, // No need to use local data.
flags);
}

private static class TabDragShadowBuilder extends View.DragShadowBuilder {
public TabDragShadowBuilder(View view) {
private PointF mStartPositionOnScreen;

public TabDragShadowBuilder(View view, PointF startPositionOnScreen) {
// Store the View parameter.
super(view);
mStartPositionOnScreen = startPositionOnScreen;
}

// Defines a callback that sends the drag shadow dimensions and touch point
Expand All @@ -237,11 +249,10 @@ public void onProvideShadowMetrics(Point size, Point touch) {
// to the system through the size parameter.
size.set(width, height);

// 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
// to the Drag Shadow/Chrome Window.
touch.set(width / 2, 50);
// Set the touch point of the drag shadow to be user's hold/touch point within Chrome
// Window.
touch.set(Math.round(mStartPositionOnScreen.x), Math.round(mStartPositionOnScreen.y));
Log.d(TAG, "DnD onProvideShadowMetrics: " + mStartPositionOnScreen);
}
}

Expand Down Expand Up @@ -288,9 +299,10 @@ String getClipDataInfo(Tab clickedTab) {
* @param sourceStripLayoutHelper @{link MultiInstanceManager} to forward drag message to @{link
* StripLayoutHelper} for reodering tabs.
* @param tabBeingDragged @{link Tab} is the selected tab being dragged.
* @param startPoint Position of the drag start point in view coordinates.
*/
public boolean startTabDragAction(
View tabsToolbarView, StripLayoutHelper sourceStripLayoutHelper, Tab tabBeingDragged) {
public boolean startTabDragAction(View tabsToolbarView,
StripLayoutHelper sourceStripLayoutHelper, Tab tabBeingDragged, PointF startPoint) {
if (!ChromeFeatureList.sTabDragDropAndroid.isEnabled()) return false;
if (getDragSourceTabsToolbarHashCode() != 0) return false;

Expand All @@ -303,7 +315,7 @@ public boolean startTabDragAction(
ClipData tabClipData = createClipDataFromTab(tabBeingDragged, tabsToolbarView);
if (tabClipData != null) {
mTabBeingDragged = tabBeingDragged;
initiateTabDragAndDrop(tabsToolbarView, tabClipData);
initiateTabDragAndDrop(tabsToolbarView, tabClipData, startPoint);

// Save this View handle to identify the source view.
mDragSourceTabsToolbarHashCode = System.identityHashCode(tabsToolbarView);
Expand Down Expand Up @@ -452,4 +464,19 @@ void sendPositionInfoToSysUI(View view, float startXInView, float startYInView,
"DnD Position info for SysUI: tId=" + activity.getTaskId() + ", xOff="
+ xOffsetRelative2WindowWidth + ", yOff=" + yOffsetRelative2WindowHeight);
}

private PointF getPositionOnScreen(View view, PointF positionInView) {
int[] topLeftLocationOfToolbarView = new int[2];
view.getLocationOnScreen(topLeftLocationOfToolbarView);

int[] topLeftLocationOfDecorView = new int[2];
View decorView = ((Activity) view.getContext()).getWindow().getDecorView();
decorView.getLocationOnScreen(topLeftLocationOfDecorView);

float positionXOnScreen = (topLeftLocationOfToolbarView[0] - topLeftLocationOfDecorView[0])
+ positionInView.x / mPxToDp;
float positionYOnScreen = (topLeftLocationOfToolbarView[1] - topLeftLocationOfDecorView[1])
+ positionInView.y / mPxToDp;
return new PointF(positionXOnScreen, positionYOnScreen);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -714,7 +714,7 @@ public void moveTabToNewWindow(Tab tab) {
if (!ChromeFeatureList.sTabDragDropAndroid.isEnabled()) return;

// Check if the new Chrome instance can be opened.
if (getInstanceInfo().size() < mMaxInstances) {
if (MultiWindowUtils.getInstanceCount() < mMaxInstances) {
moveAndReparentTabToNewWindow(tab, INVALID_INSTANCE_ID, /*preferNew=*/true,
/*openAdjacently=*/false, /*addTrustedIntentExtras=*/true);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.atLeastOnce;
import static org.mockito.Mockito.atMostOnce;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
Expand All @@ -30,6 +29,7 @@
import android.content.pm.PackageManager;
import android.content.pm.PackageManager.NameNotFoundException;
import android.content.res.Resources;
import android.graphics.PointF;
import android.text.TextUtils;
import android.util.DisplayMetrics;
import android.view.ContextThemeWrapper;
Expand Down Expand Up @@ -152,6 +152,7 @@ public class StripLayoutHelperTest {
private static final float NEW_TAB_BUTTON_WITH_MODEL_SELECTOR_BUTTON_PADDING = 8.f;
private static final float BUTTON_END_PADDING_TSR = 12.f;
private static final float MODEL_SELECTOR_BUTTON_BG_WIDTH_TSR = 32.f;
private static final PointF DRAG_START_POINT = new PointF(70f, 20f);

private static final float CLOSE_BTN_VISIBILITY_THRESHOLD_END = 72;

Expand Down Expand Up @@ -2318,7 +2319,7 @@ private void setTabDragSourceMock() {
} catch (Exception ex) {
assert (false);
}
when(mTabDragSource.startTabDragAction(any(), any(), any())).thenReturn(true);
when(mTabDragSource.startTabDragAction(any(), any(), any(), any())).thenReturn(true);

try {
mContextForDragDrop = Mockito.spy(ContextUtils.getApplicationContext());
Expand Down Expand Up @@ -2363,10 +2364,9 @@ public void testDrag_AllowMovingTabOutOfStripLayout_SetActiveTab() {
mStripLayoutHelper.getActiveClickedTab() == null);

// Act and verify.
mStripLayoutHelper.allowMovingTabOutOfStripLayout(theClickedTab);
mStripLayoutHelper.allowMovingTabOutOfStripLayout(theClickedTab, DRAG_START_POINT);

verify(mTabDragSource, atLeastOnce()).startTabDragAction(any(), any(), any());
verify(mTabDragSource, atMostOnce()).startTabDragAction(any(), any(), any());
verify(mTabDragSource, times(1)).startTabDragAction(any(), any(), any(), any());
assertTrue("Tab being dragged should exist during drag action.",
mStripLayoutHelper.getActiveClickedTab() != null);
assertTrue("Dragged Tab should match selected tab during drag action.",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@

import android.app.Activity;
import android.content.Context;
import android.graphics.Point;
import android.graphics.PointF;
import android.os.Parcel;
import android.view.ContextThemeWrapper;
import android.view.DragEvent;
Expand Down Expand Up @@ -101,6 +103,7 @@ public class TabDragSourceTest {
private static final long TIMESTAMP = 5000;
private static final float DROP_X_SCREEN_POS = 1000.f;
private static final float DROP_Y_SCREEN_POS = 500.f;
private static final PointF DRAG_START_POINT = new PointF(TAB_X_OFFSET, TAB_Y_OFFSET_WITHIN);

/** Resets the environment before each test. */
@Before
Expand Down Expand Up @@ -201,7 +204,7 @@ public void test_startTabDragAction_ReturnsTrueForValidTab() {
mTabDragSource.prepareForDragDrop(mTabsToolbarView, mMultiInstanceManager, mTabDropTarget);
assertTrue("Failed to start the tag drag action.",
mTabDragSource.startTabDragAction(
mTabsToolbarView, mStripLayoutHelper, tabBeingDragged));
mTabsToolbarView, mStripLayoutHelper, tabBeingDragged, DRAG_START_POINT));
verify(mTabsToolbarView, atLeastOnce()).startDragAndDrop(any(), any(), any(), anyInt());
verify(mTabsToolbarView, atMostOnce()).startDragAndDrop(any(), any(), any(), anyInt());
}
Expand All @@ -222,7 +225,7 @@ public void test_startTabDragAction_ReturnsFalseForInvalidTab() {
// Act and verify.
mTabDragSource.prepareForDragDrop(mTabsToolbarView, mMultiInstanceManager, mTabDropTarget);
assertFalse(mTabDragSource.startTabDragAction(
mTabsToolbarView, mStripLayoutHelper, tabBeingDragged));
mTabsToolbarView, mStripLayoutHelper, tabBeingDragged, DRAG_START_POINT));
verify(mTabsToolbarView, never()).startDragAndDrop(any(), any(), any(), anyInt());
}

Expand Down Expand Up @@ -261,7 +264,7 @@ public void test_getDragSourceTabsToolbarHashCode_ReturnHashCodeAfterDragAction(
mTabDragSource.prepareForDragDrop(mTabsToolbarView, mMultiInstanceManager, mTabDropTarget);
assertTrue(mTabDragSource.getDragSourceTabsToolbarHashCode() == 0);
assertTrue(mTabDragSource.startTabDragAction(
mTabsToolbarView, mStripLayoutHelper, tabBeingDragged));
mTabsToolbarView, mStripLayoutHelper, tabBeingDragged, DRAG_START_POINT));
assertTrue(mTabDragSource.getDragSourceTabsToolbarHashCode()
== System.identityHashCode(mTabsToolbarView));
}
Expand All @@ -277,7 +280,7 @@ public void test_OnDragListenerImpl_SimulateDragDropWithinStripLayout_ReturnsSuc
initializeTest(false, false, 1, 5);
mTabDragSource.prepareForDragDrop(mTabsToolbarView, mMultiInstanceManager, mTabDropTarget);
mTabDragSource.startTabDragAction(mTabsToolbarView, mStripLayoutHelper,
mStripLayoutHelper.getTabById(mClickedTab.getId()));
mStripLayoutHelper.getTabById(mClickedTab.getId()), DRAG_START_POINT);

// Perform drag n drop simulation actions for movement within the strip layout.
TabDragSource.OnDragListenerImpl onTabDragListener = simulateDragDropEvents(true);
Expand All @@ -302,7 +305,7 @@ public void test_OnDragListenerImpl_SimulateDragDropOutsideStripLayout_ReturnsSu
initializeTest(false, false, 1, 5);
mTabDragSource.prepareForDragDrop(mTabsToolbarView, mMultiInstanceManager, mTabDropTarget);
mTabDragSource.startTabDragAction(mTabsToolbarView, mStripLayoutHelper,
mStripLayoutHelper.getTabById(mClickedTab.getId()));
mStripLayoutHelper.getTabById(mClickedTab.getId()), DRAG_START_POINT);

// Perform drag n drop simulation actions for movement outside the strip layout.
TabDragSource.OnDragListenerImpl onTabDragListener =
Expand All @@ -329,7 +332,7 @@ public void test_OnDragListenerImpl_SimulateDragDropOutsideStripLayout_ReturnsSu
initializeTest(false, false, 1, 5);
mTabDragSource.prepareForDragDrop(mTabsToolbarView, mMultiInstanceManager, mTabDropTarget);
mTabDragSource.startTabDragAction(mTabsToolbarView, mStripLayoutHelper,
mStripLayoutHelper.getTabById(mClickedTab.getId()));
mStripLayoutHelper.getTabById(mClickedTab.getId()), DRAG_START_POINT);

// Perform drag n drop simulation actions for movement within the strip layout.
TabDragSource.OnDragListenerImpl onTabDragListener = simulateDragDropEvents(true);
Expand All @@ -351,7 +354,7 @@ public void test_OnDragListenerImpl_ForOutsideStripMovement_NewWindowIsOpened_Re
initializeTest(false, false, 1, 5);
mTabDragSource.prepareForDragDrop(mTabsToolbarView, mMultiInstanceManager, mTabDropTarget);
mTabDragSource.startTabDragAction(mTabsToolbarView, mStripLayoutHelper,
mStripLayoutHelper.getTabById(mClickedTab.getId()));
mStripLayoutHelper.getTabById(mClickedTab.getId()), DRAG_START_POINT);

// Perform drag n drop simulation actions for movement outside the strip layout.
TabDragSource.OnDragListenerImpl onTabDragListener =
Expand All @@ -374,7 +377,7 @@ public void test_clearActiveClickedTab_SimulateDragDrop_ReturnsSuccess() {
initializeTest(false, false, 1, 5);
mTabDragSource.prepareForDragDrop(mTabsToolbarView, mMultiInstanceManager, mTabDropTarget);
mTabDragSource.startTabDragAction(mTabsToolbarView, mStripLayoutHelper,
mStripLayoutHelper.getTabById(mClickedTab.getId()));
mStripLayoutHelper.getTabById(mClickedTab.getId()), DRAG_START_POINT);

// Perform drag n drop simulation action.
TabDragSource.OnDragListenerImpl onTabDragListener = simulateDragDropEvents(true);
Expand Down Expand Up @@ -483,7 +486,7 @@ public void test_canAcceptTabDrop_SimulateDragDrops_ReturnsSuccess() {
initializeTest(false, false, 1, 5);
mTabDragSource.prepareForDragDrop(mTabsToolbarView, mMultiInstanceManager, mTabDropTarget);
mTabDragSource.startTabDragAction(mTabsToolbarView, mStripLayoutHelper,
mStripLayoutHelper.getTabById(mClickedTab.getId()));
mStripLayoutHelper.getTabById(mClickedTab.getId()), DRAG_START_POINT);
View tabsToolbarViewForDrop = getAnotherToolbarView();
when(tabsToolbarViewForDrop.getWidth()).thenReturn(300);
when(tabsToolbarViewForDrop.getHeight()).thenReturn(50);
Expand Down Expand Up @@ -535,7 +538,7 @@ public void test_sendPositionInfoToSysUI_WithNewWindowIsOpened_ReturnsSuccess()
initializeTest(false, false, 1, 5);
mTabDragSource.prepareForDragDrop(mTabsToolbarView, mMultiInstanceManager, mTabDropTarget);
mTabDragSource.startTabDragAction(mTabsToolbarView, mStripLayoutHelper,
mStripLayoutHelper.getTabById(mClickedTab.getId()));
mStripLayoutHelper.getTabById(mClickedTab.getId()), DRAG_START_POINT);

// Perform drag n drop simulation actions for movement outside the strip layout.
TabDragSource.OnDragListenerImpl onTabDragListener =
Expand All @@ -548,4 +551,31 @@ public void test_sendPositionInfoToSysUI_WithNewWindowIsOpened_ReturnsSuccess()
.sendPositionInfoToSysUI(eq(mTabsToolbarView), anyFloat(), anyFloat(),
eq(DROP_X_SCREEN_POS), eq(DROP_Y_SCREEN_POS));
}

/**
* Tests the instance of the local class {@link TabDragSource#TabDragShadowBuilder}.
*
* Checks that a drag start position of the drag is used as the anchor point.
*/
@Test
public void test_onProvideShadowMetrics_WithDesiredStartPosition_ReturnsSuccess() {
// Prepare
final float dragStartXPosition = 90f;
final float dragStartYPosition = 45f;
initializeTest(false, false, 1, 5);
mTabDragSource.prepareForDragDrop(mTabsToolbarView, mMultiInstanceManager, mTabDropTarget);
mTabDragSource.startTabDragAction(mTabsToolbarView, mStripLayoutHelper,
mStripLayoutHelper.getTabById(mClickedTab.getId()),
new PointF(dragStartXPosition, dragStartYPosition));
View.DragShadowBuilder tabDragShadowBuilder = mTabDragSource.getTabDragShadowBuilder();

// Perform asking the TabDragShadowBuilder what is the anchor point.
Point dragSize = new Point(0, 0);
Point dragAnchor = new Point(0, 0);
tabDragShadowBuilder.onProvideShadowMetrics(dragSize, dragAnchor);

// Validate anchor.
assertTrue(dragAnchor.x == Math.round(dragStartXPosition));
assertTrue(dragAnchor.y == Math.round(dragStartYPosition));
}
}

0 comments on commit 32be0de

Please sign in to comment.