Close the Chrome instance when the last tab is dragged out.
The closing is done asynchronously at the end of the drag process and the tab has already been moved out.
If any incognito tab is open in the instance then the Chrome instance is not closed.
The screen recording - http://shortn/_6CO0A9NX5q
Bug: b/315868023
Change-Id: I03ca5ae369319249bc1c7f8c4f1c04c0cf30a699
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5224828
Reviewed-by: Theresa Sullivan <twellington@chromium.org>
Reviewed-by: Wenyu Fu <wenyufu@chromium.org>
Commit-Queue: Gurmeet Kalra <gurmeetk@google.com>
Cr-Commit-Position: refs/heads/main@{#1255152}
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/compositor/overlays/strip/TabDragSource.java b/chrome/android/java/src/org/chromium/chrome/browser/compositor/overlays/strip/TabDragSource.java
index bcca84c..831a94e2 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/compositor/overlays/strip/TabDragSource.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/compositor/overlays/strip/TabDragSource.java
@@ -370,15 +370,19 @@
DragDropTabResult.IGNORED_DIFF_MODEL_NOT_SUPPORTED);
return false;
}
+ int sourceInstanceId =
+ DragDropGlobalState.getState(sDragTrackerToken).getDragSourceInstance();
if (!tabDraggedBelongToCurrentModel) {
mMultiInstanceManager.moveTabToWindow(
getActivity(),
tabBeingDragged,
- mTabModelSelector.getModel(tabBeingDragged.isIncognito()).getCount());
+ mTabModelSelector.getModel(tabBeingDragged.isIncognito()).getCount(),
+ sourceInstanceId);
showDroppedDifferentModelToast(mWindowAndroid.getContext().get());
} else {
int tabIndex = helper.getTabIndexForTabDrop(dropEvent.getX() * mPxToDp);
- mMultiInstanceManager.moveTabToWindow(getActivity(), tabBeingDragged, tabIndex);
+ mMultiInstanceManager.moveTabToWindow(
+ getActivity(), tabBeingDragged, tabIndex, sourceInstanceId);
helper.mergeToGroupForTabDropIfNeeded(groupRootId, tabBeingDragged.getId(), tabIndex);
}
DragDropMetricUtils.recordTabDragDropType(DragDropType.TAB_STRIP_TO_TAB_STRIP);
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/dragdrop/ChromeTabbedOnDragListener.java b/chrome/android/java/src/org/chromium/chrome/browser/dragdrop/ChromeTabbedOnDragListener.java
index 030eeb5..96b3524 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/dragdrop/ChromeTabbedOnDragListener.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/dragdrop/ChromeTabbedOnDragListener.java
@@ -103,7 +103,8 @@
TabModelUtils.getTabIndexById(
mTabModelSelector.getModel(currentTab.isIncognito()),
currentTab.getId())
- + 1);
+ + 1,
+ globalState.getDragSourceInstance());
DragDropMetricUtils.recordTabDragDropType(DragDropType.TAB_STRIP_TO_CONTENT);
return true;
}
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/multiwindow/MultiInstanceManager.java b/chrome/android/java/src/org/chromium/chrome/browser/multiwindow/MultiInstanceManager.java
index 3c176119..55e4954 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/multiwindow/MultiInstanceManager.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/multiwindow/MultiInstanceManager.java
@@ -497,7 +497,7 @@
// Not implemented
}
- public void moveTabToWindow(Activity activity, Tab tab, int atIndex) {
+ public void moveTabToWindow(Activity activity, Tab tab, int atIndex, int fromWindowInstanceId) {
// Not implemented
}
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/multiwindow/MultiInstanceManagerApi31.java b/chrome/android/java/src/org/chromium/chrome/browser/multiwindow/MultiInstanceManagerApi31.java
index b7a66f4..369c6da5 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/multiwindow/MultiInstanceManagerApi31.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/multiwindow/MultiInstanceManagerApi31.java
@@ -12,6 +12,8 @@
import android.os.Build.VERSION;
import android.os.Build.VERSION_CODES;
import android.os.Bundle;
+import android.os.Handler;
+import android.os.Looper;
import android.text.TextUtils;
import android.text.format.DateUtils;
import android.util.Pair;
@@ -74,6 +76,7 @@
public static final int INVALID_TASK_ID = MultiWindowUtils.INVALID_TASK_ID;
private static final String EMPTY_DATA = "";
+ private static final long DEFAULT_WINDOW_CLOSING_DELAY_FOR_DRAG_DROP_IN_MILLIS = 50;
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
protected final int mMaxInstances;
@@ -142,15 +145,31 @@
mActivity,
mModalDialogManagerSupplier.get(),
new LargeIconBridge(getProfile()),
- (instanceInfo) -> moveTabAction(instanceInfo, tab, TabList.INVALID_TAB_INDEX),
+ (instanceInfo) -> {
+ moveTabAction(instanceInfo, tab, TabList.INVALID_TAB_INDEX, mInstanceId, false);
+ },
getInstanceInfo());
}
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
- void moveTabAction(InstanceInfo info, Tab tab, int tabAtIndex) {
+ void moveTabAction(
+ InstanceInfo info,
+ Tab tab,
+ int tabAtIndex,
+ int fromWindowInstanceId,
+ boolean delayInWindowClosing) {
Activity targetActivity = getActivityById(info.instanceId);
if (targetActivity != null) {
reparentTabToRunningActivity((ChromeTabbedActivity) targetActivity, tab, tabAtIndex);
+
+ // Close the source instance window, if needed.
+ if (canCloseEmptyChromeWindow(fromWindowInstanceId)) {
+ closeEmptyChromeWindowAsynchronously(
+ fromWindowInstanceId,
+ delayInWindowClosing
+ ? DEFAULT_WINDOW_CLOSING_DELAY_FOR_DRAG_DROP_IN_MILLIS
+ : 0);
+ }
} else {
moveAndReparentTabToNewWindow(
tab,
@@ -930,18 +949,20 @@
/**
* Move the specified tab to the current instance of the ChromeTabbedActivity window.
+ *
* @param activity Activity of the Chrome Window in which the tab is to be moved.
* @param tab Tab that is to be moved to the current instance.
* @param atIndex Tab position index in the destination window instance.
+ * @param fromWindowInstanceId InNstance Id of the Chrome window it is being moved from.
*/
@Override
- public void moveTabToWindow(Activity activity, Tab tab, int atIndex) {
+ public void moveTabToWindow(Activity activity, Tab tab, int atIndex, int fromWindowInstanceId) {
if (!TabUiFeatureUtilities.isTabDragEnabled()) return;
// Get the current instance and move tab there.
InstanceInfo info = getInstanceInfoFor(activity);
if (info != null) {
- moveTabAction(info, tab, atIndex);
+ moveTabAction(info, tab, atIndex, fromWindowInstanceId, true);
} else {
Log.w(TAG, "DnD: InstanceInfo of Chrome Window not found.");
}
@@ -973,4 +994,33 @@
}
return null;
}
+
+ private boolean canCloseEmptyChromeWindow(int windowInstanceId) {
+ // Close the source instance window after reparenting if permitted by the feature flag and
+ // the source instance is known.
+ if (TabUiFeatureUtilities.isTabDragAsWindowEnabled()
+ && windowInstanceId != INVALID_INSTANCE_ID) {
+ TabModelSelector selector =
+ TabWindowManagerSingleton.getInstance()
+ .getTabModelSelectorById(windowInstanceId);
+ // Lastly determine if the drag source Chrome instance window has any tabs including
+ // incognito ones left so as to close if it is empty.
+ return selector.getTotalTabCount() == 0;
+ }
+ return false;
+ }
+
+ private void closeEmptyChromeWindowAsynchronously(
+ final int instanceId, long delayInMilliseconds) {
+ new Handler(Looper.getMainLooper())
+ // Posting the message to process at the end of the current event queue.
+ // Delay is needed for closing the instance during drag and drop as the
+ // DragEvent.ACTION_DRAG_ENDED requires processing in the source instance.
+ .postDelayed(
+ () -> {
+ Log.i(TAG, "Closing empty Chrome instance as no tabs exist.");
+ closeInstance(instanceId, INVALID_TASK_ID);
+ },
+ delayInMilliseconds);
+ }
}
diff --git a/chrome/android/junit/src/org/chromium/chrome/browser/compositor/overlays/strip/TabDragSourceTest.java b/chrome/android/junit/src/org/chromium/chrome/browser/compositor/overlays/strip/TabDragSourceTest.java
index 73b4db9..fd8002d 100644
--- a/chrome/android/junit/src/org/chromium/chrome/browser/compositor/overlays/strip/TabDragSourceTest.java
+++ b/chrome/android/junit/src/org/chromium/chrome/browser/compositor/overlays/strip/TabDragSourceTest.java
@@ -438,7 +438,8 @@
verify(mSourceStripLayoutHelper, times(1)).onUpOrCancel(anyLong());
// Verify tab is not moved.
verify(mSourceMultiInstanceManager, times(0)).moveTabToNewWindow(mTabBeingDragged);
- verify(mSourceMultiInstanceManager, times(0)).moveTabToWindow(any(), any(), anyInt());
+ verify(mSourceMultiInstanceManager, times(0))
+ .moveTabToWindow(any(), any(), anyInt(), anyInt());
// Verify clear.
verify(mSourceStripLayoutHelper, times(1)).clearTabDragState();
// Verify destination strip not invoked.
@@ -475,7 +476,8 @@
.clearForTabDrop(anyLong(), anyBoolean(), anyBoolean());
// Verify tab is not moved since drop is on source toolbar.
verify(mSourceMultiInstanceManager, times(0)).moveTabToNewWindow(mTabBeingDragged);
- verify(mSourceMultiInstanceManager, times(0)).moveTabToWindow(any(), any(), anyInt());
+ verify(mSourceMultiInstanceManager, times(0))
+ .moveTabToWindow(any(), any(), anyInt(), anyInt());
// Verify tab cleared.
verify(mSourceStripLayoutHelper, times(1)).clearTabDragState();
// Verify destination strip not invoked.
@@ -504,7 +506,8 @@
.clearForTabDrop(anyLong(), anyBoolean(), anyBoolean());
// Verify tab is not moved since drop is outside strip.
verify(mSourceMultiInstanceManager, times(0)).moveTabToNewWindow(mTabBeingDragged);
- verify(mSourceMultiInstanceManager, times(0)).moveTabToWindow(any(), any(), anyInt());
+ verify(mSourceMultiInstanceManager, times(0))
+ .moveTabToWindow(any(), any(), anyInt(), anyInt());
// Verify tab cleared.
verify(mSourceStripLayoutHelper, times(1)).clearTabDragState();
// Verify destination strip not invoked.
@@ -553,7 +556,7 @@
// Verify - Tab moved to destination window at TAB_INDEX.
verify(mDestMultiInstanceManager, times(1))
- .moveTabToWindow(any(), eq(mTabBeingDragged), eq(TAB_INDEX));
+ .moveTabToWindow(any(), eq(mTabBeingDragged), eq(TAB_INDEX), eq(CURR_INSTANCE_ID));
// Verify tab cleared.
verify(mSourceStripLayoutHelper, times(1)).clearTabDragState();
// Verify destination strip calls.
@@ -587,7 +590,7 @@
// Verify - Tab moved to destination window at end.
verify(mDestMultiInstanceManager, times(1))
- .moveTabToWindow(any(), eq(mTabBeingDragged), eq(5));
+ .moveTabToWindow(any(), eq(mTabBeingDragged), eq(5), eq(CURR_INSTANCE_ID));
assertNotNull(ShadowToast.getLatestToast());
TextView textView = (TextView) ShadowToast.getLatestToast().getView();
@@ -653,7 +656,8 @@
.clearForTabDrop(anyLong(), anyBoolean(), anyBoolean());
// Verify tab is not moved since drop is on source toolbar.
verify(mSourceMultiInstanceManager, times(0)).moveTabToNewWindow(mTabBeingDragged);
- verify(mSourceMultiInstanceManager, times(0)).moveTabToWindow(any(), any(), anyInt());
+ verify(mSourceMultiInstanceManager, times(0))
+ .moveTabToWindow(any(), any(), anyInt(), anyInt());
// Verify tab cleared.
verify(mSourceStripLayoutHelper, times(1)).clearTabDragState();
histogramExpectation.assertExpected();
@@ -686,7 +690,8 @@
verify(mSourceStripLayoutHelper, times(1)).onUpOrCancel(anyLong());
// Verify tab is not moved.
verify(mSourceMultiInstanceManager, times(0)).moveTabToNewWindow(mTabBeingDragged);
- verify(mSourceMultiInstanceManager, times(0)).moveTabToWindow(any(), any(), anyInt());
+ verify(mSourceMultiInstanceManager, times(0))
+ .moveTabToWindow(any(), any(), anyInt(), anyInt());
// Verify clear.
verify(mSourceStripLayoutHelper, times(1)).clearTabDragState();
// Verify destination strip not invoked.
@@ -733,7 +738,7 @@
// Verify - Move to new window not invoked.
verify(mDestMultiInstanceManager, times(0))
- .moveTabToWindow(any(), eq(mTabBeingDragged), anyInt());
+ .moveTabToWindow(any(), eq(mTabBeingDragged), anyInt(), anyInt());
histogramExpectation.assertExpected();
}
@@ -760,7 +765,7 @@
// Verify - Tab is not moved to destination window.
verify(mDestMultiInstanceManager, times(0))
- .moveTabToWindow(any(), eq(mTabBeingDragged), anyInt());
+ .moveTabToWindow(any(), eq(mTabBeingDragged), anyInt(), anyInt());
assertNull(ShadowToast.getLatestToast());
histogramExpectation.assertExpected();
diff --git a/chrome/android/junit/src/org/chromium/chrome/browser/multiwindow/MultiInstanceManagerApi31UnitTest.java b/chrome/android/junit/src/org/chromium/chrome/browser/multiwindow/MultiInstanceManagerApi31UnitTest.java
index aba1a636..6ec0f46 100644
--- a/chrome/android/junit/src/org/chromium/chrome/browser/multiwindow/MultiInstanceManagerApi31UnitTest.java
+++ b/chrome/android/junit/src/org/chromium/chrome/browser/multiwindow/MultiInstanceManagerApi31UnitTest.java
@@ -9,6 +9,7 @@
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
@@ -1130,13 +1131,15 @@
Mockito.doNothing()
.when(mMultiInstanceManager)
- .moveTabAction(any(), eq(mTab1), eq(tabAtIndex));
+ .moveTabAction(any(), eq(mTab1), eq(tabAtIndex), anyInt(), eq(true));
// Action
- mMultiInstanceManager.moveTabToWindow(mTabbedActivityTask63, mTab1, tabAtIndex);
+ mMultiInstanceManager.moveTabToWindow(
+ mTabbedActivityTask63, mTab1, tabAtIndex, INSTANCE_ID_1);
// Verify moveTabAction and getCurrentInstanceInfo are each called once.
- verify(mMultiInstanceManager, times(1)).moveTabAction(any(), eq(mTab1), eq(tabAtIndex));
+ verify(mMultiInstanceManager, times(1))
+ .moveTabAction(any(), eq(mTab1), eq(tabAtIndex), anyInt(), eq(true));
verify(mMultiInstanceManager, times(1)).getInstanceInfoFor(any());
}
@@ -1151,13 +1154,15 @@
INSTANCE_ID_1, TASK_ID_62, List.of(mTab1, mTab2, mTab3)));
Mockito.doNothing()
.when(multiInstanceManager)
- .moveTabAction(any(), eq(mTab2), eq(tabAtIndex));
+ .moveTabAction(any(), eq(mTab2), eq(tabAtIndex), anyInt(), eq(false));
// Action
- multiInstanceManager.moveTabToWindow(mTabbedActivityTask62, mTab2, tabAtIndex);
+ multiInstanceManager.moveTabToWindow(
+ mTabbedActivityTask62, mTab2, tabAtIndex, INSTANCE_ID_1);
// Verify moveTabAction is not called.
- verify(multiInstanceManager, times(0)).moveTabAction(any(), eq(mTab2), eq(tabAtIndex));
+ verify(multiInstanceManager, times(0))
+ .moveTabAction(any(), eq(mTab2), eq(tabAtIndex), anyInt(), eq(false));
}
@Test
@@ -1172,7 +1177,7 @@
// Action
InstanceInfo info = mMultiInstanceManager.getInstanceInfoFor(mTabbedActivityTask63);
- mMultiInstanceManager.moveTabAction(info, mTab1, /* atIndex= */ 0);
+ mMultiInstanceManager.moveTabAction(info, mTab1, /* atIndex= */ 0, INSTANCE_ID_1, true);
// Verify reparentTabToRunningActivity is called once.
verify(mMultiInstanceManager, times(1))
@@ -1218,7 +1223,7 @@
0,
false);
- mMultiInstanceManager.moveTabAction(info, mTab1, /* atIndex= */ 0);
+ mMultiInstanceManager.moveTabAction(info, mTab1, /* atIndex= */ 0, INSTANCE_ID_1, true);
// Verify moveAndReparentTabToNewWindow is called made with desired parameters once. The
// method is validated in integration test here
diff --git a/ui/android/java/src/org/chromium/ui/dragdrop/DragDropGlobalState.java b/ui/android/java/src/org/chromium/ui/dragdrop/DragDropGlobalState.java
index df7d63d..dead210 100644
--- a/ui/android/java/src/org/chromium/ui/dragdrop/DragDropGlobalState.java
+++ b/ui/android/java/src/org/chromium/ui/dragdrop/DragDropGlobalState.java
@@ -7,8 +7,10 @@
import android.os.SystemClock;
import android.view.DragEvent;
import android.view.View.DragShadowBuilder;
+
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
+
import org.chromium.base.Log;
import org.chromium.base.ResettersForTesting;
@@ -125,6 +127,11 @@
return mDragSourceInstanceId == instanceId;
}
+ /** Return the Chrome instance id of the drag source held by the global state. */
+ public int getDragSourceInstance() {
+ return mDragSourceInstanceId;
+ }
+
/** Return the {@link DropDataAndroid} held by the global state. */
public @NonNull DropDataAndroid getData() {
return mDropData;