Enable new-files-app-navigation by default

This CL reverses the value of the feature flag to mean enabled, and
fixes all tests to pass with the new My Files on navigation menu.

Change MyFiles tests to not append the flag anymore, because when the
flag is provided it now disables "My Files" :)

Fix navigation_list_model_unittest.js to pass flag true/false on
NavigationListModel with the desired flag state.

Stop calling "notifyEntriesChanged" on
DirectoryTree.updateSharedStatusIcon, because this causes the cache
invalidation, which was causing some metadata model to be invalidated
before being displayed on file list, in particular the modificationTime
column on file list. crbug.com/857343 tracks further investigation if
this call was really still needed.

Change folder_shortcuts.js to account to new My Files on directory
tree, because it changes the keyboard shortcut to select top-level
directories. Also fix and improve some comments.

Change tab_index.js to account to My Files on breadcrumbs being focused
during tab cycle. This requires adding an id on breadcrumbs.

Bug: 857335, 862897
Cq-Include-Trybots: luci.chromium.try:closure_compilation
Change-Id: Iddb2545772e62c0e8adebd8b278bd3a5765f5040
Reviewed-on: https://chromium-review.googlesource.com/1131025Reviewed-by: Noel Gordon <noel@chromium.org>
Commit-Queue: Luciano Pacheco <lucmult@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574939}

chrome/browser/chromeos/file_manager/file_manager_browsertest.cc

@@ -42,17 +42,11 @@ struct TestCase {
     return *this;
   }
 
-  TestCase& EnableMyFiles() {
-    enable_new_navigation = true;
-    return *this;
-  }
-
   const char* test_case_name = nullptr;
   GuestMode guest_mode = NOT_IN_GUEST_MODE;
   bool trusted_events = false;
   bool tablet_mode = false;
   bool enable_drivefs = false;
-  bool enable_new_navigation = false;
 };
 
 // EventCase: FilesAppBrowserTest with trusted JS Events.
@@ -83,11 +77,6 @@ class FilesAppBrowserTest : public FileManagerBrowserTestBase,
     if (GetParam().tablet_mode) {
       command_line->AppendSwitchASCII("force-tablet-mode", "touch_view");
     }
-
-    // If requested, enable the new-files-app-navigation flag.
-    if (GetParam().enable_new_navigation) {
-      command_line->AppendSwitchASCII("new-files-app-navigation", "");
-    }
   }
 
   GuestMode GetGuestMode() const override { return GetParam().guest_mode; }
@@ -133,9 +122,6 @@ std::string PostTestCaseName(const ::testing::TestParamInfo<TestCase>& test) {
   if (test.param.enable_drivefs)
     name.append("_DriveFs");
 
-  if (test.param.enable_new_navigation)
-    name.append("_MyFiles");
-
   return name;
 }
 
