Support rename/moves of files backing holding space items.

When a file rename/move is observed by the `FileChangeService`, we will
be notified of the event in the `HoldingSpaceFileSystemDelegate`. This
delegate will update the holding space model appropriately and update
any file watches as necessary.

When a holding space item tracked in the model is moved, the
`HoldingSpacePersistenceDelegate` will be notified of the event and
update persistence so that the moved file will be successfully restore
on next restart.

Bug: 1136173
Change-Id: I758b7a828dd880a15a5fb5ab59de8f1bd0889c08
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2583133
Reviewed-by: Toni Baržić <tbarzic@chromium.org>
Commit-Queue: David Black <dmblack@google.com>
Cr-Commit-Position: refs/heads/master@{#835948}

ash/public/cpp/holding_space/holding_space_item.cc

@@ -123,6 +123,13 @@ void HoldingSpaceItem::Finalize(const GURL& file_system_url) {
   file_system_url_ = file_system_url;
 }
 
+void HoldingSpaceItem::UpdateBackingFile(const base::FilePath& file_path,
+                                         const GURL& file_system_url) {
+  file_path_ = file_path;
+  file_system_url_ = file_system_url;
+  text_ = file_path.BaseName().LossyDisplayName();
+}
+
 HoldingSpaceItem::HoldingSpaceItem(Type type,
                                    const std::string& id,
                                    const base::FilePath& file_path,

ash/public/cpp/holding_space/holding_space_item.h

@@ -82,6 +82,10 @@ class ASH_PUBLIC_EXPORT HoldingSpaceItem {
   // Used to finalize partially initialized items created by `Deserialize()`.
   void Finalize(const GURL& file_system_url);
 
+  // Updates the file backing the item to `file_path` and `file_system_url`.
+  void UpdateBackingFile(const base::FilePath& file_path,
+                         const GURL& file_system_url);
+
   const std::string& id() const { return id_; }
 
   Type type() const { return type_; }

ash/public/cpp/holding_space/holding_space_model.cc

@@ -65,6 +65,26 @@ void HoldingSpaceModel::FinalizeOrRemoveItem(const std::string& id,
     observer.OnHoldingSpaceItemFinalized(item);
 }
 
+void HoldingSpaceModel::UpdateBackingFileForItem(
+    const std::string& id,
+    const base::FilePath& file_path,
+    const GURL& file_system_url) {
+  auto item_it = std::find_if(
+      items_.begin(), items_.end(),
+      [&id](const std::unique_ptr<HoldingSpaceItem>& item) -> bool {
+        return item->id() == id;
+      });
+  DCHECK(item_it != items_.end());
+
+  HoldingSpaceItem* item = item_it->get();
+  DCHECK(item->IsFinalized());
+
+  item->UpdateBackingFile(file_path, file_system_url);
+
+  for (auto& observer : observers_)
+    observer.OnHoldingSpaceItemUpdated(item);
+}
+
 void HoldingSpaceModel::RemoveIf(Predicate predicate) {
   for (int i = items_.size() - 1; i >= 0; --i) {
     const HoldingSpaceItem* item = items_.at(i).get();

ash/public/cpp/holding_space/holding_space_model.h

@@ -49,6 +49,12 @@ class ASH_PUBLIC_EXPORT HoldingSpaceModel {
   // file system URL. The item will be removed if the file system url is empty.
   void FinalizeOrRemoveItem(const std::string& id, const GURL& file_system_url);
 
+  // Updates the backing file for a single holding space item to the specified
+  // `file_path` and `file_system_url`.
+  void UpdateBackingFileForItem(const std::string& id,
+                                const base::FilePath& file_path,
+                                const GURL& file_system_url);
+
   // Removes all holding space items from the model for which the specified
   // `predicate` returns true.
   using Predicate = base::RepeatingCallback<bool(const HoldingSpaceItem*)>;

ash/public/cpp/holding_space/holding_space_model_observer.h

@@ -15,14 +15,17 @@ class HoldingSpaceItem;
 class ASH_PUBLIC_EXPORT HoldingSpaceModelObserver
     : public base::CheckedObserver {
  public:
-  // Called when an item gets added to the holding space model.
-  virtual void OnHoldingSpaceItemAdded(const HoldingSpaceItem* item) = 0;
+  // Called when an `item` gets added to the holding space model.
+  virtual void OnHoldingSpaceItemAdded(const HoldingSpaceItem* item) {}
 
-  // Called when an item gets removed from the holding space model.
-  virtual void OnHoldingSpaceItemRemoved(const HoldingSpaceItem* item) = 0;
+  // Called when an `item` gets removed from the holding space model.
+  virtual void OnHoldingSpaceItemRemoved(const HoldingSpaceItem* item) {}
+
+  // Called when an `item` gets updated within the holding space model.
+  virtual void OnHoldingSpaceItemUpdated(const HoldingSpaceItem* item) {}
 
   // Called when a partially initialized holding space `item` gets finalized.
-  virtual void OnHoldingSpaceItemFinalized(const HoldingSpaceItem* item) = 0;
+  virtual void OnHoldingSpaceItemFinalized(const HoldingSpaceItem* item) {}
 };
 
 }  // namespace ash

ash/system/holding_space/holding_space_item_chip_view.cc

@@ -130,6 +130,13 @@ HoldingSpaceItemChipView::HoldingSpaceItemChipView(
 
 HoldingSpaceItemChipView::~HoldingSpaceItemChipView() = default;
 
+void HoldingSpaceItemChipView::OnHoldingSpaceItemUpdated(
+    const HoldingSpaceItem* item) {
+  HoldingSpaceItemView::OnHoldingSpaceItemUpdated(item);
+  if (this->item() == item)
+    label_->SetText(item->text());
+}
+
 void HoldingSpaceItemChipView::OnPinVisiblityChanged(bool pin_visible) {
   if (label_mask_layer_owner_->layer()->bounds() !=
       label_and_pin_button_container_->bounds()) {

ash/system/holding_space/holding_space_item_chip_view.h

@@ -38,6 +38,7 @@ class ASH_EXPORT HoldingSpaceItemChipView : public HoldingSpaceItemView {
   class LabelMaskLayerOwner;
 
   // HoldingSpaceItemView:
+  void OnHoldingSpaceItemUpdated(const HoldingSpaceItem* item) override;
   void OnPinVisiblityChanged(bool pin_visible) override;
 
   void UpdateImage();

ash/system/holding_space/holding_space_item_view.cc

@@ -110,6 +110,8 @@ HoldingSpaceItemView::HoldingSpaceItemView(
     HoldingSpaceItemViewDelegate* delegate,
     const HoldingSpaceItem* item)
     : delegate_(delegate), item_(item), item_id_(item->id()) {
+  model_observer_.Observe(HoldingSpaceController::Get()->model());
+
   SetProperty(kIsHoldingSpaceItemViewProperty, true);
 
   set_context_menu_controller(delegate_);
@@ -238,6 +240,12 @@ void HoldingSpaceItemView::OnMouseReleased(const ui::MouseEvent& event) {
   delegate_->OnHoldingSpaceItemViewMouseReleased(this, event);
 }
 
+void HoldingSpaceItemView::OnHoldingSpaceItemUpdated(
+    const HoldingSpaceItem* item) {
+  if (item_ == item)
+    GetViewAccessibility().OverrideName(item->text());
+}
+
 void HoldingSpaceItemView::AnimateIn(ui::LayerAnimationObserver* observer) {
   AnimateImmediatelyTo(/*opacity=*/1.f, gfx::Transform(), observer);
 }

ash/system/holding_space/holding_space_item_view.h

@@ -8,6 +8,9 @@
 #include <memory>
 
 #include "ash/ash_export.h"
+#include "ash/public/cpp/holding_space/holding_space_model.h"
+#include "ash/public/cpp/holding_space/holding_space_model_observer.h"
+#include "base/scoped_observation.h"
 #include "ui/views/animation/ink_drop_host_view.h"
 #include "ui/views/metadata/metadata_header_macros.h"
 
@@ -28,7 +31,8 @@ class HoldingSpaceItemViewDelegate;
 // `HoldingSpaceItemScreenCaptureView`. Note that `HoldingSpaceItemView` may
 // temporarily outlive its associated `HoldingSpaceItem` when it is being
 // animated out.
-class ASH_EXPORT HoldingSpaceItemView : public views::InkDropHostView {
+class ASH_EXPORT HoldingSpaceItemView : public views::InkDropHostView,
+                                        public HoldingSpaceModelObserver {
  public:
   METADATA_HEADER(HoldingSpaceItemView);
 
@@ -56,6 +60,9 @@ class ASH_EXPORT HoldingSpaceItemView : public views::InkDropHostView {
   bool OnMousePressed(const ui::MouseEvent& event) override;
   void OnMouseReleased(const ui::MouseEvent& event) override;
 
+  // HoldingSpaceModelObserver:
+  void OnHoldingSpaceItemUpdated(const HoldingSpaceItem* item) override;
+
   // Invoked to initiate animate in/out of this view. Any animations created
   // will be associated with the specified `observer`.
   void AnimateIn(ui::LayerAnimationObserver* observer);
@@ -107,6 +114,9 @@ class ASH_EXPORT HoldingSpaceItemView : public views::InkDropHostView {
   // Whether or not this view is selected.
   bool selected_ = false;
 
+  base::ScopedObservation<HoldingSpaceModel, HoldingSpaceModelObserver>
+      model_observer_{this};
+
   base::WeakPtrFactory<HoldingSpaceItemView> weak_factory_{this};
 };
 

chrome/browser/ui/ash/holding_space/holding_space_file_system_delegate.cc

@@ -14,6 +14,7 @@
 #include "base/task/task_traits.h"
 #include "base/task/thread_pool.h"
 #include "base/threading/sequenced_task_runner_handle.h"
+#include "chrome/browser/chromeos/fileapi/file_change_service_factory.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
 
@@ -101,8 +102,13 @@ void HoldingSpaceFileSystemDelegate::Init() {
       base::Bind(&HoldingSpaceFileSystemDelegate::OnFilePathChanged,
                  weak_factory_.GetWeakPtr()));
 
-  if (file_manager::VolumeManager::Get(profile()))
-    volume_manager_observer_.Add(file_manager::VolumeManager::Get(profile()));
+  file_change_service_observer_.Observe(
+      chromeos::FileChangeServiceFactory::GetInstance()->GetService(profile()));
+
+  if (file_manager::VolumeManager::Get(profile())) {
+    volume_manager_observer_.Observe(
+        file_manager::VolumeManager::Get(profile()));
+  }
 
   // Schedule a task to clean up items that belong to volumes that haven't been
   // mounted in a reasonable amount of time.
@@ -153,29 +159,25 @@ void HoldingSpaceFileSystemDelegate::OnHoldingSpaceItemAdded(
 void HoldingSpaceFileSystemDelegate::OnHoldingSpaceItemRemoved(
     const HoldingSpaceItem* item) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  MaybeRemoveWatch(item->file_path().DirName());
+}
 
-  // Since we were watching the directory containing `item`'s backing file and
-  // not the backing file itself, we only need to remove the associated watch if
-  // there are no other holding space items backed by the same directory.
-  const bool remove_watch = std::none_of(
-      model()->items().begin(), model()->items().end(),
-      [removed_item = item](const auto& item) {
-        return item->IsFinalized() && item->file_path().DirName() ==
-                                          removed_item->file_path().DirName();
-      });
-
-  if (remove_watch)
-    RemoveWatch(item->file_path().DirName());
+void HoldingSpaceFileSystemDelegate::OnHoldingSpaceItemUpdated(
+    const HoldingSpaceItem* item) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  AddWatchForParent(item->file_path());
 }
 
 void HoldingSpaceFileSystemDelegate::OnHoldingSpaceItemFinalized(
     const HoldingSpaceItem* item) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   AddWatchForParent(item->file_path());
 }
 
 void HoldingSpaceFileSystemDelegate::OnVolumeMounted(
     chromeos::MountError error_code,
     const file_manager::Volume& volume) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   holding_space_util::FilePathsWithValidityRequirements
       file_paths_with_requirements;
   // Check validity of partially initialized items under the volume's mount
@@ -196,6 +198,7 @@ void HoldingSpaceFileSystemDelegate::OnVolumeMounted(
 void HoldingSpaceFileSystemDelegate::OnVolumeUnmounted(
     chromeos::MountError error_code,
     const file_manager::Volume& volume) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   // Schedule task to remove items under the unmounted file path from the model.
   // During suspend, some volumes get unmounted - for example, drive FS. The
   // file system delegate gets shutdown to avoid removing items from unmounted
@@ -209,6 +212,38 @@ void HoldingSpaceFileSystemDelegate::OnVolumeUnmounted(
                      weak_factory_.GetWeakPtr(), volume.mount_path()));
 }
 
+void HoldingSpaceFileSystemDelegate::OnFileMoved(
+    const storage::FileSystemURL& src,
+    const storage::FileSystemURL& dst) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  // Update backing files for any holding space `item` associated with `src`.
+  bool did_update_backing_file = false;
+  for (auto& item : model()->items()) {
+    base::FilePath new_file_path;
+    if (src.path() == item->file_path()) {
+      // The file backing `item` has moved to `dst`.
+      new_file_path = dst.path();
+    } else if (src.path().IsParent(item->file_path())) {
+      // A parent directory of the file backing `item` has moved to `dst` so
+      // the file backing `item` needs to be re-parented.
+      new_file_path = dst.path();
+      if (!src.path().AppendRelativePath(item->file_path(), &new_file_path))
+        NOTREACHED();
+    }
+    if (!new_file_path.empty()) {
+      model()->UpdateBackingFileForItem(
+          item->id(), new_file_path,
+          holding_space_util::ResolveFileSystemUrl(profile(), new_file_path));
+      did_update_backing_file = true;
+    }
+  }
+  // If a backing file update occurred, it's possible that there are no longer
+  // any holding space items associated with `src`. When that is the case, `src`
+  // no longer needs to be watched.
+  if (did_update_backing_file)
+    MaybeRemoveWatch(src.path());
+}
+
 void HoldingSpaceFileSystemDelegate::OnFilePathChanged(
     const base::FilePath& file_path,
     bool error) {
@@ -228,6 +263,7 @@ void HoldingSpaceFileSystemDelegate::OnFilePathChanged(
 
 void HoldingSpaceFileSystemDelegate::ScheduleFilePathValidityCheck(
     holding_space_util::FilePathWithValidityRequirement requirement) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   pending_file_path_validity_checks_.push_back(std::move(requirement));
   if (file_path_validity_checks_scheduled_)
     return;
@@ -245,6 +281,7 @@ void HoldingSpaceFileSystemDelegate::ScheduleFilePathValidityCheck(
 }
 
 void HoldingSpaceFileSystemDelegate::RunPendingFilePathValidityChecks() {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   holding_space_util::FilePathsWithValidityRequirements requirements;
   requirements.swap(pending_file_path_validity_checks_);
 
@@ -259,6 +296,7 @@ void HoldingSpaceFileSystemDelegate::RunPendingFilePathValidityChecks() {
 void HoldingSpaceFileSystemDelegate::OnFilePathValidityChecksComplete(
     std::vector<base::FilePath> valid_paths,
     std::vector<base::FilePath> invalid_paths) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   // Remove items with invalid paths.
   // When `file_path` is removed, we need to remove any associated items.
   model()->RemoveIf(base::BindRepeating(
@@ -291,9 +329,20 @@ void HoldingSpaceFileSystemDelegate::AddWatchForParent(
                                 file_system_watcher_->GetWeakPtr(), file_path));
 }
 
-void HoldingSpaceFileSystemDelegate::RemoveWatch(
+void HoldingSpaceFileSystemDelegate::MaybeRemoveWatch(
     const base::FilePath& file_path) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  // The watch for `file_path` should only be removed if no holding space items
+  // exist in the model which are backed by files it directly parents.
+  const bool remove_watch = std::none_of(
+      model()->items().begin(), model()->items().end(),
+      [&file_path](const auto& item) {
+        return item->IsFinalized() && item->file_path().DirName() == file_path;
+      });
+
+  if (!remove_watch)
+    return;
+
   file_system_watcher_runner_->PostTask(
       FROM_HERE, base::BindOnce(&FileSystemWatcher::RemoveWatch,
                                 file_system_watcher_->GetWeakPtr(), file_path));
@@ -301,6 +350,7 @@ void HoldingSpaceFileSystemDelegate::RemoveWatch(
 
 void HoldingSpaceFileSystemDelegate::RemoveItemsParentedByPath(
     const base::FilePath& parent_path) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   model()->RemoveIf(base::BindRepeating(
       [](const base::FilePath& parent_path, const HoldingSpaceItem* item) {
         return parent_path.IsParent(item->file_path());
@@ -309,6 +359,7 @@ void HoldingSpaceFileSystemDelegate::RemoveItemsParentedByPath(
 }
 
 void HoldingSpaceFileSystemDelegate::ClearNonFinalizedItems() {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   model()->RemoveIf(base::BindRepeating(
       [](const HoldingSpaceItem* item) { return !item->IsFinalized(); }));
 }

chrome/browser/ui/ash/holding_space/holding_space_file_system_delegate.h

@@ -8,10 +8,12 @@
 #include <memory>
 
 #include "base/callback.h"
-#include "base/scoped_observer.h"
+#include "base/scoped_observation.h"
 #include "base/timer/timer.h"
 #include "chrome/browser/chromeos/file_manager/volume_manager.h"
 #include "chrome/browser/chromeos/file_manager/volume_manager_observer.h"
+#include "chrome/browser/chromeos/fileapi/file_change_service.h"
+#include "chrome/browser/chromeos/fileapi/file_change_service_observer.h"
 #include "chrome/browser/ui/ash/holding_space/holding_space_keyed_service_delegate.h"
 #include "chrome/browser/ui/ash/holding_space/holding_space_util.h"
 
@@ -25,10 +27,12 @@ namespace ash {
 // files backing holding space items. The delegate:
 // *  Finalizes partially initialized items loaded from persistent storage once
 //    the validity of the backing file path was verified.
-// *  Monitors the file system for removal of files backing holding space
-//    items.
-class HoldingSpaceFileSystemDelegate : public HoldingSpaceKeyedServiceDelegate,
-                                       file_manager::VolumeManagerObserver {
+// *  Monitors the file system for removal, rename, and move of files backing
+//    holding space items.
+class HoldingSpaceFileSystemDelegate
+    : public HoldingSpaceKeyedServiceDelegate,
+      public chromeos::FileChangeServiceObserver,
+      public file_manager::VolumeManagerObserver {
  public:
   HoldingSpaceFileSystemDelegate(Profile* profile, HoldingSpaceModel* model);
   HoldingSpaceFileSystemDelegate(const HoldingSpaceFileSystemDelegate&) =
@@ -44,6 +48,7 @@ class HoldingSpaceFileSystemDelegate : public HoldingSpaceKeyedServiceDelegate,
   void Init() override;
   void OnHoldingSpaceItemAdded(const HoldingSpaceItem* item) override;
   void OnHoldingSpaceItemRemoved(const HoldingSpaceItem* item) override;
+  void OnHoldingSpaceItemUpdated(const HoldingSpaceItem* item) override;
   void OnHoldingSpaceItemFinalized(const HoldingSpaceItem* item) override;
 
   // file_manager::VolumeManagerObserver:
@@ -52,6 +57,10 @@ class HoldingSpaceFileSystemDelegate : public HoldingSpaceKeyedServiceDelegate,
   void OnVolumeUnmounted(chromeos::MountError error_code,
                          const file_manager::Volume& volume) override;
 
+  // chromeos::FileChangeServiceObserver:
+  void OnFileMoved(const storage::FileSystemURL& src,
+                   const storage::FileSystemURL& dst) override;
+
   // Invoked when the specified `file_path` has changed.
   void OnFilePathChanged(const base::FilePath& file_path, bool error);
 
@@ -73,10 +82,12 @@ class HoldingSpaceFileSystemDelegate : public HoldingSpaceKeyedServiceDelegate,
       std::vector<base::FilePath> invalid_paths);
 
   // Adds/removes a watch for the specified `file_path`.
-  // Note that `AddWatchForParent` will add a watch for the `file_path`'s parent
-  // directory.
+  // Note that `AddWatchForParent()` will add a watch for the `file_path`'s
+  // parent directory. Also note that `MaybeRemoveWatch()` will only remove the
+  // watch for `file_path` if no backing file for a holding space item exists
+  // which is directly parented by it.
   void AddWatchForParent(const base::FilePath& file_path);
-  void RemoveWatch(const base::FilePath& file_path);
+  void MaybeRemoveWatch(const base::FilePath& file_path);
 
   // Removes items that are (transitively) parented by `parent_path` from the
   // holding space model.
@@ -108,8 +119,12 @@ class HoldingSpaceFileSystemDelegate : public HoldingSpaceKeyedServiceDelegate,
   // to has not been yet mounted).
   base::OneShotTimer clear_non_finalized_items_timer_;
 
-  ScopedObserver<file_manager::VolumeManager,
-                 file_manager::VolumeManagerObserver>
+  base::ScopedObservation<chromeos::FileChangeService,
+                          chromeos::FileChangeServiceObserver>
+      file_change_service_observer_{this};
+
+  base::ScopedObservation<file_manager::VolumeManager,
+                          file_manager::VolumeManagerObserver>
       volume_manager_observer_{this};
 
   base::WeakPtrFactory<HoldingSpaceFileSystemDelegate> weak_factory_{this};

chrome/browser/ui/ash/holding_space/holding_space_keyed_service_factory.cc

@@ -6,6 +6,7 @@
 
 #include "ash/public/cpp/ash_features.h"
 #include "chrome/browser/chromeos/file_manager/volume_manager_factory.h"
+#include "chrome/browser/chromeos/fileapi/file_change_service_factory.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/ash/holding_space/holding_space_keyed_service.h"
@@ -26,6 +27,7 @@ HoldingSpaceKeyedServiceFactory::HoldingSpaceKeyedServiceFactory()
     : BrowserContextKeyedServiceFactory(
           "HoldingSpaceService",
           BrowserContextDependencyManager::GetInstance()) {
+  DependsOn(chromeos::FileChangeServiceFactory::GetInstance());
   DependsOn(file_manager::VolumeManagerFactory::GetInstance());
 }
 

chrome/browser/ui/ash/holding_space/holding_space_keyed_service_unittest.cc

@@ -51,6 +51,7 @@
 #include "storage/browser/file_system/external_mount_points.h"
 #include "storage/browser/file_system/file_system_context.h"
 #include "storage/browser/file_system/file_system_url.h"
+#include "storage/browser/test/async_file_test_helper.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/skia/include/core/SkBitmap.h"
@@ -121,6 +122,43 @@ class HoldingSpaceModelAttachedWaiter : public HoldingSpaceControllerObserver {
   std::unique_ptr<base::RunLoop> wait_loop_;
 };
 
+class ItemUpdatedWaiter : public HoldingSpaceModelObserver {
+ public:
+  explicit ItemUpdatedWaiter(HoldingSpaceModel* model) {
+    model_observer_.Observe(model);
+  }
+
+  ItemUpdatedWaiter(const ItemUpdatedWaiter&) = delete;
+  ItemUpdatedWaiter& operator=(const ItemUpdatedWaiter&) = delete;
+  ~ItemUpdatedWaiter() override = default;
+
+  void Wait(const HoldingSpaceItem* item) {
+    ASSERT_FALSE(wait_item_);
+    ASSERT_FALSE(wait_loop_);
+
+    wait_item_ = item;
+
+    wait_loop_ = std::make_unique<base::RunLoop>();
+    wait_loop_->Run();
+    wait_loop_.reset();
+
+    wait_item_ = nullptr;
+  }
+
+ private:
+  // HoldingSpaceModelObserver:
+  void OnHoldingSpaceItemUpdated(const HoldingSpaceItem* item) override {
+    if (item == wait_item_)
+      wait_loop_->Quit();
+  }
+
+  const HoldingSpaceItem* wait_item_ = nullptr;
+  std::unique_ptr<base::RunLoop> wait_loop_;
+
+  base::ScopedObservation<HoldingSpaceModel, HoldingSpaceModelObserver>
+      model_observer_{this};
+};
+
 class ItemsFinalizedWaiter : public HoldingSpaceModelObserver {
  public:
   // Predicate that determines whether the waiter should wait for an item to be
@@ -539,6 +577,125 @@ TEST_F(HoldingSpaceKeyedServiceTest, UpdatePersistentStorage) {
   }
 }
 
+// Verifies that when a file backing a holding space item is moved, the holding
+// space item is updated in place and persistence storage is updated.
+TEST_F(HoldingSpaceKeyedServiceTest, UpdatePersistentStorageAfterMove) {
+  // Create a file system mount point.
+  std::unique_ptr<ScopedTestMountPoint> downloads_mount =
+      ScopedTestMountPoint::CreateAndMountDownloads(GetProfile());
+  ASSERT_TRUE(downloads_mount->IsValid());
+
+  // Cache the holding space model for the primary profile.
+  HoldingSpaceKeyedService* const primary_holding_space_service =
+      HoldingSpaceKeyedServiceFactory::GetInstance()->GetService(GetProfile());
+  HoldingSpaceModel* const primary_holding_space_model =
+      HoldingSpaceController::Get()->model();
+  ASSERT_EQ(primary_holding_space_model,
+            primary_holding_space_service->model_for_testing());
+
+  // Cache the file system context.
+  storage::FileSystemContext* context =
+      file_manager::util::GetFileSystemContextForExtensionId(
+          GetProfile(), file_manager::kFileManagerAppId);
+  ASSERT_TRUE(context);
+
+  base::ListValue persisted_holding_space_items;
+
+  // Verify persistent storage is updated when adding each type of item.
+  for (const HoldingSpaceItem::Type type : GetHoldingSpaceItemTypes()) {
+    // Note that each item is being added to a unique parent directory so that
+    // moving the parent directory later will not affect other items.
+    const base::FilePath file_path = downloads_mount->CreateFile(
+        base::FilePath(base::NumberToString(static_cast<int>(type)))
+            .Append("foo.txt"),
+        /*content=*/std::string());
+    const GURL file_system_url = GetFileSystemUrl(GetProfile(), file_path);
+
+    // Create the holding space item.
+    auto holding_space_item = HoldingSpaceItem::CreateFileBackedItem(
+        type, file_path, file_system_url,
+        holding_space_util::ResolveImage(
+            primary_holding_space_service->thumbnail_loader_for_testing(), type,
+            file_path));
+
+    // Add the holding space item to the model and verify persistence.
+    persisted_holding_space_items.Append(holding_space_item->Serialize());
+    primary_holding_space_model->AddItem(std::move(holding_space_item));
+    EXPECT_EQ(*GetProfile()->GetPrefs()->GetList(
+                  HoldingSpacePersistenceDelegate::kPersistencePath),
+              persisted_holding_space_items);
+  }
+
+  // Verify persistent storage is updated when moving each type of item and
+  // that the holding space items themselves are updated in place.
+  for (size_t i = 0; i < primary_holding_space_model->items().size(); ++i) {
+    const auto* holding_space_item =
+        primary_holding_space_model->items()[i].get();
+
+    // Rename the file backing the holding space item.
+    base::FilePath file_path = holding_space_item->file_path();
+    base::FilePath new_file_path = file_path.InsertBeforeExtension(" (Moved)");
+    GURL file_path_url = GetFileSystemUrl(GetProfile(), file_path);
+    GURL new_file_path_url = GetFileSystemUrl(GetProfile(), new_file_path);
+    ASSERT_EQ(storage::AsyncFileTestHelper::Move(
+                  context, context->CrackURL(file_path_url),
+                  context->CrackURL(new_file_path_url)),
+              base::File::FILE_OK);
+
+    // File changes must be posted to the UI thread, wait for the update to
+    // reach the holding space model.
+    ItemUpdatedWaiter(primary_holding_space_model).Wait(holding_space_item);
+
+    // Verify that the holding space item has been updated in place.
+    ASSERT_EQ(holding_space_item->file_path(), new_file_path);
+    ASSERT_EQ(holding_space_item->file_system_url(), new_file_path_url);
+    ASSERT_EQ(holding_space_item->text(),
+              new_file_path.BaseName().LossyDisplayName());
+
+    // Verify that persistence has been updated.
+    persisted_holding_space_items.GetList()[i] =
+        holding_space_item->Serialize();
+    ASSERT_EQ(*GetProfile()->GetPrefs()->GetList(
+                  HoldingSpacePersistenceDelegate::kPersistencePath),
+              persisted_holding_space_items);
+
+    // Cache the base name of the file backing the holding space item as it will
+    // not change due to rename of the holding space item's parent directory.
+    base::FilePath base_name = holding_space_item->file_path().BaseName();
+
+    // Rename the file backing the holding space item's parent directory.
+    file_path = new_file_path.DirName();
+    new_file_path = file_path.InsertBeforeExtension(" (Moved)");
+    file_path_url = GetFileSystemUrl(GetProfile(), file_path);
+    new_file_path_url = GetFileSystemUrl(GetProfile(), new_file_path);
+    ASSERT_EQ(storage::AsyncFileTestHelper::Move(
+                  context, context->CrackURL(file_path_url),
+                  context->CrackURL(new_file_path_url)),
+              base::File::FILE_OK);
+
+    // File changes must be posted to the UI thread, wait for the update to
+    // reach the holding space model.
+    ItemUpdatedWaiter(primary_holding_space_model).Wait(holding_space_item);
+
+    // The file backing the holding space item is expected to have re-parented.
+    new_file_path = new_file_path.Append(base_name);
+    new_file_path_url = GetFileSystemUrl(GetProfile(), new_file_path);
+
+    // Verify that the holding space item has been updated in place.
+    ASSERT_EQ(holding_space_item->file_path(), new_file_path);
+    ASSERT_EQ(holding_space_item->file_system_url(), new_file_path_url);
+    ASSERT_EQ(holding_space_item->text(),
+              new_file_path.BaseName().LossyDisplayName());
+
+    // Verify that persistence has been updated.
+    persisted_holding_space_items.GetList()[i] =
+        holding_space_item->Serialize();
+    ASSERT_EQ(*GetProfile()->GetPrefs()->GetList(
+                  HoldingSpacePersistenceDelegate::kPersistencePath),
+              persisted_holding_space_items);
+  }
+}
+
 // Verifies that the holding space model is restored from persistence. Note that
 // when restoring from persistence, existence of backing files is verified and
 // any stale holding space items are removed.

chrome/browser/ui/ash/holding_space/holding_space_persistence_delegate.cc

@@ -66,6 +66,24 @@ void HoldingSpacePersistenceDelegate::OnHoldingSpaceItemRemoved(
   });
 }
 
+void HoldingSpacePersistenceDelegate::OnHoldingSpaceItemUpdated(
+    const HoldingSpaceItem* item) {
+  if (is_restoring_persistence())
+    return;
+
+  // Update the `item` in persistent storage.
+  ListPrefUpdate update(profile()->GetPrefs(), kPersistencePath);
+  auto item_it = std::find_if(
+      update->begin(), update->end(),
+      [&item](const base::Value& persisted_item) {
+        return HoldingSpaceItem::DeserializeId(base::Value::AsDictionaryValue(
+                   persisted_item)) == item->id();
+      });
+
+  DCHECK(item_it != update->end());
+  *item_it = item->Serialize();
+}
+
 void HoldingSpacePersistenceDelegate::RestoreModelFromPersistence() {
   DCHECK(model()->items().empty());
 

chrome/browser/ui/ash/holding_space/holding_space_persistence_delegate.h

@@ -62,6 +62,7 @@ class HoldingSpacePersistenceDelegate
   void Init() override;
   void OnHoldingSpaceItemAdded(const HoldingSpaceItem* item) override;
   void OnHoldingSpaceItemRemoved(const HoldingSpaceItem* item) override;
+  void OnHoldingSpaceItemUpdated(const HoldingSpaceItem* item) override;
 
   // Restores the holding space model from persistent storage.
   void RestoreModelFromPersistence();