Add storage handler my files unit tests

Extend storage handler's tests by checking my files size computation.

The "My files" row on the setting's storage page displays the size of
My files and android play files (excluding apps). The Downloads folder
is bind mounted on [android files path]/Download. The files in these
folders shouldn't be counted twice.

This test registers a temporary location for My files and Android
files, for a given profile, and tests that the size computed under
"My files" produces the expected output.

Bug: 1013459, 1044495
Change-Id: Ib5f6b58a73eb6b732f88c9117bbfefbcdc179f14
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2016689
Commit-Queue: Jeremie Boulic <jboulic@chromium.org>
Reviewed-by: Luciano Pacheco <lucmult@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#735765}

chrome/browser/chromeos/file_manager/path_util.cc

@@ -157,6 +157,17 @@ base::FilePath GetMyFilesFolderForProfile(Profile* profile) {
   return profile->GetPath().AppendASCII(kFolderNameMyFiles);
 }
 
+base::FilePath GetAndroidPlayFilesPath() {
+  // Check if Android has a registered path already. This happens for tests.
+  const std::string mount_point_name = util::GetAndroidFilesMountPointName();
+  storage::ExternalMountPoints* const mount_points =
+      storage::ExternalMountPoints::GetSystemInstance();
+  base::FilePath path;
+  if (mount_points->GetRegisteredPath(mount_point_name, &path))
+    return path;
+  return base::FilePath(file_manager::util::kAndroidFilesPath);
+}
+
 bool MigratePathFromOldFormat(Profile* profile,
                               const base::FilePath& old_base,
                               const base::FilePath& old_path,

chrome/browser/chromeos/file_manager/path_util.h

@@ -33,6 +33,10 @@ base::FilePath GetDownloadsFolderForProfile(Profile* profile);
 // Gets the absolute path for the 'MyFiles' folder for the |profile|.
 base::FilePath GetMyFilesFolderForProfile(Profile* profile);
 
+// Gets the absolute path for the user's Android Play files (Movies, Pictures,
+// etc..., Android apps excluded). The default path may be overridden by tests.
+base::FilePath GetAndroidPlayFilesPath();
+
 // Converts |old_path| to |new_path| and returns true, if the old path points
 // to an old location of user folders (in "Downloads" or "Google Drive").
 // The |profile| argument is used for determining the location of the

chrome/browser/ui/webui/settings/chromeos/device_storage_handler.cc

@@ -65,6 +65,10 @@ void StorageHandler::TestAPI::OnGetSizeStat(int64_t* total_size,
   handler_->OnGetSizeStat(total_size, available_size);
 }
 
+void StorageHandler::TestAPI::UpdateMyFilesSize() {
+  handler_->UpdateMyFilesSize();
+}
+
 namespace {
 
 void GetSizeStatBlocking(const base::FilePath& mount_path,
@@ -216,15 +220,15 @@ void StorageHandler::HandleUpdateExternalStorages(
 }
 
 void StorageHandler::UpdateSizeStat() {
-  const base::FilePath downloads_path =
-      file_manager::util::GetDownloadsFolderForProfile(profile_);
+  const base::FilePath my_files_path =
+      file_manager::util::GetMyFilesFolderForProfile(profile_);
 
   int64_t* total_size = new int64_t(0);
   int64_t* available_size = new int64_t(0);
   base::PostTaskAndReply(
       FROM_HERE,
       {base::ThreadPool(), base::MayBlock(), base::TaskPriority::USER_VISIBLE},
-      base::Bind(&GetSizeStatBlocking, downloads_path, total_size,
+      base::Bind(&GetSizeStatBlocking, my_files_path, total_size,
                  available_size),
       base::Bind(&StorageHandler::OnGetSizeStat, weak_ptr_factory_.GetWeakPtr(),
                  base::Owned(total_size), base::Owned(available_size)));
@@ -256,34 +260,36 @@ void StorageHandler::UpdateMyFilesSize() {
   const base::FilePath my_files_path =
       file_manager::util::GetMyFilesFolderForProfile(profile_);
 
+  const base::FilePath android_files_path =
+      base::FilePath(file_manager::util::GetAndroidPlayFilesPath());
+
   base::PostTaskAndReplyWithResult(
       FROM_HERE,
       {base::ThreadPool(), base::MayBlock(), base::TaskPriority::BEST_EFFORT},
       base::BindOnce(&StorageHandler::ComputeLocalFilesSize,
-                     base::Unretained(this), my_files_path),
+                     base::Unretained(this), my_files_path, android_files_path),
       base::BindOnce(&StorageHandler::OnGetMyFilesSize,
                      weak_ptr_factory_.GetWeakPtr()));
 }
 
 int64_t StorageHandler::ComputeLocalFilesSize(
-    const base::FilePath& my_files_path) {
+    const base::FilePath& my_files_path,
+    const base::FilePath& android_files_path) {
   int64_t size = 0;
 
   // Compute directory size of My Files
   size += base::ComputeDirectorySize(my_files_path);
 
   // Compute directory size of Play Files
-  const base::FilePath android_files_path =
-      base::FilePath(file_manager::util::kAndroidFilesPath);
   size += base::ComputeDirectorySize(android_files_path);
 
   // Remove size of Download. If Android is enabled, the size of the Download
   // folder is counted in both My Files and Play files. If Android is disabled,
   // the Download folder doesn't exist and the returned size is 0.
   const base::FilePath download_files_path =
-      base::FilePath(file_manager::util::kAndroidFilesPath)
-          .AppendASCII("Download");
+      android_files_path.AppendASCII("Download");
   size -= base::ComputeDirectorySize(download_files_path);
+
   return size;
 }
 

chrome/browser/ui/webui/settings/chromeos/device_storage_handler.h

@@ -67,6 +67,9 @@ class StorageHandler
     // Simulate a callback with controlled total and available size inputs.
     void OnGetSizeStat(int64_t* total_size, int64_t* available_size);
 
+    // Simulate a request to update MyFiles size.
+    void UpdateMyFilesSize();
+
    private:
     StorageHandler* handler_;  // Not owned.
   };
@@ -110,7 +113,8 @@ class StorageHandler
   void UpdateMyFilesSize();
 
   // Computes the size of My Files and Play files.
-  int64_t ComputeLocalFilesSize(const base::FilePath& my_files_path);
+  int64_t ComputeLocalFilesSize(const base::FilePath& my_files_path,
+                                const base::FilePath& android_files_path);
 
   // Callback to update the UI about the size of Downloads directory.
   void OnGetMyFilesSize(int64_t size);

chrome/browser/ui/webui/settings/chromeos/device_storage_handler_unittest.cc

@@ -3,11 +3,17 @@
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/webui/settings/chromeos/device_storage_handler.h"
+
+#include "base/files/file.h"
+#include "base/files/file_util.h"
+#include "base/path_service.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/system/sys_info.h"
 #include "chrome/browser/chromeos/arc/session/arc_session_manager.h"
 #include "chrome/browser/chromeos/file_manager/fake_disk_mount_manager.h"
 #include "chrome/browser/chromeos/file_manager/path_util.h"
+#include "chrome/browser/chromeos/scoped_set_running_on_chromeos_for_testing.h"
+#include "chrome/common/chrome_paths.h"
 #include "chrome/common/webui_url_constants.h"
 #include "chrome/test/base/testing_browser_process.h"
 #include "chrome/test/base/testing_profile_manager.h"
@@ -16,6 +22,7 @@
 #include "content/public/browser/web_ui_data_source.h"
 #include "content/public/test/browser_task_environment.h"
 #include "content/public/test/test_web_ui.h"
+#include "storage/browser/file_system/external_mount_points.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/base/text/bytes_formatting.h"
 
@@ -24,6 +31,10 @@ namespace settings {
 
 namespace {
 
+const char kLsbRelease[] =
+    "CHROMEOS_RELEASE_NAME=Chrome OS\n"
+    "CHROMEOS_RELEASE_VERSION=1.2.3.4\n";
+
 class TestStorageHandler : public StorageHandler {
  public:
   explicit TestStorageHandler(Profile* profile,
@@ -63,12 +74,26 @@ class StorageHandlerTest : public testing::Test {
         std::make_unique<StorageHandler::TestAPI>(handler_.get());
     handler_->set_web_ui(&web_ui_);
     handler_->AllowJavascriptForTesting();
+
+    // Create and register My files directory.
+    // By emulating chromeos running, GetMyFilesFolderForProfile will return the
+    // profile's temporary location instead of $HOME/Downloads.
+    chromeos::ScopedSetRunningOnChromeOSForTesting fake_release(kLsbRelease,
+                                                                base::Time());
+    const base::FilePath my_files_path =
+        file_manager::util::GetMyFilesFolderForProfile(profile_);
+    CHECK(base::CreateDirectory(my_files_path));
+    CHECK(storage::ExternalMountPoints::GetSystemInstance()->RegisterFileSystem(
+        file_manager::util::GetDownloadsMountPointName(profile_),
+        storage::kFileSystemTypeNativeLocal, storage::FileSystemMountOption(),
+        my_files_path));
   }
 
   void TearDown() override {
     handler_.reset();
     handler_test_api_.reset();
     chromeos::disks::DiskMountManager::Shutdown();
+    storage::ExternalMountPoints::GetSystemInstance()->RevokeAllFileSystems();
   }
 
  protected:
@@ -84,8 +109,9 @@ class StorageHandlerTest : public testing::Test {
     return space_state;
   }
 
-  // Expects a callback message with a given event_name. A non null base::Value
-  // is returned if the callback message is found and has associated data.
+  // Expects a callback message with a given |event_name|. A non null
+  // base::Value is returned if the callback message is found and has associated
+  // data.
   const base::Value* GetWebUICallbackMessage(const std::string& event_name) {
     for (auto it = web_ui_.call_data().rbegin();
          it != web_ui_.call_data().rend(); ++it) {
@@ -101,6 +127,38 @@ class StorageHandlerTest : public testing::Test {
     return nullptr;
   }
 
+  // Get the path to file manager's test data directory.
+  base::FilePath GetTestDataFilePath(const std::string& file_name) {
+    // Get the path to file manager's test data directory.
+    base::FilePath source_dir;
+    CHECK(base::PathService::Get(base::DIR_SOURCE_ROOT, &source_dir));
+    base::FilePath test_data_dir = source_dir.AppendASCII("chrome")
+                                       .AppendASCII("test")
+                                       .AppendASCII("data")
+                                       .AppendASCII("chromeos")
+                                       .AppendASCII("file_manager");
+
+    // Return full test data path to the given |file_name|.
+    return test_data_dir.Append(base::FilePath::FromUTF8Unsafe(file_name));
+  }
+
+  // Copy a file from the file manager's test data directory to the specified
+  // target_path.
+  void AddFile(const std::string& file_name,
+               int64_t expected_size,
+               base::FilePath target_path) {
+    const base::FilePath entry_path = GetTestDataFilePath(file_name);
+    target_path = target_path.AppendASCII(file_name);
+    ASSERT_TRUE(base::CopyFile(entry_path, target_path))
+        << "Copy from " << entry_path.value() << " to " << target_path.value()
+        << " failed.";
+    // Verify file size.
+    base::stat_wrapper_t stat;
+    const int res = base::File::Lstat(target_path.value().c_str(), &stat);
+    ASSERT_FALSE(res < 0) << "Couldn't stat" << target_path.value();
+    ASSERT_EQ(expected_size, stat.st_size);
+  }
+
   std::unique_ptr<TestStorageHandler> handler_;
   std::unique_ptr<TestStorageHandler::TestAPI> handler_test_api_;
   content::TestWebUI web_ui_;
@@ -173,6 +231,49 @@ TEST_F(StorageHandlerTest, StorageSpaceState) {
   EXPECT_EQ(handler_->STORAGE_SPACE_NORMAL, space_state);
 }
 
+TEST_F(StorageHandlerTest, MyFilesSize) {
+  base::ScopedAllowBlockingForTesting allow_blocking;
+
+  const base::FilePath my_files_path =
+      file_manager::util::GetMyFilesFolderForProfile(profile_);
+  const base::FilePath downloads_path =
+      file_manager::util::GetDownloadsFolderForProfile(profile_);
+  const base::FilePath android_files_path =
+      profile_->GetPath().Append("AndroidFiles");
+  const base::FilePath android_files_download_path =
+      android_files_path.Append("Download");
+
+  // Create directories.
+  CHECK(base::CreateDirectory(downloads_path));
+  CHECK(base::CreateDirectory(android_files_path));
+  CHECK(base::CreateDirectory(android_files_download_path));
+
+  // Register android files mount point.
+  CHECK(storage::ExternalMountPoints::GetSystemInstance()->RegisterFileSystem(
+      file_manager::util::GetAndroidFilesMountPointName(),
+      storage::kFileSystemTypeNativeLocal, storage::FileSystemMountOption(),
+      android_files_path));
+
+  // Add files in My files and android files.
+  AddFile("random.bin", 8092, my_files_path);      // ~7.9K
+  AddFile("tall.pdf", 15271, android_files_path);  // ~14.9K
+  // Add file in Downloads and simulate bind mount with
+  // [android files]/Download.
+  AddFile("video.ogv", 59943, downloads_path);  // ~58.6K
+  AddFile("video.ogv", 59943, android_files_download_path);
+
+  // Calculate My files size.
+  handler_test_api_->UpdateMyFilesSize();
+  task_environment_.RunUntilIdle();
+
+  const base::Value* callback =
+      GetWebUICallbackMessage("storage-my-files-size-changed");
+  ASSERT_TRUE(callback) << "No 'storage-my-files-size-changed' callback";
+
+  // Check return value.
+  EXPECT_EQ("81.4 KB", callback->GetString());
+}
+
 }  // namespace
 
 }  // namespace settings