Revert "Change NavigationListModel to support MyFiles as Volume."

This reverts commit 2903236d.

Reason for revert: ChromiumOS MSAN got flaky/failing crbug.com/899664

Original change's description:
> Change NavigationListModel to support MyFiles as Volume.
> 
> Add MYFILES_VOLUME_ENABLED in private strings to be able to have the
> feature flag in the JS/UI code.
> 
> Change VolumeManager to use MyFiles folder for Downloads volume, the
> method |GetMyFilesFolderForProfile| takes care of returning the right
> value based on the feature flag MyFilesVolume.
> 
> Change NavigationListModel to support MyFiles as Volume when
> MyFilesVolume feature flag is enabled:
> 1. MyFiles volume is the Downloads volume (after the feature flag is
> removed we can rename it accordingly). Setup Downloads volume as
> VolumeEntry,
> 2. Create a NavigationModelFakeItem with "My files" label, based on the
> VolumeEntry (step #1).
> 3. Skip adding "Downloads" to Myfiles, since now MyFiles is in fact
> Downloads volume.
> 4. Continue to add Linux and Play files to Myfiles.
> 
> Test: unit_tests --gtest_filter='VolumeManagerTest.GetVolumeListMyFilesVolume' and browser_test --gtest_filter='FileManagerJsTest.DirectoryTreeTest:FileManagerJsTest.NavigationList*'
> Bug: 873539
> Change-Id: Ia242d52a1e4d7b4fb3c7ca219d9cfdc4fce72543
> Reviewed-on: https://chromium-review.googlesource.com/c/1308235
> Commit-Queue: Luciano Pacheco <lucmult@chromium.org>
> Reviewed-by: Joel Hockey <joelhockey@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#604143}

TBR=joelhockey@chromium.org,lucmult@chromium.org,ioanap@chromium.org

Change-Id: I0c5b1fd5f23b2ec2b56ab13ab93a2d3a1ffd277a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 873539
Reviewed-on: https://chromium-review.googlesource.com/c/1309569Reviewed-by: Luciano Pacheco <lucmult@chromium.org>
Commit-Queue: Luciano Pacheco <lucmult@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604193}

chrome/browser/chromeos/extensions/file_manager/private_api_strings.cc

@@ -839,9 +839,6 @@ ExtensionFunction::ResponseAction FileManagerPrivateGetStringsFunction::Run() {
                    chromeos::DemoSession::IsDeviceInDemoMode());
   dict->SetBoolean("DRIVE_FS_ENABLED",
                    base::FeatureList::IsEnabled(chromeos::features::kDriveFs));
-  dict->SetBoolean(
-      "MY_FILES_VOLUME_ENABLED",
-      base::FeatureList::IsEnabled(chromeos::features::kMyFilesVolume));
   dict->SetString("CHROMEOS_RELEASE_BOARD",
                   base::SysInfo::GetLsbReleaseBoard());
   dict->SetString(

chrome/browser/chromeos/file_manager/volume_manager.cc

@@ -416,13 +416,14 @@ void VolumeManager::Initialize() {
     return;
   }
 
-  const base::FilePath localVolume =
-      file_manager::util::GetMyFilesFolderForProfile(profile_);
-  const bool success = RegisterDownloadsMountPoint(profile_, localVolume);
+  // Register 'Downloads' folder for the profile to the file system.
+  const base::FilePath downloads =
+      file_manager::util::GetDownloadsFolderForProfile(profile_);
+  const bool success = RegisterDownloadsMountPoint(profile_, downloads);
   DCHECK(success);
 
   DoMountEvent(chromeos::MOUNT_ERROR_NONE,
-               Volume::CreateForDownloads(localVolume));
+               Volume::CreateForDownloads(downloads));
 
   // Subscribe to DriveIntegrationService.
   drive_integration_service_->AddObserver(this);

chrome/browser/chromeos/file_manager/volume_manager_unittest.cc

