Revert "WebUI Tab Strip: Animate tabs when moved"

This reverts commit dc9187da.

Reason for revert: Causing failures on Network Service Linux:

https://ci.chromium.org/p/chromium/builders/ci/Network%20Service%20Linux/6688

Mocha test failed: TabList PlaceTabElementAnimatesTabMoved
AssertionError: expected 2 to equal 1

Original change's description:
> WebUI Tab Strip: Animate tabs when moved
> 
> This CL adds animation when an ungrouped, unpinned tab is moved, to
> mostly facilitate the drag and drop experience. When the TabList's
> placeTabElement is called, the method first determines if the
> placement is causing a TabElement to move, then identifies which
> elements the TabElement is being dragged across, and finally
> animates the necessary distances for each TabElement.
> 
> Bug: 1082344
> Change-Id: Ia0b55f4c8539429969197258d148ce236b29e7e2
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2215248
> Commit-Queue: John Lee <johntlee@chromium.org>
> Reviewed-by: dpapad <dpapad@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#774469}

TBR=dpapad@chromium.org,johntlee@chromium.org

Change-Id: Id1e22487b0aa2e48ba31edabf0ffe4554b2c0a63
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1082344
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2228370Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Cr-Commit-Position: refs/heads/master@{#774498}

chrome/browser/resources/tab_strip/drag_manager.js

@@ -388,9 +388,8 @@ class DragSession {
     const dragOverTabElement =
         /** @type {?TabElement} */ (composedPath.find(isTabElement));
     if (dragOverTabElement &&
-        (dragOverTabElement.tab.pinned !== tabElement.tab.pinned ||
-         !dragOverTabElement.isValidDragOverTarget)) {
-      // Can only drag between the same pinned states and valid TabElements.
+        dragOverTabElement.tab.pinned !== tabElement.tab.pinned) {
+      // Can only drag between the same pinned states.
       return;
     }
 

chrome/browser/resources/tab_strip/tab.js

@@ -92,14 +92,6 @@ export class TabElement extends CustomElement {
     /** @private {!HTMLElement} */
     this.titleTextEl_ = /** @type {!HTMLElement} */ (this.$('#titleText'));
 
-    /**
-     * Flag indicating if this TabElement can accept dragover events. This
-     * is used to pause dragover events while animating as animating causes
-     * the elements below the pointer to shift.
-     * @private {boolean}
-     */
-    this.isValidDragOverTarget_ = true;
-
     this.tabEl_.addEventListener('click', () => this.onClick_());
     this.tabEl_.addEventListener('contextmenu', e => this.onContextMenu_(e));
     this.tabEl_.addEventListener(
@@ -177,16 +169,6 @@ export class TabElement extends CustomElement {
     this.tab_ = Object.freeze(tab);
   }
 
-  /** @return {boolean} */
-  get isValidDragOverTarget() {
-    return !this.hasAttribute('dragging_') && this.isValidDragOverTarget_;
-  }
-
-  /** @param {boolean} isValid */
-  set isValidDragOverTarget(isValid) {
-    this.isValidDragOverTarget_ = isValid;
-  }
-
   /** @param {!Function} callback */
   set onTabActivating(callback) {
     this.onTabActivating_ = callback;

chrome/browser/resources/tab_strip/tab_list.js

@@ -47,56 +47,6 @@ const LayoutVariable = {
   TAB_WIDTH: '--tabstrip-tab-thumbnail-width',
 };
 
-/**
- * Animates a series of elements to indicate that tabs have moved position.
- * @param {!Element} movedElement
- * @param {?Element} elementsToAnimateStart
- * @param {?Element} elementsToAnimateEnd
- * @param {number} direction, +1 if moving right, -1 if moving left
- */
-function animateTabElementMoved(
-    movedElement, elementsToAnimateStart, elementsToAnimateEnd, direction) {
-  let elementToAnimate = elementsToAnimateStart;
-  let numOfTabs = 0;
-
-  // Loop through every element from elementsToAnimateStart to
-  // elementsToAnimateEnd and animate each of them.
-  while (elementToAnimate && elementsToAnimateEnd &&
-         elementToAnimate !== elementsToAnimateEnd.nextElementSibling) {
-    slideElement(elementToAnimate, direction);
-    elementToAnimate = elementToAnimate.nextElementSibling;
-    numOfTabs++;
-  }
-
-  // Animate the moved TabElement itself the total number of tabs that it
-  // has been moved across.
-  slideElement(movedElement, -1 * direction * numOfTabs);
-}
-
-/**
- * Animates the horizontal slide of an element across the tab strip.
- * @param {!Element} element
- * @param {number} scale
- */
-function slideElement(element, scale) {
-  element.isValidDragOverTarget = false;
-  const animation = element.animate(
-      [
-        {
-          transform: 'translateX(calc(' + scale + ' ' +
-              '* (var(--tabstrip-tab-width) + var(--tabstrip-tab-spacing))))',
-        },
-        {transform: 'translateX(0)'},
-      ],
-      {
-        duration: 120,
-        easing: 'ease-out',
-      });
-  animation.onfinish = () => {
-    element.isValidDragOverTarget = true;
-  };
-}
-
 /** @implements {DragManagerDelegate} */
 class TabListElement extends CustomElement {
   static get template() {
@@ -688,33 +638,49 @@ class TabListElement extends CustomElement {
   placeTabElement(element, index, pinned, groupId) {
     const isInserting = !element.isConnected;
 
-    // TODO(johntlee): Animate pinned tabs and grouped tabs.
-    const shouldAnimate = !pinned && !groupId && !isInserting;
+    // Remove the element if it already exists in the DOM.
+    element.remove();
 
-    // Cache the previous and next element siblings as these will be needed
-    // after the placement to determine which tabs to animate.
-    let initialDomPrevSibling = null, initialDomNextSibling = null;
-    if (!isInserting) {
-      initialDomPrevSibling = element.previousElementSibling;
-      initialDomNextSibling = element.nextElementSibling;
-    }
+    if (pinned) {
+      this.pinnedTabsElement_.insertBefore(
+          element, this.pinnedTabsElement_.childNodes[index]);
+    } else {
+      let elementToInsert = element;
+      let elementAtIndex = this.$all('tabstrip-tab').item(index);
+      let parentElement = this.unpinnedTabsElement_;
+
+      if (groupId) {
+        let tabGroupElement = this.findTabGroupElement_(groupId);
+        if (tabGroupElement) {
+          // If a TabGroupElement already exists, add the TabElement to it.
+          parentElement = tabGroupElement;
+        } else {
+          // If a TabGroupElement does not exist, create one and add the
+          // TabGroupElement into the DOM.
+          tabGroupElement = document.createElement('tabstrip-tab-group');
+          tabGroupElement.setAttribute('data-group-id', groupId);
+          tabGroupElement.appendChild(element);
+          elementToInsert = tabGroupElement;
+        }
+      }
+
+      if (elementAtIndex && elementAtIndex.parentElement &&
+          isTabGroupElement(elementAtIndex.parentElement) &&
+          (elementAtIndex.previousElementSibling === null &&
+           elementAtIndex.tab.groupId !== groupId)) {
+        // If the element at the model index is in a group, and the group is
+        // different from the new tab's group, and is the first element in its
+        // group, insert the new element before its TabGroupElement. If a
+        // TabElement is being sandwiched between two TabElements in a group, it
+        // can be assumed that the tab will eventually be inserted into the
+        // group as well.
+        elementAtIndex = elementAtIndex.parentElement;
+      }
 
-    this.updateTabElementDomPosition_(element, index, pinned, groupId);
-
-    if (shouldAnimate) {
-      if (initialDomNextSibling &&
-          element.compareDocumentPosition(initialDomNextSibling) &
-              Node.DOCUMENT_POSITION_PRECEDING) {
-        // Element has moved right.
-        animateTabElementMoved(
-            element, initialDomNextSibling, element.previousElementSibling, 1);
-      } else if (
-          initialDomPrevSibling &&
-          element.compareDocumentPosition(initialDomPrevSibling) &
-              Node.DOCUMENT_POSITION_FOLLOWING) {
-        // Element has moved left.
-        animateTabElementMoved(
-            element, element.nextElementSibling, initialDomPrevSibling, -1);
+      if (elementAtIndex && elementAtIndex.parentElement === parentElement) {
+        parentElement.insertBefore(elementToInsert, elementAtIndex);
+      } else {
+        parentElement.appendChild(elementToInsert);
       }
     }
 
@@ -808,63 +774,6 @@ class TabListElement extends CustomElement {
     }
   }
 
-  /**
-   * @param {!TabElement} element
-   * @param {number} index
-   * @param {boolean} pinned
-   * @param {string=} groupId
-   * @private
-   */
-  updateTabElementDomPosition_(element, index, pinned, groupId) {
-    // Remove the element if it already exists in the DOM. This simplifies
-    // the way indices work as it does not have to count its old index in
-    // the initial layout of the DOM.
-    element.remove();
-
-    if (pinned) {
-      this.pinnedTabsElement_.insertBefore(
-          element, this.pinnedTabsElement_.childNodes[index]);
-    } else {
-      let elementToInsert = element;
-      let elementAtIndex = this.$all('tabstrip-tab').item(index);
-      let parentElement = this.unpinnedTabsElement_;
-
-      if (groupId) {
-        let tabGroupElement = this.findTabGroupElement_(groupId);
-        if (tabGroupElement) {
-          // If a TabGroupElement already exists, add the TabElement to it.
-          parentElement = tabGroupElement;
-        } else {
-          // If a TabGroupElement does not exist, create one and add the
-          // TabGroupElement into the DOM.
-          tabGroupElement = document.createElement('tabstrip-tab-group');
-          tabGroupElement.setAttribute('data-group-id', groupId);
-          tabGroupElement.appendChild(element);
-          elementToInsert = tabGroupElement;
-        }
-      }
-
-      if (elementAtIndex && elementAtIndex.parentElement &&
-          isTabGroupElement(elementAtIndex.parentElement) &&
-          (elementAtIndex.previousElementSibling === null &&
-           elementAtIndex.tab.groupId !== groupId)) {
-        // If the element at the model index is in a group, and the group is
-        // different from the new tab's group, and is the first element in its
-        // group, insert the new element before its TabGroupElement. If a
-        // TabElement is being sandwiched between two TabElements in a group, it
-        // can be assumed that the tab will eventually be inserted into the
-        // group as well.
-        elementAtIndex = elementAtIndex.parentElement;
-      }
-
-      if (elementAtIndex && elementAtIndex.parentElement === parentElement) {
-        parentElement.insertBefore(elementToInsert, elementAtIndex);
-      } else {
-        parentElement.appendChild(elementToInsert);
-      }
-    }
-  }
-
   /**
    * @param {!TabElement} tabElement
    * @private

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

@@ -568,35 +568,4 @@ suite('DragManager', () => {
     assertFalse(
         !!delegate.querySelector(`[data-tab-id="${PLACEHOLDER_TAB_ID}"]`));
   });
-
-  test('DragOverInvalidDragOverTarget', () => {
-    const draggedIndex = 0;
-    const dragOverIndex = 1;
-    const draggedTab = delegate.children[draggedIndex];
-    const dragOverTab = delegate.children[dragOverIndex];
-    const mockDataTransfer = new MockDataTransfer();
-
-    // Dispatch a dragstart event to start the drag process.
-    const dragStartEvent = new DragEvent('dragstart', {
-      bubbles: true,
-      composed: true,
-      clientX: 100,
-      clientY: 150,
-      dataTransfer: mockDataTransfer,
-    });
-    draggedTab.dispatchEvent(dragStartEvent);
-
-    // Mark the dragOverIndex tab to be an invalid dragover target.
-    dragOverTab.isValidDragOverTarget = false;
-    const dragOverEvent = new DragEvent('dragover', {
-      bubbles: true,
-      composed: true,
-      dataTransfer: mockDataTransfer,
-    });
-    dragOverTab.dispatchEvent(dragOverEvent);
-
-    // Dragover tab and dragged tab remain in their initial positions.
-    assertEquals(draggedTab, delegate.children[draggedIndex]);
-    assertEquals(dragOverTab, delegate.children[dragOverIndex]);
-  });
 });

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

@@ -242,52 +242,6 @@ suite('TabList', () => {
     assertEquals('TABSTRIP-TAB-GROUP', groupedTab.parentElement.tagName);
   });
 
-  test('PlaceTabElementAnimatesTabMoved', async () => {
-    await tabList.animationPromises;
-    let unpinnedTabs = getUnpinnedTabs();
-
-    function testAnimationParams(element, scale) {
-      const animations = element.getAnimations();
-      assertEquals(1, animations.length);
-      assertEquals('running', animations[0].playState);
-      assertEquals(120, animations[0].effect.getTiming().duration);
-      assertEquals('ease-out', animations[0].effect.getTiming().easing);
-
-      const keyframes = animations[0].effect.getKeyframes();
-      const tabSpacingVars =
-          '(var(--tabstrip-tab-width) + var(--tabstrip-tab-spacing))';
-      assertEquals(2, keyframes.length);
-      assertEquals(
-          `translateX(calc(${scale} * ${tabSpacingVars}))`,
-          keyframes[0].transform);
-      assertEquals('translateX(0px)', keyframes[1].transform);
-
-      animations[0].finish();
-    }
-
-    // Move the last tab to the first tab to test right to left animations.
-    const movedTab = unpinnedTabs[unpinnedTabs.length - 1];
-    tabList.placeTabElement(movedTab, 0, false, undefined);
-    testAnimationParams(movedTab, unpinnedTabs.length - 1);
-
-    // All other tabs should animate a space of 1 TabElement to the right.
-    Array.from(unpinnedTabs)
-        .filter(tabElement => tabElement !== movedTab)
-        .forEach(tabElement => testAnimationParams(tabElement, -1));
-
-    // Move the first tab to the last tab to test left to right animations.
-    unpinnedTabs = getUnpinnedTabs();
-    const movedTab2 = unpinnedTabs[0];
-    tabList.placeTabElement(
-        movedTab2, unpinnedTabs.length - 1, false, undefined);
-    testAnimationParams(movedTab2, -1 * (unpinnedTabs.length - 1));
-
-    // All other tabs should animate a space of 1 TabElement to the left.
-    Array.from(unpinnedTabs)
-        .filter(tabElement => tabElement !== movedTab2)
-        .forEach(tabElement => testAnimationParams(tabElement, 1));
-  });
-
   test('PlacesTabGroupElement', () => {
     const tabGroupElement = document.createElement('tabstrip-tab-group');
     tabList.placeTabGroupElement(tabGroupElement, 2);