WebUI Tab Strip: Show context menu after dropping tab

Bug: 1117652
Change-Id: If097acd73096be0b1b36e94088361a9715f53936
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2360344
Commit-Queue: John Lee <johntlee@chromium.org>
Reviewed-by: dpapad <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#799834}

chrome/browser/resources/tab_strip/drag_manager.js

@@ -9,6 +9,7 @@ import {loadTimeData} from 'chrome://resources/js/load_time_data.m.js';
 
 import {isTabElement, TabElement} from './tab.js';
 import {isTabGroupElement, TabGroupElement} from './tab_group.js';
+import {TabStripEmbedderProxy, TabStripEmbedderProxyImpl} from './tab_strip_embedder_proxy.js';
 import {TabData, TabNetworkState, TabsApiProxy, TabsApiProxyImpl} from './tabs_api_proxy.js';
 
 /** @const {number} */
@@ -100,6 +101,13 @@ class DragSession {
     /** @const {!TabElement|!TabGroupElement} */
     this.element_ = element;
 
+    /**
+     * Flag indicating if during the drag session, the element has at least
+     * moved once.
+     * @private {boolean}
+     */
+    this.hasMoved_ = false;
+
     /** @const {number} */
     this.srcIndex = srcIndex;
 
@@ -108,6 +116,9 @@ class DragSession {
 
     /** @private @const {!TabsApiProxy} */
     this.tabsProxy_ = TabsApiProxyImpl.getInstance();
+
+    /** @private {!TabStripEmbedderProxy} */
+    this.tabStripEmbedderProxy_ = TabStripEmbedderProxyImpl.getInstance();
   }
 
   /**
@@ -279,6 +290,13 @@ class DragSession {
 
     this.element_.setDragging(false);
     this.element_.setDraggedOut(false);
+
+    if (isTabElement(this.element_) && !this.hasMoved_) {
+      // If the user was dragging a tab and the tab has not ever been moved,
+      // show a context menu instead.
+      this.tabStripEmbedderProxy_.showTabContextMenu(
+          this.element_.tab.id, event.clientX, event.clientY);
+    }
   }
 
   /**
@@ -352,6 +370,7 @@ class DragSession {
   update(event) {
     if (event.type === 'dragleave') {
       this.element_.setDraggedOut(true);
+      this.hasMoved_ = true;
       return;
     }
 
@@ -385,6 +404,7 @@ class DragSession {
       dragOverIndex +=
           this.shouldOffsetIndexForGroup_(dragOverTabElement) ? 1 : 0;
       this.delegate_.placeTabGroupElement(tabGroupElement, dragOverIndex);
+      this.hasMoved_ = true;
       return;
     }
 
@@ -396,6 +416,7 @@ class DragSession {
       dragOverIndex +=
           this.shouldOffsetIndexForGroup_(dragOverGroupElement) ? 1 : 0;
       this.delegate_.placeTabGroupElement(tabGroupElement, dragOverIndex);
+      this.hasMoved_ = true;
     }
   }
 
@@ -427,12 +448,14 @@ class DragSession {
         dragOverTabGroup.isValidDragOverTarget) {
       this.delegate_.placeTabElement(
           tabElement, this.dstIndex, false, dragOverTabGroup.dataset.groupId);
+      this.hasMoved_ = true;
       return;
     }
 
     if (!dragOverTabGroup && previousGroupId) {
       this.delegate_.placeTabElement(
           tabElement, this.dstIndex, false, undefined);
+      this.hasMoved_ = true;
       return;
     }
 
@@ -443,6 +466,7 @@ class DragSession {
     const dragOverIndex = this.delegate_.getIndexOfTab(dragOverTabElement);
     this.delegate_.placeTabElement(
         tabElement, dragOverIndex, tabElement.tab.pinned, previousGroupId);
+    this.hasMoved_ = true;
   }
 }
 

chrome/browser/resources/tab_strip/tab.js

@@ -233,13 +233,6 @@ export class TabElement extends CustomElement {
    */
   onContextMenu_(event) {
     event.preventDefault();
-
-    if (!this.tab_) {
-      return;
-    }
-
-    this.embedderApi_.showTabContextMenu(
-        this.tab_.id, event.clientX, event.clientY);
     event.stopPropagation();
   }
 

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

@@ -2,8 +2,6 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/ui/webui/tab_strip/tab_strip_ui.h"
-
 #include <memory>
 
 #include "base/command_line.h"
@@ -14,6 +12,7 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/ui_features.h"
+#include "chrome/browser/ui/webui/tab_strip/tab_strip_ui.h"
 #include "chrome/browser/ui/webui/tab_strip/tab_strip_ui_embedder.h"
 #include "chrome/browser/ui/webui/tab_strip/tab_strip_ui_layout.h"
 #include "chrome/common/chrome_isolated_world_ids.h"
@@ -111,24 +110,6 @@ IN_PROC_BROWSER_TEST_F(TabStripUIBrowserTest, ActivatingTabClosesEmbedder) {
                               ISOLATED_WORLD_ID_CHROME_INTERNAL));
 }
 
