Change directory if the active list item on navigation list is changed.

We want to change current directory if the active item on navigation list is changed.
But we don't want to change current directory when listed items are spliced or permuted.
In both cases, 'change' event is dispatched from selection model and we can't distinguish these cases from event itself.
currentActiveItem_ is introduced to detect change of actual active item.

BUG=375663
TEST=go through steps in Issue 375663 and 361047

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=273601

Review URL: https://codereview.chromium.org/303503004

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

chrome/browser/chromeos/file_manager/file_manager_browsertest.cc

@@ -900,6 +900,29 @@ INSTANTIATE_TEST_CASE_P(
     ::testing::Values(TestParameter(NOT_IN_GUEST_MODE,
                                     "traverseNavigationList")));
 
+// Unlike TEST/TEST_F, which are macros that expand to further macros,
+// INSTANTIATE_TEST_CASE_P is a macro that expands directly to code that
+// stringizes the arguments. As a result, macros passed as parameters (such as
+// prefix or test_case_name) will not be expanded by the preprocessor. To work
+// around this, indirect the macro for INSTANTIATE_TEST_CASE_P, so that the
+// pre-processor will expand macros such as MAYBE_test_name before
+// instantiating the test.
+#define WRAPPED_INSTANTIATE_TEST_CASE_P(prefix, test_case_name, generator) \
+    INSTANTIATE_TEST_CASE_P(prefix, test_case_name, generator)
+
+// This test is too slow to run in debug build. http://crbug.com/327719
+#if !defined(NDEBUG)
+#define MAYBE_FolderShortcuts DISABLED_FolderShortcuts
+#else
+#define MAYBE_FolderShortcuts FolderShortcuts
+#endif
+WRAPPED_INSTANTIATE_TEST_CASE_P(
+    MAYBE_FolderShortcuts,
+    FileManagerBrowserTest,
+    ::testing::Values(
+        TestParameter(NOT_IN_GUEST_MODE, "traverseFolderShortcuts"),
+        TestParameter(NOT_IN_GUEST_MODE, "addRemoveFolderShortcuts")));
+
 INSTANTIATE_TEST_CASE_P(
     TabIndex,
     FileManagerBrowserTest,

chrome/test/data/extensions/api_test/file_manager_browsertest/file_manager_test_manifest.json

@@ -11,17 +11,18 @@
       // namespace is defined here.
       "test_util.js",
       "file_manager/background.js",
+      "file_manager/copy_between_windows.js",
       "file_manager/create_new_folder.js",
       "file_manager/drive_specific.js",
-      "file_manager/copy_between_windows.js",
       "file_manager/execute_default_task.js",
       "file_manager/file_display.js",
+      "file_manager/folder_shortcuts.js",
       "file_manager/keyboard_operations.js",
       "file_manager/multi_profile.js",
       "file_manager/navigation_list.js",
       "file_manager/open_audio_files.js",
-      "file_manager/open_zip_files.js",
       "file_manager/open_video_files.js",
+      "file_manager/open_zip_files.js",
       "file_manager/restore_geometry.js",
       "file_manager/restore_prefs.js",
       "file_manager/share_dialog.js",

chrome/test/data/extensions/api_test/file_manager_browsertest/test_util.js

@@ -263,6 +263,21 @@ var ENTRIES = {
       null, SharedOption.NONE, 'Jan 1, 2000 1:00 AM',
       'C', '--', 'Folder'),
 
+  directoryD: new TestEntryInfo(
+      EntryType.DIRECTORY, null, 'D',
+      null, SharedOption.NONE, 'Jan 1, 2000 1:00 AM',
+      'D', '--', 'Folder'),
+
+  directoryE: new TestEntryInfo(
+      EntryType.DIRECTORY, null, 'D/E',
+      null, SharedOption.NONE, 'Jan 1, 2000 1:00 AM',
+      'E', '--', 'Folder'),
+
+  directoryF: new TestEntryInfo(
+      EntryType.DIRECTORY, null, 'D/E/F',
+      null, SharedOption.NONE, 'Jan 1, 2000 1:00 AM',
+      'F', '--', 'Folder'),
+
   zipArchive: new TestEntryInfo(
       EntryType.FILE, 'archive.zip', 'archive.zip',
       'application/x-zip', SharedOption.NONE, 'Jan 1, 2014 1:00 AM',

ui/file_manager/file_manager/background/js/test_util.js

@@ -409,6 +409,24 @@ test.util.sync.fakeMouseClick = function(
   return resultMouseOver && resultMouseDown && resultMouseUp && resultClick;
 };
 
+/**
+ * Simulates a fake mouse click (right button, single click) on the element
+ * specified by |targetQuery|.
+ *
+ * @param {Window} contentWindow Window to be tested.
+ * @param {string} targetQuery Query to specify the element.
+ * @param {string=} opt_iframeQuery Optional iframe selector.
+ * @return {boolean} True if the event is sent to the target, false
+ *     otherwise.
+ */
+test.util.sync.fakeMouseRightClick = function(
+    contentWindow, targetQuery, opt_iframeQuery) {
+  var contextMenuEvent = new MouseEvent('contextmenu', {bubbles: true});
+  var result = test.util.sync.sendEvent(
+      contentWindow, targetQuery, contextMenuEvent, opt_iframeQuery);
+  return result;
+};
+
 /**
  * Simulates a fake double click event (left button) to the element specified by
  * |targetQuery|.

ui/file_manager/file_manager/foreground/js/ui/navigation_list.js

@@ -201,6 +201,9 @@ NavigationList.prototype.decorate = function(volumeManager, directoryModel) {
   this.scrollBar_ = new ScrollBar();
   this.scrollBar_.initialize(this.parentNode, this);
 
+  // Keeps track of selected model item to detect if it is changed actually.
+  this.currentModelItem_ = null;
+
   // Overriding default role 'list' set by cr.ui.List.decorate() to 'listbox'
   // role for better accessibility on ChromeOS.
   this.setAttribute('role', 'listbox');
@@ -287,6 +290,7 @@ NavigationList.prototype.selectByIndex = function(index) {
   if (index < 0 || index > this.dataModel.length - 1)
     return false;
 
+  this.selectionModel.selectedIndex = index;
   this.activateModelItem_(this.dataModel.item(index));
   return true;
 };
@@ -299,16 +303,11 @@ NavigationList.prototype.selectByIndex = function(index) {
  */
 NavigationList.prototype.activateModelItem_ = function(modelItem) {
   var onEntryResolved = function(entry) {
-    // If the root item of active directory was same as newly activated
-    // root item, keep the active directory as it was.
-    var currentDir = this.directoryModel_.getCurrentDirEntry();
-    if (util.isSameEntry(entry, currentDir) ||
-        util.isDescendantEntry(entry, currentDir)) {
-      return;
+    // Changes directory to the model item's root directory if needed.
+    if (!util.isSameEntry(this.directoryModel_.getCurrentDirEntry(), entry)) {
+      metrics.recordUserAction('FolderShortcut.Navigate');
+      this.directoryModel_.changeDirectoryEntry(entry);
     }
-
-    metrics.recordUserAction('FolderShortcut.Navigate');
-    this.directoryModel_.changeDirectoryEntry(entry);
   }.bind(this);
 
   if (modelItem.isVolume) {
@@ -350,12 +349,25 @@ NavigationList.prototype.onBeforeSelectionChange_ = function(event) {
  * @private
  */
 NavigationList.prototype.onSelectionChange_ = function(event) {
+  var index = this.selectionModel.selectedIndex;
+  if (index < 0 || index > this.dataModel.length - 1)
+    return;
+
+  // If the selected model item is not changed actually, we don't change the
+  // current directory even if the selected index is changed.
+  var modelItem = this.dataModel.item(index);
+  if (modelItem === this.currentModelItem_)
+    return;
+
+  // Remembers the selected model item.
+  this.currentModelItem_ = modelItem;
+
   // This handler is invoked even when the navigation list itself changes the
   // selection. In such case, we shouldn't handle the event.
   if (this.dontHandleSelectionEvent_)
     return;
 
-  this.selectByIndex(this.selectionModel.selectedIndex);
+  this.activateModelItem_(modelItem);
 };
 
 /**