WebUI Tab Strip: Allow dragging tabs between windows

This CL sets up the ability to drag a tab from one WebUI tab strip and
drop it into another WebUI tab strip. The CL currently only places the
dropped tab to the end of the tab strip, and does not set the index
or the pinned state of the tab.

Future CLs will handle the UI for dropping a tab specifically into
place, and visually differentiating the drops that are allowed and
disallowed.

This implementation currently still uses the Extensions API for
moving tabs between windows, and the Extensions API already checks to
make sure the tab can be legally moved, taking into account
multiple profiles and incognito windows.

Bug: 1005560
Change-Id: I7da09a561144d911660f068f76919efe72f3638a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2020609
Commit-Queue: John Lee <johntlee@chromium.org>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#736060}

chrome/browser/resources/tab_strip/tab_list.js

@@ -36,6 +36,15 @@ export function setScrollAnimationEnabledForTesting(enabled) {
   scrollAnimationEnabled = enabled;
 }
 
+/**
+ * Gets the data type of tab IDs on DataTransfer objects in drag events. This
+ * is a function so that loadTimeData can get overridden by tests.
+ * @return {string}
+ */
+function getTabIdDataType() {
+  return loadTimeData.getString('tabIdDataType');
+}
+
 /**
  * @enum {string}
  */
