crostini: share file not whole directory for export

BUG=chromium:987127

Change-Id: I5941380c70786e7c8d9f3b6c2d4a992944775a53
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1715170Reviewed-by: Joel Hockey <joelhockey@chromium.org>
Commit-Queue: Julian Watson <juwa@google.com>
Cr-Commit-Position: refs/heads/master@{#681088}

chrome/browser/chromeos/crostini/crostini_export_import.cc

@@ -7,8 +7,10 @@
 #include <utility>
 
 #include "base/bind.h"
+#include "base/files/file_util.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/strings/stringprintf.h"
+#include "base/task/post_task.h"
 #include "base/time/time.h"
 #include "chrome/browser/chromeos/crostini/crostini_manager_factory.h"
 #include "chrome/browser/chromeos/crostini/crostini_util.h"
@@ -175,12 +177,26 @@ void CrostiniExportImport::Start(
 
   switch (type) {
     case ExportImportType::EXPORT:
-      guest_os::GuestOsSharePath::GetForProfile(profile_)->SharePath(
-          kCrostiniDefaultVmName, path.DirName(), false,
-          base::BindOnce(&CrostiniExportImport::ExportAfterSharing,
-                         weak_ptr_factory_.GetWeakPtr(),
-                         std::move(container_id), path.BaseName(),
-                         std::move(callback)));
+      base::PostTaskWithTraitsAndReply(
+          FROM_HERE, {base::MayBlock()},
+          // Ensure file exists so that it can be shared.
+          base::BindOnce(
+              [](const base::FilePath& path) {
+                base::File file(path, base::File::FLAG_CREATE_ALWAYS |
+                                          base::File::FLAG_WRITE);
+                DCHECK(file.IsValid()) << path << " is invalid";
+              },
+              path),
+          base::BindOnce(
+              &guest_os::GuestOsSharePath::SharePath,
+              base::Unretained(
+                  guest_os::GuestOsSharePath::GetForProfile(profile_)),
+              kCrostiniDefaultVmName, path, false,
+              base::BindOnce(&CrostiniExportImport::ExportAfterSharing,
+                             weak_ptr_factory_.GetWeakPtr(),
+                             std::move(container_id), std::move(callback))
+
+                  ));
       break;
     case ExportImportType::IMPORT:
       guest_os::GuestOsSharePath::GetForProfile(profile_)->SharePath(
@@ -194,7 +210,6 @@ void CrostiniExportImport::Start(
 
 void CrostiniExportImport::ExportAfterSharing(
     const ContainerId& container_id,
-    const base::FilePath& filename,
     CrostiniManager::CrostiniResultCallback callback,
     const base::FilePath& container_path,
     bool result,
@@ -209,8 +224,7 @@ void CrostiniExportImport::ExportAfterSharing(
     return;
   }
   CrostiniManager::GetForProfile(profile_)->ExportLxdContainer(
-      kCrostiniDefaultVmName, kCrostiniDefaultContainerName,
-      container_path.Append(filename),
+      kCrostiniDefaultVmName, kCrostiniDefaultContainerName, container_path,
       base::BindOnce(&CrostiniExportImport::OnExportComplete,
                      weak_ptr_factory_.GetWeakPtr(), base::Time::Now(),
                      container_id, std::move(callback)));
@@ -238,6 +252,10 @@ void CrostiniExportImport::OnExportComplete(
     }
   } else {
     LOG(ERROR) << "Error exporting " << int(result);
+    base::PostTaskWithTraits(
+        FROM_HERE, {base::MayBlock(), base::TaskPriority::BEST_EFFORT},
+        base::BindOnce(base::IgnoreResult(&base::DeleteFile),
+                       it->second->path(), false));
     switch (result) {
       case CrostiniResult::CONTAINER_EXPORT_IMPORT_FAILED_VM_STOPPED:
         enum_hist_result = ExportContainerResult::kFailedVmStopped;

chrome/browser/chromeos/crostini/crostini_export_import.h

@@ -131,7 +131,6 @@ class CrostiniExportImport : public KeyedService,
                                  uint64_t minimum_required_space) override;
 
   void ExportAfterSharing(const ContainerId& container_id,
-                          const base::FilePath& filename,
                           CrostiniManager::CrostiniResultCallback callback,
                           const base::FilePath& container_path,
                           bool result,

chrome/browser/chromeos/crostini/crostini_export_import_notification.h

@@ -55,6 +55,7 @@ class CrostiniExportImportNotification
 
   Status status() const { return status_; }
   ExportImportType type() const { return type_; }
+  const base::FilePath& path() const { return path_; }
   // Getters for testing.
   message_center::Notification* get_notification() {
     return notification_.get();

chrome/browser/chromeos/crostini/crostini_export_import_unittest.cc

@@ -174,6 +174,9 @@ TEST_F(CrostiniExportImportTest, TestDeprecatedExportSuccess) {
       vm_tools::cicerone::ExportLxdContainerProgressSignal_Status_DONE);
   EXPECT_EQ(notification->status(),
             CrostiniExportImportNotification::Status::DONE);
+  // CrostiniExportImport should've created the exported file.
+  thread_bundle_.RunUntilIdle();
+  EXPECT_TRUE(base::PathExists(tarball_));
 }
 
 TEST_F(CrostiniExportImportTest, TestExportSuccess) {
@@ -229,6 +232,9 @@ TEST_F(CrostiniExportImportTest, TestExportSuccess) {
       vm_tools::cicerone::ExportLxdContainerProgressSignal_Status_DONE);
   EXPECT_EQ(notification->status(),
             CrostiniExportImportNotification::Status::DONE);
+  // CrostiniExportImport should've created the exported file.
+  thread_bundle_.RunUntilIdle();
+  EXPECT_TRUE(base::PathExists(tarball_));
 }
 
 TEST_F(CrostiniExportImportTest, TestExportFail) {
@@ -243,6 +249,9 @@ TEST_F(CrostiniExportImportTest, TestExportFail) {
       vm_tools::cicerone::ExportLxdContainerProgressSignal_Status_FAILED);
   EXPECT_EQ(notification->status(),
             CrostiniExportImportNotification::Status::FAILED);
+  // CrostiniExportImport should cleanup the file if an export fails.
+  thread_bundle_.RunUntilIdle();
+  EXPECT_FALSE(base::PathExists(tarball_));
 }
 
 TEST_F(CrostiniExportImportTest, TestImportSuccess) {

chrome/browser/chromeos/guest_os/guest_os_share_path.cc

@@ -632,18 +632,20 @@ void GuestOsSharePath::CheckIfPathDeleted(const base::FilePath& path) {
     return;
   }
 
+  // VolumeManager will be nullptr if running inside a test.
+  auto* vmgr = file_manager::VolumeManager::Get(profile_);
+  if (!vmgr) {
+    return;
+  }
   // If we can't find the path, check if the volume was unmounted.
   // FileWatchers may fire before VolumeManager::OnVolumeUnmounted.
-  bool volume_still_mounted = false;
-  const std::vector<base::WeakPtr<file_manager::Volume>>& volume_list =
-      file_manager::VolumeManager::Get(profile_)->GetVolumeList();
-  for (const auto& volume : volume_list) {
-    if ((path == volume->mount_path() || volume->mount_path().IsParent(path)) &&
-        base::PathExists(volume->mount_path())) {
-      volume_still_mounted = true;
-      break;
-    }
-  }
+  const auto volume_list = vmgr->GetVolumeList();
+  const bool volume_still_mounted = std::any_of(
+      volume_list.begin(), volume_list.end(), [&path](const auto& volume) {
+        return (path == volume->mount_path() ||
+                volume->mount_path().IsParent(path)) &&
+               base::PathExists(volume->mount_path());
+      });
   if (!volume_still_mounted) {
     return;
   }