-// Checks that the contextmenu event on a tab gets forwarded to the
-// TabStripUI::Embedder.
-IN_PROC_BROWSER_TEST_F(TabStripUIBrowserTest,
-                       InvokesEmbedderContextMenuForTab) {
-  using ::testing::_;
-
-  const std::string invoke_menu_js =
-      "const event ="
-      "    new MouseEvent('contextmenu', { clientX: 100, clientY: 50 });" +
-      tab_query_js + ".dispatchEvent(event)";
-
-  EXPECT_CALL(mock_embedder_, ShowContextMenuAtPoint(gfx::Point(100, 50), _))
-      .Times(1);
-  ASSERT_TRUE(content::ExecJs(webui_contents_.get(), invoke_menu_js,
-                              content::EXECUTE_SCRIPT_DEFAULT_OPTIONS,
-                              ISOLATED_WORLD_ID_CHROME_INTERNAL));
-}
-
 IN_PROC_BROWSER_TEST_F(TabStripUIBrowserTest, InvokesEditDialogForGroups) {
   using ::testing::_;
 

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

@@ -6,10 +6,12 @@ import {loadTimeData} from 'chrome://resources/js/load_time_data.m.js';
 import {DragManager, DragManagerDelegate, PLACEHOLDER_GROUP_ID, PLACEHOLDER_TAB_ID} from 'chrome://tab-strip/drag_manager.js';
 import {TabElement} from 'chrome://tab-strip/tab.js';
 import {TabGroupElement} from 'chrome://tab-strip/tab_group.js';
+import {TabStripEmbedderProxyImpl} from 'chrome://tab-strip/tab_strip_embedder_proxy.js';
 import {TabData, TabsApiProxyImpl} from 'chrome://tab-strip/tabs_api_proxy.js';
 
 import {assertEquals, assertFalse, assertTrue} from '../chai_assert.js';
 
+import {TestTabStripEmbedderProxy} from './test_tab_strip_embedder_proxy.js';
 import {TestTabsApiProxy} from './test_tabs_api_proxy.js';
 
 /** @implements {DragManagerDelegate} */
@@ -88,6 +90,9 @@ suite('DragManager', () => {
   let dragManager;
   let testTabsApiProxy;
 
+  /** @type {!TestTabStripEmbedderProxy} */
+  let testTabStripEmbedderProxy;
+
   const tabs = [
     {
       active: true,
@@ -134,6 +139,9 @@ suite('DragManager', () => {
     testTabsApiProxy = new TestTabsApiProxy();
     TabsApiProxyImpl.instance_ = testTabsApiProxy;
 
+    testTabStripEmbedderProxy = new TestTabStripEmbedderProxy();
+    TabStripEmbedderProxyImpl.instance_ = testTabStripEmbedderProxy;
+
     delegate = new MockDelegate();
     tabs.forEach(tab => {
       const tabElement = document.createElement('tabstrip-tab');
@@ -641,4 +649,44 @@ suite('DragManager', () => {
     delegate.dispatchEvent(new DragEvent('dragover', {dataTransfer}));
     assertFalse(isDraggedOut);
   });
+
+  test('DropTabWithoutMovingShowsContextMenu', async () => {
+    const draggedTab = delegate.children[0];
+    const dragDetails = {
+      bubbles: true,
+      composed: true,
+      clientX: 100,
+      clientY: 150,
+      dataTransfer: new MockDataTransfer(),
+    };
+    draggedTab.dispatchEvent(new DragEvent('dragstart', dragDetails));
+    draggedTab.dispatchEvent(new DragEvent('drop', dragDetails));
+
+    assertEquals(
+        1, testTabStripEmbedderProxy.getCallCount('showTabContextMenu'));
+    const [tabId, clientX, clientY] =
+        await testTabStripEmbedderProxy.whenCalled('showTabContextMenu');
+    assertEquals(draggedTab.tab.id, tabId);
+    assertEquals(dragDetails.clientX, clientX);
+    assertEquals(dragDetails.clientY, clientY);
+  });
+
+  test('DropTabAfterMovingDoesNotShowContextMenu', async () => {
+    const draggedTab = delegate.children[0];
+    const dragOverTab = delegate.children[1];
+    const dragDetails = {
+      bubbles: true,
+      composed: true,
+      clientX: 100,
+      clientY: 150,
+      dataTransfer: new MockDataTransfer(),
+    };
+    draggedTab.dispatchEvent(new DragEvent('dragstart', dragDetails));
+    dragOverTab.dispatchEvent(new DragEvent(
+        'dragover', Object.assign({}, dragDetails, {clientX: 200})));
+    draggedTab.dispatchEvent(new DragEvent('drop', dragDetails));
+
+    assertEquals(
+        0, testTabStripEmbedderProxy.getCallCount('showTabContextMenu'));
+  });
 });

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

@@ -398,19 +398,6 @@ suite('Tab', function() {
         tabElement.shadowRoot.querySelector('#dragImage'));
   });
 
-  test('has custom context menu', async () => {
-    let event = new Event('contextmenu');
-    event.clientX = 1;
-    event.clientY = 2;
-    tabElement.shadowRoot.querySelector('#tab').dispatchEvent(event);
-
-    const contextMenuArgs =
-        await testTabStripEmbedderProxy.whenCalled('showTabContextMenu');
-    assertEquals(contextMenuArgs[0], tabElement.tab.id);
-    assertEquals(contextMenuArgs[1], 1);
-    assertEquals(contextMenuArgs[2], 2);
-  });
-
   test('activating closes WebUI container', () => {
     assertEquals(testTabStripEmbedderProxy.getCallCount('closeContainer'), 0);
     tabElement.shadowRoot.querySelector('#tab').click();