@@ -494,10 +480,10 @@ WRAPPED_INSTANTIATE_TEST_CASE_P(
     MyFiles, /* my_files.js */
     FilesAppBrowserTest,
     ::testing::Values(
-        TestCase("showMyFiles").EnableMyFiles(),
-        TestCase("hideSearchButton").EnableMyFiles(),
-        TestCase("myFilesDisplaysAndOpensEntries").EnableMyFiles(),
-        TestCase("directoryTreeRefresh").EnableMyFiles()));
+        TestCase("showMyFiles"),
+        TestCase("hideSearchButton"),
+        TestCase("myFilesDisplaysAndOpensEntries"),
+        TestCase("directoryTreeRefresh")));
 
 // Structure to describe an account info.
 struct TestAccountInfo {

ui/file_manager/file_manager/foreground/js/navigation_list_model.js

@@ -278,7 +278,9 @@ function NavigationListModel(
   }
 
   // True if the flag new-files-app-navigation is enabled.
-  this.useNewNavigation_ = !!opt_useNewNavigation;
+  // Yes the member name is the opposite meaning.
+  // TODO(https://crbug.com/850348): Remove this flag.
+  this.disableNewNavigation_ = !!opt_useNewNavigation;
 
   // Reorder volumes, shortcuts, and optional items for initial display.
   this.reorderNavigationItems_();
@@ -418,7 +420,7 @@ NavigationListModel.prototype = {
  * it's disabled it has a flat structure with Linux Files after Recent menu.
  */
 NavigationListModel.prototype.reorderNavigationItems_ = function() {
-  if (this.useNewNavigation_) {
+  if (!this.disableNewNavigation_) {
     return this.orderAndNestItems_();
   } else {
     return this.flatNavigationItems_();

ui/file_manager/file_manager/foreground/js/navigation_list_model_unittest.js

@@ -45,7 +45,7 @@ function testModel() {
   var addNewServicesItem = new NavigationModelMenuItem(
       'menu-button-label', '#add-new-services', 'menu-button-icon');
   var model = new NavigationListModel(
-      volumeManager, shortcutListModel, recentItem, addNewServicesItem);
+      volumeManager, shortcutListModel, recentItem, addNewServicesItem, true);
   model.linuxFilesItem = new NavigationModelFakeItem(
       'linux-files-label', NavigationModelItemType.CROSTINI,
       {toURL: () => 'fake-entry://linux-files'});
@@ -69,7 +69,7 @@ function testNoRecentOrLinuxFiles() {
   var addNewServicesItem = new NavigationModelMenuItem(
       'menu-button-label', '#add-new-services', 'menu-button-icon');
   var model = new NavigationListModel(
-      volumeManager, shortcutListModel, recentItem, addNewServicesItem);
+      volumeManager, shortcutListModel, recentItem, addNewServicesItem, true);
 
   assertEquals(4, model.length);
   assertEquals('drive', model.item(0).volumeInfo.volumeId);
@@ -87,7 +87,7 @@ function testAddAndRemoveShortcuts() {
   var recentItem = null;
   var addNewServicesItem = null;
   var model = new NavigationListModel(
-      volumeManager, shortcutListModel, recentItem, addNewServicesItem);
+      volumeManager, shortcutListModel, recentItem, addNewServicesItem, true);
 
   assertEquals(3, model.length);
 
@@ -122,7 +122,7 @@ function testAddAndRemoveVolumes() {
   var recentItem = null;
   var addNewServicesItem = null;
   var model = new NavigationListModel(
-      volumeManager, shortcutListModel, recentItem, addNewServicesItem);
+      volumeManager, shortcutListModel, recentItem, addNewServicesItem, true);
 
   assertEquals(3, model.length);
 
@@ -217,7 +217,7 @@ function testOrderAndNestItems() {
 
   // Constructor already calls orderAndNestItems_.
   const model = new NavigationListModel(
-      volumeManager, shortcutListModel, recentItem, addNewServicesItem, true);
+      volumeManager, shortcutListModel, recentItem, addNewServicesItem, false);
 
   // Check items order and that MTP/Archive/Removable respect the original
   // order.

ui/file_manager/file_manager/foreground/js/ui/directory_tree.js

@@ -236,7 +236,6 @@ DirectoryItem.prototype.updateSubElementsFromList = function(recursive) {
           if (currentElement.expanded) {
             currentElement.updateSubDirectories(true /* recursive */);
           }
-
           // Show the expander even without knowing if there are children.
           currentElement.mayHaveChildren_ = true;
         } else {
@@ -613,7 +612,11 @@ SubDirectoryItem.prototype = {
  */
 SubDirectoryItem.prototype.updateSharedStatusIcon = function() {
   var icon = this.querySelector('.icon');
-  this.parentTree_.metadataModel.notifyEntriesChanged([this.dirEntry_]);
+  // TODO(crbug.com/857343): Evaluate if this can be fully removed.
+  // This line invalidates the metadata model cache and was causing some
+  // directories to not display modificationTime which comes from metadata
+  // because it invalidated before displaying it.
+  // this.parentTree_.metadataModel.notifyEntriesChanged([this.dirEntry_]);
   this.parentTree_.metadataModel.get([this.dirEntry_], ['shared']).then(
       function(metadata) {
         icon.classList.toggle('shared', !!(metadata[0] && metadata[0].shared));

ui/file_manager/file_manager/foreground/js/ui/location_line.js

@@ -150,6 +150,7 @@ LocationLine.prototype.update_ = function(components) {
     // Add a component.
     var component = components[i];
     var button = document.createElement('button');
+    button.id = 'breadcrumb-path-' + i;
     button.classList.add(
         'breadcrumb-path', 'entry-name', 'imitate-paper-button');
     var nameElement = document.createElement('div');

ui/file_manager/integration_tests/file_manager/folder_shortcuts.js

@@ -294,39 +294,38 @@ testcase.traverseFolderShortcuts = function() {
       expectSelection(
           windowId, DIRECTORY.Drive, DIRECTORY.Drive).then(this.next);
     },
-
-    // Press Ctrl+6 to select 5th shortcut.
-    // Current directory should be D.
-    // Shortcut to C should be selected.
+    // Press Ctrl+3 to select 3d shortcut.
     function() {
       remoteCall.callRemoteTestUtil(
-          'fakeKeyDown', windowId,
-          ['#file-list', '6', 'U+0034', true, false, false], this.next);
+          'fakeKeyDown', windowId, ['#file-list', '3', '3', true, false, false],
+          this.next);
     },
+    // Current directory should be D.
     function(result) {
       chrome.test.assertTrue(result);
       expectSelection(windowId, DIRECTORY.D, DIRECTORY.D).then(this.next);
     },
-
-    // Press UP to select 4th shortcut.
+    // Press UP to select shortcut (above D).
     // Current directory should remain D.
-    // Shortcut to C should be selected.
+    // But Shortcut to C should be selected.
     function() {
       remoteCall.callRemoteTestUtil('fakeKeyDown', windowId,
           ['#directory-tree', 'ArrowUp', 'Up', false, false, false], this.next);
     },
+    // Current directory should remain D.
     function(result) {
       chrome.test.assertTrue(result);
       expectSelection(windowId, DIRECTORY.D, DIRECTORY.C).then(this.next);
     },
-
     // Press Enter to change the directory to C.
-    // Current directory should be C.
+    // Then current directory should change to C.
     function() {
       remoteCall.callRemoteTestUtil('fakeKeyDown', windowId,
           ['#directory-tree', 'Enter', 'Enter', false, false, false],
            this.next);
     },
+
+    // Current directory should be C.
     function(result) {
       chrome.test.assertTrue(result);
       expectSelection(windowId, DIRECTORY.C, DIRECTORY.C).then(this.next);

ui/file_manager/integration_tests/file_manager/tab_index.js

@@ -116,12 +116,13 @@ testcase.tabindexFocusDownloads = function() {
     function(results) {
       appId = results.windowId;
       remoteCall.waitForElement(appId, ['#file-list:focus']).then(this.next);
-    },
-    // Press the Tab key.
-    function(element) {
+    }, function(element) {
       remoteCall.callRemoteTestUtil('getActiveElement', appId, [], this.next);
     }, function(element) {
       chrome.test.assertEq('list', element.attributes['class']);
+      remoteCall.checkNextTabFocus(appId, 'breadcrumb-path-0').then(this.next);
+    }, function(result) {
+      chrome.test.assertTrue(result);
       remoteCall.checkNextTabFocus(appId, 'search-button').then(this.next);
     }, function(result) {
       chrome.test.assertTrue(result);
@@ -309,8 +310,9 @@ testcase.tabindexOpenDialogDownloads = function() {
             'selectFile', appId, ['hello.txt']);
       },
       ['#ok-button:not([disabled])'],
-      ['ok-button', 'cancel-button', 'search-button', 'view-button',
-       'sort-button', 'gear-button', 'directory-tree', 'file-list']));
+      ['ok-button', 'cancel-button', 'breadcrumb-path-0', 'search-button',
+      'view-button', 'sort-button', 'gear-button', 'directory-tree',
+      'file-list']));
 };
 
 /**
@@ -339,9 +341,9 @@ testcase.tabindexSaveFileDialogDownloads = function() {
       },
       'downloads', BASIC_LOCAL_ENTRY_SET, null,
       ['#ok-button:not([disabled])'],
-      ['ok-button', 'cancel-button', 'search-button', 'view-button',
-       'sort-button', 'gear-button', 'directory-tree', 'file-list',
-       'new-folder-button', 'filename-input-textbox']));
+      ['ok-button', 'cancel-button', 'breadcrumb-path-0', 'search-button',
+      'view-button', 'sort-button', 'gear-button', 'directory-tree',
+      'file-list', 'new-folder-button', 'filename-input-textbox']));
 };
 
 /**