History: Fix selecting multiple visits.

Initially the visist IDs for the checkboxes were set as the visits were
retrieved but currently the visits can be displayed in a different order.

Fix this by setting the ID when the visit is displayed, fixing multiple
selection. Add a test to check that in the future.

BUG=170690

Review URL: https://chromiumcodereview.appspot.com/12217015

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@182470 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/resources/history/history.js

@@ -36,16 +36,17 @@ MenuButton.createDropDownArrows();
  * @param {boolean} continued Whether this visit is on the same day as the
  *     visit before it.
  * @param {HistoryModel} model The model object this entry belongs to.
- * @param {number} id The identifier for the entry.
  * @constructor
  */
-function Visit(result, continued, model, id) {
+function Visit(result, continued, model) {
   this.model_ = model;
   this.title_ = result.title;
   this.url_ = result.url;
   this.starred_ = result.starred;
   this.snippet_ = result.snippet || '';
-  this.id_ = id;
+  // The id will be set according to when the visit was displayed, not
+  // received. Set to -1 to show that it has not been set yet.
+  this.id_ = -1;
 
   this.isRendered = false;  // Has the visit already been rendered on the page?
 
@@ -103,6 +104,8 @@ Visit.prototype.getResultDOM = function(propertyBag) {
   dropDown.setAttribute('menu', '#action-menu');
   cr.ui.decorate(dropDown, MenuButton);
 
+  this.id_ = this.model_.nextVisitId_++;
+
   // Checkbox is always created, but only visible on hover & when checked.
   var checkbox = document.createElement('input');
   checkbox.type = 'checkbox';
@@ -436,7 +439,7 @@ HistoryModel.prototype.addResults = function(info, results) {
       visit.addDuplicateTimestamp(thisResult.timestamp);
       continue;
     }
-    visit = new Visit(thisResult, isSameDay, this, this.nextVisitId_++);
+    visit = new Visit(thisResult, isSameDay, this);
     this.urlsFromLastSeenDay_[thisResult.url] = visit;
     this.visits_.push(visit);
     this.changed = true;

chrome/test/data/webui/history_browsertest.js

@@ -49,6 +49,20 @@ function callFrontendAsync(functionName) {
   }, 1);
 }
 
+/**
+ * Checks that all the checkboxes in the [|start|, |end|] interval are checked
+ * and that their IDs are properly set. Does that against the checkboxes in
+ * |checked|, starting from the |startInChecked| position.
+ * @param {Array} checked An array of all the relevant checked checkboxes
+ *    on this page.
+ * @param {Number} start The starting checkbox id.
+ * @param {Number} end The ending checkbox id.
+ */
+function checkInterval(checked, start, end) {
+  for (var i = start; i <= end; i++)
+    expectEquals('checkbox-' + i, checked[i - start].id);
+}
+
 /**
  * Base fixture for History WebUI testing.
  * @extends {testing.Test}
@@ -316,3 +330,66 @@ TEST_F('HistoryWebUITest', 'deletion', function() {
     });
   });
 });
+
+/**
+ * Test selecting multiple entries using shift click.
+ */
+TEST_F('HistoryWebUITest', 'multipleSelect', function() {
+  var checkboxes = document.querySelectorAll(
+      '#results-display input[type=checkbox]');
+
+  var getAllChecked = function () {
+    return Array.prototype.slice.call(document.querySelectorAll(
+        '#results-display input[type=checkbox]:checked'));
+  }
+
+  // Make sure that nothing is checked.
+  expectEquals(0, getAllChecked().length);
+
+  var shiftClick = function(el) {
+    el.dispatchEvent(new MouseEvent('click', { shiftKey: true }));
+  };
+
+  // Check the start.
+  shiftClick($('checkbox-4'));
+  // And the end.
+  shiftClick($('checkbox-9'));
+
+  // See if they are checked.
+  var checked = getAllChecked();
+  expectEquals(6, checked.length);
+  checkInterval(checked, 4, 9);
+
+  // Extend the selection.
+  shiftClick($('checkbox-14'));
+
+  checked = getAllChecked();
+  expectEquals(11, checked.length);
+  checkInterval(checked, 4, 14);
+
+  // Now do a normal click on a higher ID box and a shift click on a lower ID
+  // one (test the other way around).
+  $('checkbox-24').click();
+  shiftClick($('checkbox-19'));
+
+  checked = getAllChecked();
+  expectEquals(17, checked.length);
+  // First set of checkboxes (11).
+  checkInterval(checked, 4, 14);
+  // Second set (6).
+  checkInterval(checked.slice(11), 19, 24);
+
+  // Test deselection.
+  $('checkbox-26').click();
+  shiftClick($('checkbox-20'));
+
+  checked = getAllChecked();
+  // checkbox-20 to checkbox-24 should be deselected now.
+  expectEquals(12, checked.length);
+  // First set of checkboxes (11).
+  checkInterval(checked, 4, 14);
+  // Only checkbox-19 should still be selected.
+  expectEquals('checkbox-19', checked[11].id);
+
+  testDone();
+});