Reland "Change NavigationListModel to support MyFiles as Volume."

This is a reland of 2903236d

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}

Bug: 873539
Change-Id: I396cda87781c21856a736c6f5eae8f76efb4e62c
Reviewed-on: https://chromium-review.googlesource.com/c/1314012Reviewed-by: Luciano Pacheco <lucmult@chromium.org>
Reviewed-by: Joel Hockey <joelhockey@chromium.org>
Commit-Queue: Luciano Pacheco <lucmult@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604836}

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

@@ -839,6 +839,9 @@ 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,14 +416,13 @@ void VolumeManager::Initialize() {
     return;
   }
 
-  // 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);
+  const base::FilePath localVolume =
+      file_manager::util::GetMyFilesFolderForProfile(profile_);
+  const bool success = RegisterDownloadsMountPoint(profile_, localVolume);
   DCHECK(success);
 
   DoMountEvent(chromeos::MOUNT_ERROR_NONE,
-               Volume::CreateForDownloads(downloads));
+               Volume::CreateForDownloads(localVolume));
 
   // Subscribe to DriveIntegrationService.
   drive_integration_service_->AddObserver(this);

chrome/browser/chromeos/file_manager/volume_manager_unittest.cc

@@ -14,14 +14,17 @@
 #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"
@@ -41,6 +44,9 @@ 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:
@@ -848,6 +854,21 @@ 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

@@ -187,6 +187,16 @@ 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,12 +507,29 @@ NavigationListModel.prototype.orderAndNestItems_ = function() {
 
   let myFilesEntry, myFilesModel;
   if (!this.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;
+    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;
+    }
   } else {
     myFilesEntry = this.myFilesModel_.entry;
     myFilesModel = this.myFilesModel_;
@@ -510,15 +537,18 @@ NavigationListModel.prototype.orderAndNestItems_ = function() {
   this.navigationItems_.push(myFilesModel);
 
   // Add Downloads to My Files.
-  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));
+  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);
     }
-  } else {
-    myFilesEntry.removeByVolumeType(VolumeManagerCommon.VolumeType.DOWNLOADS);
   }
 
   // Add Android to My Files.

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

@@ -28,11 +28,16 @@ loadTimeData.data = {
 function setUp() {
   new MockCommandLinePrivate();
   // Override VolumeInfo.prototype.resolveDisplayRoot.
-  VolumeInfoImpl.prototype.resolveDisplayRoot = function() {};
+  VolumeInfoImpl.prototype.resolveDisplayRoot = function(successCallback) {
+    this.displayRoot_ = this.fileSystem_.root;
+    successCallback(this.displayRoot_);
+  };
 
   // 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() {
@@ -50,10 +55,6 @@ 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);
@@ -61,8 +62,6 @@ 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);
@@ -297,3 +296,69 @@ 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,8 +502,9 @@ 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);