FSO: Destruct the file observation when the file is deleted
This change updates FileSystemAccessObserverObservation::OnChanges to send an errored event and destruct the browser side observation when the file under observation is deleted. These updates also allows us to remove some (now) dead code in FileSystemAccessLocalPathWatcher. Bug: 350791549 Change-Id: I03409194ef6e58fc455c76b001bf802e68523d7e Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6015025 Reviewed-by: Nathan Memmott <memmott@chromium.org> Commit-Queue: Rahul Singh <rahsin@microsoft.com> Cr-Commit-Position: refs/heads/main@{#1383341}
This commit is contained in:

committed by
Chromium LUCI CQ

parent
98242bfda0
commit
fda19616dc
content/browser/file_system_access
@ -136,8 +136,6 @@ void FileSystemAccessLocalPathWatcher::OnFilePathChanged(
|
|||||||
FilePathWatcher::FilePathType::kFile,
|
FilePathWatcher::FilePathType::kFile,
|
||||||
FilePathWatcher::ChangeType::kDeleted, root_path};
|
FilePathWatcher::ChangeType::kDeleted, root_path};
|
||||||
NotifyOfChange(base::FilePath(), error, file_change_info);
|
NotifyOfChange(base::FilePath(), error, file_change_info);
|
||||||
NotifyOfChange(base::FilePath(), /*error=*/true,
|
|
||||||
FilePathWatcher::ChangeInfo());
|
|
||||||
} else if (change_info.moved_from_path == root_path) {
|
} else if (change_info.moved_from_path == root_path) {
|
||||||
// If the file is renamed, we report a kDeleted event for the `root_path`.
|
// If the file is renamed, we report a kDeleted event for the `root_path`.
|
||||||
// If we were directly watching the file, this would be handled in the
|
// If we were directly watching the file, this would be handled in the
|
||||||
|
@ -190,6 +190,11 @@ void FileSystemAccessObserverObservation::OnChanges(
|
|||||||
changes_or_error) {
|
changes_or_error) {
|
||||||
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
|
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
|
||||||
|
|
||||||
|
// We should never receive changes for an observation with
|
||||||
|
// `WatchType::kAllBucketFileSystems`.
|
||||||
|
CHECK(observation_->scope().GetWatchType() !=
|
||||||
|
FileSystemAccessWatchScope::WatchType::kAllBucketFileSystems);
|
||||||
|
|
||||||
if (received_error_while_in_bf_cache_) {
|
if (received_error_while_in_bf_cache_) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
@ -319,10 +324,6 @@ void FileSystemAccessObserverObservation::OnChanges(
|
|||||||
}
|
}
|
||||||
|
|
||||||
observation_root_disappeared =
|
observation_root_disappeared =
|
||||||
(observation_->scope().GetWatchType() ==
|
|
||||||
FileSystemAccessWatchScope::WatchType::kDirectoryRecursive ||
|
|
||||||
observation_->scope().GetWatchType() ==
|
|
||||||
FileSystemAccessWatchScope::WatchType::kDirectoryNonRecursive) &&
|
|
||||||
mojo_change_type->is_disappeared() &&
|
mojo_change_type->is_disappeared() &&
|
||||||
observation_->scope().root_url() == change.url;
|
observation_->scope().root_url() == change.url;
|
||||||
|
|
||||||
|
@ -501,7 +501,7 @@ TEST_F(FileSystemAccessObserverObservationTest,
|
|||||||
}
|
}
|
||||||
|
|
||||||
TEST_F(FileSystemAccessObserverObservationTest,
|
TEST_F(FileSystemAccessObserverObservationTest,
|
||||||
FileObservationNotDestructedAfterFileDeleted) {
|
FileObservationDestructedAfterFileDeleted) {
|
||||||
base::FilePath file_path = CreateFile();
|
base::FilePath file_path = CreateFile();
|
||||||
storage::FileSystemURL file_url = CreateFileSystemURL(file_path);
|
storage::FileSystemURL file_url = CreateFileSystemURL(file_path);
|
||||||
std::unique_ptr<FileSystemAccessFileHandleImpl> file_handle =
|
std::unique_ptr<FileSystemAccessFileHandleImpl> file_handle =
|
||||||
@ -517,21 +517,22 @@ TEST_F(FileSystemAccessObserverObservationTest,
|
|||||||
FakeObservation observation =
|
FakeObservation observation =
|
||||||
observer.Observe(file_handle.get(), /*recursive=*/false);
|
observer.Observe(file_handle.get(), /*recursive=*/false);
|
||||||
|
|
||||||
// Deleting the file should only result in a disappeared event.
|
// Deleting the file should result in a disappeared and an errored event.
|
||||||
source.SignalChange(
|
source.SignalChange(
|
||||||
ChangeInfo(FilePathType::kFile, ChangeType::kCreated, file_path));
|
ChangeInfo(FilePathType::kFile, ChangeType::kCreated, file_path));
|
||||||
source.SignalChange(
|
source.SignalChange(
|
||||||
ChangeInfo(FilePathType::kFile, ChangeType::kDeleted, file_path));
|
ChangeInfo(FilePathType::kFile, ChangeType::kDeleted, file_path));
|
||||||
EXPECT_TRUE(observation.EventsReceivedMatches(
|
EXPECT_TRUE(observation.EventsReceivedMatches(
|
||||||
{{MojoChangeType::kAppeared, MojoFilePathType::kFile, {}},
|
{{MojoChangeType::kAppeared, MojoFilePathType::kFile, {}},
|
||||||
{MojoChangeType::kDisappeared, MojoFilePathType::kFile, {}}}));
|
{MojoChangeType::kDisappeared, MojoFilePathType::kFile, {}},
|
||||||
|
{MojoChangeType::kErrored, MojoFilePathType::kFile, {}}}));
|
||||||
|
|
||||||
// The remote observation should not have been destructed and disconnected.
|
// The remote observation should have been destructed and disconnected. So,
|
||||||
// So, further events should be received.
|
// further events should not be received.
|
||||||
source.SignalChange(
|
source.SignalChange(
|
||||||
ChangeInfo(FilePathType::kFile, ChangeType::kModified, file_path));
|
ChangeInfo(FilePathType::kFile, ChangeType::kModified, file_path));
|
||||||
EXPECT_TRUE(observation.EventsReceivedMatches(
|
EXPECT_TRUE(observation.EventsReceivedMatches({}));
|
||||||
{{MojoChangeType::kModified, MojoFilePathType::kFile, {}}}));
|
EXPECT_TRUE(observation.RemoteObservationDisconnected());
|
||||||
}
|
}
|
||||||
|
|
||||||
TEST_F(FileSystemAccessObserverObservationTest,
|
TEST_F(FileSystemAccessObserverObservationTest,
|
||||||
@ -561,11 +562,9 @@ TEST_F(FileSystemAccessObserverObservationTest,
|
|||||||
// event on the file observation.
|
// event on the file observation.
|
||||||
source.SignalChange(
|
source.SignalChange(
|
||||||
ChangeInfo(FilePathType::kFile, ChangeType::kModified, file_path));
|
ChangeInfo(FilePathType::kFile, ChangeType::kModified, file_path));
|
||||||
// We handle directory deletion by first signaling a deleted event on the file
|
// We handle directory deletion by signaling a deleted event on the file
|
||||||
// and then an error.
|
|
||||||
source.SignalChange(
|
source.SignalChange(
|
||||||
ChangeInfo(FilePathType::kFile, ChangeType::kDeleted, file_path));
|
ChangeInfo(FilePathType::kFile, ChangeType::kDeleted, file_path));
|
||||||
source.SignalError();
|
|
||||||
EXPECT_TRUE(observation.EventsReceivedMatches(
|
EXPECT_TRUE(observation.EventsReceivedMatches(
|
||||||
{{MojoChangeType::kModified, MojoFilePathType::kFile, {}},
|
{{MojoChangeType::kModified, MojoFilePathType::kFile, {}},
|
||||||
{MojoChangeType::kDisappeared, MojoFilePathType::kFile, {}},
|
{MojoChangeType::kDisappeared, MojoFilePathType::kFile, {}},
|
||||||
|
Reference in New Issue
Block a user