@@ -147,6 +156,9 @@ class TabListElement extends CustomElement {
     /** @private {!Function} */
     this.windowBlurListener_ = () => this.onWindowBlur_();
 
+    /** @private @const {number} */
+    this.windowId_;
+
     /** @private {!Function} */
     this.contextMenuListener_ = e => this.onContextMenu_(e);
 
@@ -167,6 +179,8 @@ class TabListElement extends CustomElement {
         'dragend', (e) => this.onDragEnd_(/** @type {!DragEvent} */ (e)));
     this.addEventListener(
         'dragover', (e) => this.onDragOver_(/** @type {!DragEvent} */ (e)));
+    this.addEventListener(
+        'drop', e => this.onDrop_(/** @type {!DragEvent} */ (e)));
 
     document.addEventListener('contextmenu', this.contextMenuListener_);
     document.addEventListener(
@@ -266,6 +280,10 @@ class TabListElement extends CustomElement {
         layout => this.applyCSSDictionary_(layout));
     this.fetchAndUpdateColors_();
 
+    this.tabStripEmbedderProxy_.getWindowId().then(windowId => {
+      this.windowId_ = windowId;
+    });
+
     const getTabsStartTimestamp = Date.now();
     this.tabsApi_.getTabs().then(tabs => {
       this.tabStripEmbedderProxy_.reportTabDataReceivedDuration(
@@ -508,9 +526,7 @@ class TabListElement extends CustomElement {
       return;
     }
 
-    const allTabElements =
-        Array.from(this.shadowRoot.querySelectorAll('tabstrip-tab'));
-
+    const allTabElements = Array.from(this.$all('tabstrip-tab'));
     const dragOverTabElement = composedPath.find(isTabElement);
     if (dragOverTabElement && !dragOverTabElement.tab.pinned) {
       const dragOverIndex = allTabElements.indexOf(dragOverTabElement);
@@ -558,7 +574,8 @@ class TabListElement extends CustomElement {
 
     const dragOverIndex =
         Array.from(this.$all('tabstrip-tab')).indexOf(dragOverTabElement);
-    this.tabsApi_.moveTab(this.draggedItem_.tab.id, dragOverIndex);
+    this.tabsApi_.moveTab(
+        this.draggedItem_.tab.id, this.windowId_, dragOverIndex);
   }
 
   /**
@@ -578,6 +595,32 @@ class TabListElement extends CustomElement {
     event.dataTransfer.setDragImage(
         this.draggedItem_.getDragImage(), event.clientX - draggedItemRect.left,
         event.clientY - draggedItemRect.top);
+
+    if (isTabElement(draggedItem)) {
+      event.dataTransfer.setData(
+          getTabIdDataType(), this.draggedItem_.tab.id.toString());
+    }
+  }
+
+  /**
+   * @param {!DragEvent} event
+   * @private
+   */
+  onDrop_(event) {
+    if (this.draggedItem_) {
+      // If there is a valid dragged item, the drag originated from this TabList
+      // and is handled already by previous dragover events.
+      return;
+    }
+
+    if (event.dataTransfer.types.includes(getTabIdDataType())) {
+      const tabId = Number(event.dataTransfer.getData(getTabIdDataType()));
+      if (Number.isNaN(tabId)) {
+        // Invalid tab ID. Return silently.
+        return;
+      }
+      this.tabsApi_.moveTab(tabId, this.windowId_, -1);
+    }
   }
 
   /** @private */

chrome/browser/resources/tab_strip/tab_strip_embedder_proxy.js

@@ -26,6 +26,11 @@ export class TabStripEmbedderProxy {
     return sendWithPromise('getLayout');
   }
 
+  /** @return {!Promise<number>} */
+  getWindowId() {
+    return sendWithPromise('getWindowId');
+  }
+
   observeThemeChanges() {
     chrome.send('observeThemeChanges');
   }

chrome/browser/resources/tab_strip/tabs_api_proxy.js

@@ -142,9 +142,9 @@ export class TabsApiProxy {
    * @param {number} newIndex
    * @return {!Promise<!ExtensionsApiTab>}
    */
-  moveTab(tabId, newIndex) {
+  moveTab(tabId, windowId, newIndex) {
     return new Promise(resolve => {
-      chrome.tabs.move(tabId, {index: newIndex}, tab => {
+      chrome.tabs.move(tabId, {index: newIndex, windowId}, tab => {
         resolve(tab);
       });
     });

chrome/browser/ui/webui/tab_strip/tab_strip_ui.cc

@@ -5,6 +5,7 @@
 #include "chrome/browser/ui/webui/tab_strip/tab_strip_ui.h"
 
 #include "base/feature_list.h"
+#include "chrome/browser/extensions/extension_tab_util.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/themes/theme_properties.h"
 #include "chrome/browser/themes/theme_service.h"
@@ -34,6 +35,8 @@
 #include "ui/gfx/color_utils.h"
 #include "ui/resources/grit/webui_resources.h"
 
+const char kWebUITabIdDataType[] = "application/vnd.chromium.tab";
+
 TabStripUI::TabStripUI(content::WebUI* web_ui)
     : content::WebUIController(web_ui) {
   content::HostZoomMap::Get(web_ui->GetWebContents()->GetSiteInstance())
@@ -49,6 +52,8 @@ TabStripUI::TabStripUI(content::WebUI* web_ui)
       html_source, base::make_span(kTabStripResources, kTabStripResourcesSize),
       generated_path, IDR_TAB_STRIP_HTML);
 
+  html_source->AddString("tabIdDataType", kWebUITabIdDataType);
+
   // Add a load time string for the frame color to allow the tab strip to paint
   // a background color that matches the frame before any content loads
   const ui::ThemeProvider& tp =

chrome/browser/ui/webui/tab_strip/tab_strip_ui_handler.cc

@@ -341,6 +341,9 @@ void TabStripUIHandler::RegisterMessages() {
   web_ui()->RegisterMessageCallback(
       "getTabs",
       base::Bind(&TabStripUIHandler::HandleGetTabs, base::Unretained(this)));
+  web_ui()->RegisterMessageCallback(
+      "getWindowId", base::Bind(&TabStripUIHandler::HandleGetWindowId,
+                                base::Unretained(this)));
   web_ui()->RegisterMessageCallback(
       "getGroupVisualData",
       base::Bind(&TabStripUIHandler::HandleGetGroupVisualData,
@@ -480,6 +483,14 @@ void TabStripUIHandler::HandleGetTabs(const base::ListValue* args) {
   ResolveJavascriptCallback(callback_id, tabs);
 }
 
+void TabStripUIHandler::HandleGetWindowId(const base::ListValue* args) {
+  AllowJavascript();
+  const base::Value& callback_id = args->GetList()[0];
+  ResolveJavascriptCallback(
+      callback_id,
+      base::Value(extensions::ExtensionTabUtil::GetWindowId(browser_)));
+}
+
 void TabStripUIHandler::HandleGetGroupVisualData(const base::ListValue* args) {
   AllowJavascript();
   const base::Value& callback_id = args->GetList()[0];

chrome/browser/ui/webui/tab_strip/tab_strip_ui_handler.h

@@ -58,6 +58,7 @@ class TabStripUIHandler : public content::WebUIMessageHandler,
   base::DictionaryValue GetTabData(content::WebContents* contents, int index);
   base::DictionaryValue GetTabGroupData(TabGroup* group);
   void HandleGetTabs(const base::ListValue* args);
+  void HandleGetWindowId(const base::ListValue* args);
   void HandleGetGroupVisualData(const base::ListValue* args);
   void HandleGetThemeColors(const base::ListValue* args);
   void HandleCloseContainer(const base::ListValue* args);

chrome/test/data/webui/tab_strip/tab_list_test.js

@@ -4,6 +4,7 @@
 
 import {webUIListenerCallback} from 'chrome://resources/js/cr.m.js';
 import {FocusOutlineManager} from 'chrome://resources/js/cr/ui/focus_outline_manager.m.js';
+import {loadTimeData} from 'chrome://resources/js/load_time_data.m.js';
 import {setScrollAnimationEnabledForTesting} from 'chrome://tab-strip/tab_list.js';
 import {TabStripEmbedderProxy} from 'chrome://tab-strip/tab_strip_embedder_proxy.js';
 import {TabsApiProxy} from 'chrome://tab-strip/tabs_api_proxy.js';
@@ -81,6 +82,11 @@ suite('TabList', () => {
       title: 'Tab 3',
     },
   ];
+  const currentWindowId = 1000;
+
+  const strings = {
+    tabIdDataType: 'application/tab-id',
+  };
 
   function pinTabAt(tab, index) {
     const changeInfo = {index: index, pinned: true};
@@ -107,6 +113,7 @@ suite('TabList', () => {
   }
 
   setup(() => {
+    loadTimeData.overrideValues(strings);
     document.body.innerHTML = '';
     document.body.style.margin = 0;
 
@@ -116,6 +123,7 @@ suite('TabList', () => {
     callbackRouter = testTabsApiProxy.callbackRouter;
 
     testTabStripEmbedderProxy = new TestTabStripEmbedderProxy();
+    testTabStripEmbedderProxy.setWindowId(currentWindowId);
     testTabStripEmbedderProxy.setColors({
       '--background-color': 'white',
       '--foreground-color': 'black',
@@ -132,7 +140,10 @@ suite('TabList', () => {
     tabList = document.createElement('tabstrip-tab-list');
     document.body.appendChild(tabList);
 
-    return testTabsApiProxy.whenCalled('getTabs');
+    return Promise.all([
+      testTabsApiProxy.whenCalled('getTabs'),
+      testTabStripEmbedderProxy.whenCalled('getWindowId'),
+    ]);
   });
 
   teardown(() => {
@@ -549,8 +560,10 @@ suite('TabList', () => {
     });
     dragOverTab.dispatchEvent(dragOverEvent);
     assertEquals(dragOverEvent.dataTransfer.dropEffect, 'move');
-    const [tabId, newIndex] = await testTabsApiProxy.whenCalled('moveTab');
+    const [tabId, windowId, newIndex] =
+        await testTabsApiProxy.whenCalled('moveTab');
     assertEquals(tabId, tabs[draggedIndex].id);
+    assertEquals(currentWindowId, windowId);
     assertEquals(newIndex, dragOverIndex);
   });
 
@@ -682,6 +695,26 @@ suite('TabList', () => {
     assertEquals(1, index);
   });
 
+  test('DropTabIntoList', async () => {
+    const droppedTabId = 9000;
+    const mockDataTransfer = new MockDataTransfer();
+    mockDataTransfer.setData(strings.tabIdDataType, droppedTabId);
+    const dropEvent = new DragEvent('drop', {
+      bubbles: true,
+      composed: true,
+      clientX: 100,
+      clientY: 150,
+      dataTransfer: mockDataTransfer,
+    });
+    tabList.dispatchEvent(dropEvent);
+
+    const [tabId, windowId, index] =
+        await testTabsApiProxy.whenCalled('moveTab');
+    assertEquals(droppedTabId, tabId);
+    assertEquals(currentWindowId, windowId);
+    assertEquals(-1, index);
+  });
+
   test('tracks and untracks thumbnails based on viewport', async () => {
     // Wait for slideIn animations to complete updating widths and reset
     // resolvers to track new calls.

chrome/test/data/webui/tab_strip/test_tab_strip_embedder_proxy.js

@@ -10,6 +10,7 @@ export class TestTabStripEmbedderProxy extends TestBrowserProxy {
       'closeContainer',
       'getColors',
       'getLayout',
+      'getWindowId',
       'isVisible',
       'observeThemeChanges',
       'showBackgroundContextMenu',
@@ -22,6 +23,7 @@ export class TestTabStripEmbedderProxy extends TestBrowserProxy {
     this.colors_ = {};
     this.layout_ = {};
     this.visible_ = false;
+    this.windowId_;
   }
 
   getColors() {
@@ -34,6 +36,11 @@ export class TestTabStripEmbedderProxy extends TestBrowserProxy {
     return Promise.resolve(this.layout_);
   }
 
+  getWindowId() {
+    this.methodCalled('getWindowId');
+    return Promise.resolve(this.windowId_);
+  }
+
   isVisible() {
     this.methodCalled('isVisible');
     return this.visible_;
@@ -60,6 +67,10 @@ export class TestTabStripEmbedderProxy extends TestBrowserProxy {
     return Promise.resolve();
   }
 
+  setWindowId(windowId) {
+    this.windowId_ = windowId;
+  }
+
   showBackgroundContextMenu(locationX, locationY) {
     this.methodCalled('showBackgroundContextMenu', [locationX, locationY]);
   }

chrome/test/data/webui/tab_strip/test_tabs_api_proxy.js

@@ -55,8 +55,8 @@ export class TestTabsApiProxy extends TestBrowserProxy {
     this.methodCalled('moveGroup', [groupId, newIndex]);
   }
 
-  moveTab(tabId, newIndex) {
-    this.methodCalled('moveTab', [tabId, newIndex]);
+  moveTab(tabId, windowId, newIndex) {
+    this.methodCalled('moveTab', [tabId, windowId, newIndex]);
     return Promise.resolve();
   }