@@ -14,17 +14,14 @@
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/strings/utf_string_conversions.h"
-#include "base/test/scoped_feature_list.h"
 #include "chrome/browser/chromeos/drive/file_system_util.h"
 #include "chrome/browser/chromeos/file_manager/fake_disk_mount_manager.h"
 #include "chrome/browser/chromeos/file_manager/volume_manager_observer.h"
 #include "chrome/browser/chromeos/file_system_provider/fake_extension_provider.h"
 #include "chrome/browser/chromeos/file_system_provider/service.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
-#include "chrome/browser/chromeos/scoped_set_running_on_chromeos_for_testing.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/testing_profile.h"
-#include "chromeos/chromeos_features.h"
 #include "chromeos/dbus/fake_power_manager_client.h"
 #include "chromeos/dbus/power_manager/suspend.pb.h"
 #include "chromeos/disks/disk.h"
@@ -44,9 +41,6 @@ using chromeos::disks::DiskMountManager;
 
 namespace file_manager {
 namespace {
-const char kLsbRelease[] =
-    "CHROMEOS_RELEASE_NAME=Chrome OS\n"
-    "CHROMEOS_RELEASE_VERSION=1.2.3.4\n";
 
 class LoggingObserver : public VolumeManagerObserver {
  public:
@@ -854,21 +848,6 @@ TEST_F(VolumeManagerTest, GetVolumeList) {
   EXPECT_EQ(VOLUME_TYPE_DOWNLOADS_DIRECTORY, volume_list[0]->type());
 }
 
-TEST_F(VolumeManagerTest, VolumeManagerInitializeMyFilesVolume) {
-  // Emulate running inside ChromeOS.
-  chromeos::ScopedSetRunningOnChromeOSForTesting fake_release(kLsbRelease,
-                                                              base::Time());
-  base::test::ScopedFeatureList feature_list;
-  feature_list.InitAndEnableFeature(chromeos::features::kMyFilesVolume);
-  volume_manager()->Initialize();  // Adds "Downloads"
-  std::vector<base::WeakPtr<Volume>> volume_list =
-      volume_manager()->GetVolumeList();
-  ASSERT_EQ(1u, volume_list.size());
-  auto volume = volume_list[0];
-  EXPECT_EQ("downloads:MyFiles", volume->volume_id());
-  EXPECT_EQ(VOLUME_TYPE_DOWNLOADS_DIRECTORY, volume->type());
-}
-
 TEST_F(VolumeManagerTest, FindVolumeById) {
   volume_manager()->Initialize();  // Adds "Downloads"
   base::WeakPtr<Volume> bad_volume =

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

@@ -186,16 +186,6 @@ function NavigationListModel(
    */
   this.myFilesModel_ = null;
 
-  /**
-   * True when MyFiles should be a volume and Downloads just a plain folder
-   * inside it. When false MyFiles is an EntryList, which means UI only type,
-   * which contains Downloads as a child volume.
-   * @private {boolean}
-   */
-  this.myFilesVolumeEnabled_ =
-      loadTimeData.valueExists('MY_FILES_VOLUME_ENABLED') &&
-      loadTimeData.getBoolean('MY_FILES_VOLUME_ENABLED');
-
   /**
    * All root navigation items in display order.
    * @private {!Array<!NavigationModelItem>}
@@ -497,29 +487,12 @@ NavigationListModel.prototype.orderAndNestItems_ = function() {
 
   let myFilesEntry, myFilesModel;
   if (!this.myFilesModel_) {
-    if (this.myFilesVolumeEnabled_) {
-      // When MyFilesVolume is enabled we use the Downloads volume to be the
-      // MyFiles volume.
-      const myFilesVolumeModel =
-          getSingleVolume(VolumeManagerCommon.VolumeType.DOWNLOADS);
-      if (myFilesVolumeModel) {
-        myFilesEntry = new VolumeEntry(myFilesVolumeModel.volumeInfo);
-        myFilesModel = new NavigationModelFakeItem(
-            str('MY_FILES_ROOT_LABEL'), NavigationModelItemType.ENTRY_LIST,
-            myFilesEntry);
-        this.myFilesModel_ = myFilesModel;
-      }
-    } else {
-      // Here is the initial version for MyFiles, which is only an entry in JS
-      // to be displayed in the DirectoryTree, cotaining Downloads, Linux and
-      // Play files volumes.
-      myFilesEntry = new EntryList(
-          str('MY_FILES_ROOT_LABEL'), VolumeManagerCommon.RootType.MY_FILES);
-      myFilesModel = new NavigationModelFakeItem(
-          myFilesEntry.label, NavigationModelItemType.ENTRY_LIST, myFilesEntry);
-      myFilesModel.section = NavigationSection.MY_FILES;
-      this.myFilesModel_ = myFilesModel;
-    }
+    myFilesEntry = new EntryList(
+        str('MY_FILES_ROOT_LABEL'), VolumeManagerCommon.RootType.MY_FILES);
+    myFilesModel = new NavigationModelFakeItem(
+        myFilesEntry.label, NavigationModelItemType.ENTRY_LIST, myFilesEntry);
+    myFilesModel.section = NavigationSection.MY_FILES;
+    this.myFilesModel_ = myFilesModel;
   } else {
     myFilesEntry = this.myFilesModel_.entry;
     myFilesModel = this.myFilesModel_;
@@ -527,18 +500,15 @@ NavigationListModel.prototype.orderAndNestItems_ = function() {
   this.navigationItems_.push(myFilesModel);
 
   // Add Downloads to My Files.
-  if (!this.myFilesVolumeEnabled_) {
-    const downloadsVolume =
-        getSingleVolume(VolumeManagerCommon.VolumeType.DOWNLOADS);
-    if (downloadsVolume) {
-      // Only add volume if MyFiles doesn't have it yet.
-      if (myFilesEntry.findIndexByVolumeInfo(downloadsVolume.volumeInfo) ===
-          -1) {
-        myFilesEntry.addEntry(new VolumeEntry(downloadsVolume.volumeInfo));
-      }
-    } else {
-      myFilesEntry.removeByVolumeType(VolumeManagerCommon.VolumeType.DOWNLOADS);
+  const downloadsVolume =
+      getSingleVolume(VolumeManagerCommon.VolumeType.DOWNLOADS);
+  if (downloadsVolume) {
+    // Only add volume if MyFiles doesn't have it yet.
+    if (myFilesEntry.findIndexByVolumeInfo(downloadsVolume.volumeInfo) === -1) {
+      myFilesEntry.addEntry(new VolumeEntry(downloadsVolume.volumeInfo));
     }
+  } else {
+    myFilesEntry.removeByVolumeType(VolumeManagerCommon.VolumeType.DOWNLOADS);
   }
 
   // Add Android to My Files.

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

@@ -28,16 +28,11 @@ loadTimeData.data = {
 function setUp() {
   new MockCommandLinePrivate();
   // Override VolumeInfo.prototype.resolveDisplayRoot.
-  VolumeInfoImpl.prototype.resolveDisplayRoot = function(successCallback) {
-    this.displayRoot_ = this.fileSystem_.root;
-    successCallback(this.displayRoot_);
-  };
+  VolumeInfoImpl.prototype.resolveDisplayRoot = function() {};
 
   // TODO(crbug.com/834103): Add integration test for Crostini.
   drive = new MockFileSystem('drive');
   hoge = new MockFileSystem('removable:hoge');
-
-  loadTimeData.data_['MY_FILES_VOLUME_ENABLED'] = false;
 }
 
 function testModel() {
@@ -55,6 +50,10 @@ function testModel() {
           'linux-files-label', VolumeManagerCommon.RootType.CROSTINI));
 
   assertEquals(4, model.length);
+  console.log(model.item(0).label);
+  console.log(model.item(1).label);
+  console.log(model.item(2).label);
+  console.log(model.item(3).label);
   assertEquals('fake-entry://recent', model.item(0).entry.toURL());
   assertEquals('/root/shortcut', model.item(1).entry.fullPath);
   assertEquals('My files', model.item(2).label);
@@ -62,6 +61,8 @@ function testModel() {
 
   // Downloads and Crostini are displayed within My files.
   const myFilesEntry = model.item(2).entry;
+  console.log(myFilesEntry.children[0].name);
+  console.log(myFilesEntry.children[1].name);
   assertEquals(2, myFilesEntry.children.length);
   assertEquals('Downloads', myFilesEntry.children[0].name);
   assertEquals('linux-files-label', myFilesEntry.children[1].name);
@@ -296,69 +297,3 @@ function testOrderAndNestItems() {
   // expects it to be the same instance to be able to find it on the tree.
   assertEquals(myFilesModel, model.item(6));
 }
-
-
-function testMyFilesVolumeEnabled(callback) {
-  loadTimeData.data_['MY_FILES_VOLUME_ENABLED'] = true;
-  const volumeManager = new MockVolumeManager();
-  // Index 1 is Downloads.
-  assertEquals(
-      VolumeManagerCommon.VolumeType.DOWNLOADS,
-      volumeManager.volumeInfoList.item(1).volumeType);
-
-  // Create a downloads folder inside it.
-  const downloadsVolume = volumeManager.volumeInfoList.item(1);
-  downloadsVolume.fileSystem.populate(['/Downloads/']);
-
-  const shortcutListModel = new MockFolderShortcutDataModel([]);
-  const recentItem = null;
-
-
-  const crostiniFakeItem = new NavigationModelFakeItem(
-      'linux-files-label', NavigationModelItemType.CROSTINI,
-      new FakeEntry(
-          'linux-files-label', VolumeManagerCommon.RootType.CROSTINI));
-
-  // Create Android volume.
-  volumeManager.volumeInfoList.add(MockVolumeManager.createMockVolumeInfo(
-      VolumeManagerCommon.VolumeType.ANDROID_FILES, 'android_files:droid'));
-
-  // Navigation items built above:
-  //  1. My files
-  //       -> Play files
-  //       -> Linux files
-  //  2. Drive  - added by default by MockVolumeManager.
-
-  // Constructor already calls orderAndNestItems_.
-  const model =
-      new NavigationListModel(volumeManager, shortcutListModel, recentItem);
-  model.linuxFilesItem = crostiniFakeItem;
-
-  assertEquals(2, model.length);
-  assertEquals('My files', model.item(0).label);
-  assertEquals('My Drive', model.item(1).label);
-
-  // Android and Crostini are displayed within My files. And there is no
-  // Downloads volume inside it. Downloads should be a normal folder inside My
-  // files volume.
-  const myFilesEntry = model.item(0).entry;
-  assertEquals(2, myFilesEntry.children_.length);
-  assertEquals('android_files:droid', myFilesEntry.children_[0].name);
-  assertEquals('linux-files-label', myFilesEntry.children_[1].name);
-
-  const reader = myFilesEntry.createReader();
-  const foundEntries = [];
-  reader.readEntries((entries) => {
-    for (entry of entries)
-      foundEntries.push(entry);
-  });
-  reportPromise(
-      waitUntil(() => {
-        // Wait for Downloads folder to be read from My files volume.
-        return foundEntries.length >= 1;
-      }).then(() => {
-        assertEquals(foundEntries[0].name, 'Downloads');
-        assertTrue(foundEntries[0].isDirectory);
-      }),
-      callback);
-}

ui/file_manager/file_manager/foreground/js/ui/directory_tree_unittest.js

@@ -502,9 +502,8 @@ function testUpdateSubElementsFromListSections() {
   assertEquals(NavigationSection.CLOUD, driveItem.section);
 
   const metadataModel = mockMetadataModel();
-  DirectoryTree.decorate(
-      directoryTree, directoryModel, volumeManager, metadataModel,
-      fileOperationManager, true);
+  DirectoryTree.decorate(directoryTree, directoryModel, volumeManager,
+      metadataModel, fileOperationManager, true);
   directoryTree.dataModel = treeModel;
   directoryTree.updateSubElementsFromList(false);