[NTP] Fix reordering when an add button is present and not at max shortcuts

Attempting to convert an non-existent 'index' attribute to a number
returns 0. Since 'index' is not set on the add button, this will cause
it to be moved in a weird direction.

Ignore the add button when shifting tiles, and throw an error if no
index is found.

Screencast: https://drive.google.com/open?id=1zDpvi34Ng4kte8vHYcKCPne0OEmfRctf

Bug: 961200
Change-Id: I78bf0b803ad6cc90b3913a3ca382b891320c95b5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1601647Reviewed-by: Kyle Milka <kmilka@chromium.org>
Commit-Queue: Kristi Park <kristipark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#658293}

chrome/browser/resources/local_ntp/most_visited_single.js

@@ -555,8 +555,9 @@ class Grid {
     // Shift tiles according to the new order.
     for (let i = 0; i < this.tiles_.length; i++) {
       const tileIndex = this.order_[i];
-      // Don't move the tile we're holding.
-      if (tileIndex === this.itemToReorder_) {
+      // Don't move the tile we're holding nor the add shortcut button.
+      if (tileIndex === this.itemToReorder_ ||
+          this.tiles_[i].getAttribute('add') === 'true') {
         continue;
       }
       this.applyReorder_(this.tiles_[tileIndex], i);
@@ -573,6 +574,9 @@ class Grid {
    * @private
    */
   applyReorder_(tile, newIndex) {
+    if (tile.getAttribute('index') === null) {
+      throw new Error('Tile does not have an index.');
+    }
     const index = Number(tile.getAttribute('index'));
     const x = this.position_[newIndex].x - this.position_[index].x;
     const y = this.position_[newIndex].y - this.position_[index].y;

chrome/test/data/local_ntp/most_visited_browsertest.js

@@ -779,6 +779,72 @@ test.mostVisited.testReorderInsertRtl = function() {
 };
 
 
+/**
+ * Tests if the tiles are translated properly when reordering with an add
+ * shortcut button present.
+ */
+test.mostVisited.testReorderInsertWithAddButton = function() {
+  // Set the window so that there's 10px padding around the grid.
+  window.innerHeight = 40;
+  window.innerWidth = 50;
+  const params = {  // Used to override the default grid parameters.
+    tileHeight: 10,
+    tileWidth: 10,
+    maxTilesPerRow: 3,
+    maxTiles: 6,
+    enableReorder: true
+  };
+
+  // Create a grid with uneven rows.
+  let container = document.createElement('div');
+  $(test.mostVisited.MOST_VISITED).appendChild(container);
+  test.mostVisited.initGridWithAdd(container, params, 5);
+
+  const dragStart = new Event('dragstart');
+  const mouseOver = new Event('mouseover');
+  const mouseUp = new Event('mouseup');
+
+  const tiles = document.getElementsByClassName(
+      test.mostVisited.CLASSES.GRID_TILE_CONTAINER);
+
+  // Start the reorder flow on the second tile.
+
+  let tile = tiles[1];
+  tile.firstChild.dispatchEvent(dragStart);
+
+  // Mouseover the first tile. This should shift tiles as if the held tile was
+  // inserted before.
+  let expectedLayout = [
+    'translate(10px, 0px)', '', 'translate(0px, 0px)', 'translate(0px, 0px)', ''
+  ];
+  tiles[0].dispatchEvent(mouseOver);
+
+  test.mostVisited.assertReorderInsert(container, expectedLayout, 3);
+
+  // Mouseover the last tile (the add shortcut button). This should not shift
+  // the tiles.
+  tiles[4].dispatchEvent(mouseOver);
+
+  test.mostVisited.assertReorderInsert(container, expectedLayout, 0);
+
+  // Mouseover the second to last tile. This should shift tiles as if the held
+  // tile was inserted after.
+  expectedLayout = [
+    'translate(0px, 0px)', '', 'translate(-10px, 0px)',
+    'translate(15px, -10px)', ''
+  ];
+  tiles[3].dispatchEvent(mouseOver);
+
+  test.mostVisited.assertReorderInsert(container, expectedLayout, 3);
+
+  // Stop the reorder flow.
+  document.dispatchEvent(mouseUp);
+  // Check that the correct values were sent to the EmbeddedSearchAPI.
+  assertEquals(1, test.mostVisited.reorderRid);
+  assertEquals(3, test.mostVisited.reorderNewIndex);
+};
+
+
 // ***************************** HELPER FUNCTIONS *****************************
 // These are used by the tests above.