0

Change Observers: Add Observation Groups

Adds the observation group as a middle man between Observations and the
Watcher Manager.

No functional change. Behavior will change once it is connected to the
Observation Quota Manager.

Fuchsia-Binary-Size: Size increase is unavoidable.
Bug: 338457523
Change-Id: Ib6e1e1477ee0296ab0f4ab7f261874258485ebee
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6026981
Reviewed-by: Daseul Lee <dslee@chromium.org>
Reviewed-by: Rahul Singh <rahsin@microsoft.com>
Commit-Queue: Nathan Memmott <memmott@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1387088}
This commit is contained in:
Nathan Memmott
2024-11-22 22:25:37 +00:00
committed by Chromium LUCI CQ
parent 39939356bf
commit fd6322e88f
11 changed files with 491 additions and 206 deletions

@ -1052,6 +1052,8 @@ source_set("browser") {
"file_system_access/file_system_access_lock_manager.h",
"file_system_access/file_system_access_manager_impl.cc",
"file_system_access/file_system_access_manager_impl.h",
"file_system_access/file_system_access_observation_group.cc",
"file_system_access/file_system_access_observation_group.h",
"file_system_access/file_system_access_observer_host.cc",
"file_system_access/file_system_access_observer_host.h",
"file_system_access/file_system_access_observer_observation.cc",

@ -0,0 +1,119 @@
// Copyright 2024 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "content/browser/file_system_access/file_system_access_observation_group.h"
#include "base/memory/ptr_util.h"
#include "content/browser/file_system_access/file_system_access_watcher_manager.h"
namespace content {
FileSystemAccessObservationGroup::Change::Change(
storage::FileSystemURL url,
FileSystemAccessChangeSource::ChangeInfo change_info)
: url(std::move(url)), change_info(std::move(change_info)) {}
FileSystemAccessObservationGroup::Change::~Change() = default;
FileSystemAccessObservationGroup::Change::Change(
const FileSystemAccessObservationGroup::Change& other)
: url(other.url), change_info(std::move(other.change_info)) {}
FileSystemAccessObservationGroup::Change::Change(
FileSystemAccessObservationGroup::Change&&) noexcept = default;
FileSystemAccessObservationGroup::Change&
FileSystemAccessObservationGroup::Change::operator=(
const FileSystemAccessObservationGroup::Change&) = default;
FileSystemAccessObservationGroup::Change&
FileSystemAccessObservationGroup::Change::operator=(
FileSystemAccessObservationGroup::Change&&) noexcept = default;
FileSystemAccessObservationGroup::Observer::Observer(
FileSystemAccessObservationGroup& observation_group) {
obs_.Observe(&observation_group);
}
FileSystemAccessObservationGroup::Observer::~Observer() = default;
void FileSystemAccessObservationGroup::Observer::SetCallback(
OnChangesCallback on_change_callback) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
CHECK(!on_change_callback_);
on_change_callback_ = std::move(on_change_callback);
}
void FileSystemAccessObservationGroup::Observer::NotifyOfChanges(
const std::optional<std::list<Change>>& changes_or_error) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
if (on_change_callback_) {
on_change_callback_.Run(std::move(changes_or_error));
}
}
FileSystemAccessObservationGroup::FileSystemAccessObservationGroup(
FileSystemAccessWatcherManager& watcher_manager,
FileSystemAccessWatchScope scope,
base::PassKey<FileSystemAccessWatcherManager> pass_key)
: scope_(std::move(scope)), watcher_manager_(watcher_manager) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
watcher_manager_->AddObserver(this);
}
FileSystemAccessObservationGroup::~FileSystemAccessObservationGroup() {
watcher_manager_->RemoveObserver(this);
}
std::unique_ptr<FileSystemAccessObservationGroup::Observer>
FileSystemAccessObservationGroup::CreateObserver() {
// Not using `std::make_unique` so that `Observer`'s constructor can remain
// private.
return base::WrapUnique(new Observer(*this));
}
void FileSystemAccessObservationGroup::AddObserver(Observer* observation) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
observations_.AddObserver(observation);
}
void FileSystemAccessObservationGroup::RemoveObserver(Observer* observation) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
observations_.RemoveObserver(observation);
// `FileSystemAccessObservationGroup` lifetime is tied to its observations, so
// destroy this when there are no more observations.
if (observations_.empty()) {
// `this` is destroyed after `RemoveObservationGroup` is called.
watcher_manager_->RemoveObservationGroup(scope_);
}
}
void FileSystemAccessObservationGroup::NotifyOfChanges(
const std::optional<std::list<Change>>& changes_or_error) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
for (auto& observation : observations_) {
observation.NotifyOfChanges(changes_or_error);
}
}
void FileSystemAccessObservationGroup::NotifyOfUsageChange(size_t old_usage,
size_t new_usage) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
if (on_usage_change_callback_) {
on_usage_change_callback_.Run(old_usage, new_usage);
}
// TODO(crbug.com/338457523): Notify the quota manager.
}
void FileSystemAccessObservationGroup::SetOnUsageCallbackForTesting(
OnUsageChangeCallback on_usage_change_callback) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
on_usage_change_callback_ = std::move(on_usage_change_callback);
}
} // namespace content

@ -0,0 +1,175 @@
// Copyright 2024 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef CONTENT_BROWSER_FILE_SYSTEM_ACCESS_FILE_SYSTEM_ACCESS_OBSERVATION_GROUP_H_
#define CONTENT_BROWSER_FILE_SYSTEM_ACCESS_FILE_SYSTEM_ACCESS_OBSERVATION_GROUP_H_
#include <list>
#include "base/functional/callback.h"
#include "base/observer_list_types.h"
#include "base/scoped_observation.h"
#include "base/scoped_observation_traits.h"
#include "content/browser/file_system_access/file_system_access_change_source.h"
#include "content/browser/file_system_access/file_system_access_watch_scope.h"
namespace content {
class FileSystemAccessWatcherManager;
// Represents a group of observations that all have the same
// `FileSystemAccessChangeSource`. This is created and maintained by the
// `FileSystemAccessWatcherManager`.
//
// Instances of this class must be accessed exclusively on the UI thread. Owned
// by the `FileSystemAccessManagerImpl`.
//
// TODO(crbug.com/338457523): All observations in the group should also have the
// same storage key. And this class should also be associated with an
// observation quota manager for the storage key.
//
// TODO(crbug.com/376134535): Once the watcher manager layer is removed, the
// base class should be `FileSystemAccessChangeSource::RawChangeObserver` rather
// than `base::CheckedObserver`.
class CONTENT_EXPORT FileSystemAccessObservationGroup
: public base::CheckedObserver {
public:
// Describes a change to some location in a file system.
struct CONTENT_EXPORT Change {
Change(storage::FileSystemURL url,
FileSystemAccessChangeSource::ChangeInfo change_info);
~Change();
// Copyable and movable.
Change(const Change&);
Change(Change&&) noexcept;
Change& operator=(const Change&);
Change& operator=(Change&&) noexcept;
storage::FileSystemURL url;
FileSystemAccessChangeSource::ChangeInfo change_info;
bool operator==(const Change& other) const {
return url == other.url && change_info == other.change_info;
}
};
// An observer of a `FileSystemAccessObservationGroup`.
//
// The common source/observer pattern is for the source to provide an abstract
// class for observers to implement. Here we instead fully implement the
// observer and let end observers set a callback for its events.
//
// This pattern is chosen because we want the lifetime of the
// `FileSystemAccessObservationGroup` to be tied to its `Observer`. We don't
// want there to be an empty `FileSystemAccessObservationGroup`, so the
// creator of the `FileSystemAccessObservationGroup` should be able to
// immediately create an `Observer`.
class CONTENT_EXPORT Observer : public base::CheckedObserver {
public:
using OnChangesCallback = base::RepeatingCallback<void(
const std::optional<std::list<Change>>& changes_or_error)>;
~Observer() override;
// Not copyable or movable.
Observer(const Observer&) = delete;
Observer(Observer&&) = delete;
Observer& operator=(const Observer&) = delete;
Observer& operator=(Observer&&) = delete;
// Set the callback to which changes will be reported. It is illegal to call
// this method more than once.
void SetCallback(OnChangesCallback on_change_callback);
const FileSystemAccessWatchScope& scope() const {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
return obs_.GetSource()->scope();
}
FileSystemAccessObservationGroup& GetObservationGroupForTesting() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
return *obs_.GetSource();
}
private:
friend FileSystemAccessObservationGroup;
explicit Observer(FileSystemAccessObservationGroup& observation_group);
void NotifyOfChanges(
const std::optional<std::list<Change>>& changes_or_error);
SEQUENCE_CHECKER(sequence_checker_);
OnChangesCallback on_change_callback_ GUARDED_BY_CONTEXT(sequence_checker_);
base::ScopedObservation<FileSystemAccessObservationGroup, Observer> obs_
GUARDED_BY_CONTEXT(sequence_checker_){this};
};
using OnUsageChangeCallback = FilePathWatcher::UsageChangeCallback;
explicit FileSystemAccessObservationGroup(
FileSystemAccessWatcherManager& watcher_manager,
FileSystemAccessWatchScope scope,
base::PassKey<FileSystemAccessWatcherManager> pass_key);
~FileSystemAccessObservationGroup() override;
// Not copyable or movable.
FileSystemAccessObservationGroup(const FileSystemAccessObservationGroup&) =
delete;
FileSystemAccessObservationGroup(FileSystemAccessObservationGroup&&) = delete;
FileSystemAccessObservationGroup& operator=(
const FileSystemAccessObservationGroup&) = delete;
FileSystemAccessObservationGroup& operator=(
FileSystemAccessObservationGroup&&) = delete;
// Set the callback to which usage changes will be reported. It is illegal to
// call this method more than once.
void SetOnUsageCallbackForTesting(
OnUsageChangeCallback on_usage_change_callback);
private:
friend FileSystemAccessWatcherManager;
friend base::ScopedObservationTraits<FileSystemAccessObservationGroup,
Observer>;
std::unique_ptr<Observer> CreateObserver();
void AddObserver(Observer* observer);
void RemoveObserver(Observer* observer);
void NotifyOfChanges(
const std::optional<std::list<Change>>& changes_or_error);
void NotifyOfUsageChange(size_t old_usage, size_t new_usage);
const FileSystemAccessWatchScope& scope() const { return scope_; }
SEQUENCE_CHECKER(sequence_checker_);
const FileSystemAccessWatchScope scope_;
// Observations to which this instance will notify of changes within their
// respective scope.
base::ObserverList<Observer> observations_
GUARDED_BY_CONTEXT(sequence_checker_);
OnUsageChangeCallback on_usage_change_callback_;
// The `FileSystemAccessWatcherManager` that we're observing. Safe because
// `watcher_manager_` owns this.
//
// We use this instead of a `ScopedObservation` because
// `ScopedObservationTraits` needs a full interface definition rather than a
// forward declaration of `FileSystemAccessWatcherManager`. It's not worth
// defining a custom `ScopedObservationTraits` for the minimum value
// `ScopedObservation` brings.
base::raw_ref<FileSystemAccessWatcherManager> watcher_manager_;
};
} // namespace content
#endif // CONTENT_BROWSER_FILE_SYSTEM_ACCESS_FILE_SYSTEM_ACCESS_OBSERVATION_GROUP_H_

@ -246,7 +246,7 @@ void FileSystemAccessObserverHost::GotObservation(
absl::variant<std::unique_ptr<FileSystemAccessDirectoryHandleImpl>,
std::unique_ptr<FileSystemAccessFileHandleImpl>> handle,
ObserveCallback callback,
base::expected<std::unique_ptr<FileSystemAccessWatcherManager::Observation>,
base::expected<std::unique_ptr<FileSystemAccessObservationGroup::Observer>,
blink::mojom::FileSystemAccessErrorPtr>
observation_or_error) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);

@ -90,7 +90,7 @@ class FileSystemAccessObserverHost
std::unique_ptr<FileSystemAccessFileHandleImpl>> handle,
ObserveCallback callback,
base::expected<
std::unique_ptr<FileSystemAccessWatcherManager::Observation>,
std::unique_ptr<FileSystemAccessObservationGroup::Observer>,
blink::mojom::FileSystemAccessErrorPtr> observation_or_error);
void OnHostReceiverDisconnect();

@ -140,7 +140,7 @@ bool RenderFrameHostIsActive(
FileSystemAccessObserverObservation::FileSystemAccessObserverObservation(
FileSystemAccessObserverHost* host,
std::unique_ptr<FileSystemAccessWatcherManager::Observation> observation,
std::unique_ptr<FileSystemAccessObservationGroup::Observer> observation,
mojo::PendingRemote<blink::mojom::FileSystemAccessObserver> remote,
absl::variant<std::unique_ptr<FileSystemAccessDirectoryHandleImpl>,
std::unique_ptr<FileSystemAccessFileHandleImpl>> handle)
@ -185,8 +185,7 @@ const storage::FileSystemURL& FileSystemAccessObserverObservation::handle_url()
}
void FileSystemAccessObserverObservation::OnChanges(
const std::optional<
std::list<FileSystemAccessWatcherManager::Observation::Change>>&
const std::optional<std::list<FileSystemAccessObservationGroup::Change>>&
changes_or_error) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);

@ -9,6 +9,7 @@
#include "base/sequence_checker.h"
#include "base/thread_annotations.h"
#include "content/browser/file_system_access/file_system_access_observation_group.h"
#include "content/browser/file_system_access/file_system_access_watcher_manager.h"
#include "content/public/browser/web_contents_observer.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
@ -35,7 +36,7 @@ class FileSystemAccessObserverObservation
public:
FileSystemAccessObserverObservation(
FileSystemAccessObserverHost* host,
std::unique_ptr<FileSystemAccessWatcherManager::Observation> observation,
std::unique_ptr<FileSystemAccessObservationGroup::Observer> observation,
mojo::PendingRemote<blink::mojom::FileSystemAccessObserver> remote,
absl::variant<std::unique_ptr<FileSystemAccessDirectoryHandleImpl>,
std::unique_ptr<FileSystemAccessFileHandleImpl>> handle);
@ -64,8 +65,7 @@ class FileSystemAccessObserverObservation
// processes the received change data and sends a file change event via mojo
// pipe.
void OnChanges(
const std::optional<
std::list<FileSystemAccessWatcherManager::Observation::Change>>&
const std::optional<std::list<FileSystemAccessObservationGroup::Change>>&
changes_or_error);
// Invoked if an error occurred while watching file changes. It sends a file
@ -91,7 +91,7 @@ class FileSystemAccessObserverObservation
std::unique_ptr<FileSystemAccessFileHandleImpl>>
handle_;
std::unique_ptr<FileSystemAccessWatcherManager::Observation> observation_
std::unique_ptr<FileSystemAccessObservationGroup::Observer> observation_
GUARDED_BY_CONTEXT(sequence_checker_);
// Mojo pipes that send file change notifications back to the renderer.

@ -23,6 +23,7 @@
#include "content/browser/file_system_access/file_system_access_change_source.h"
#include "content/browser/file_system_access/file_system_access_error.h"
#include "content/browser/file_system_access/file_system_access_manager_impl.h"
#include "content/browser/file_system_access/file_system_access_observation_group.h"
#include "content/browser/file_system_access/file_system_access_observer_host.h"
#include "content/browser/file_system_access/file_system_access_observer_observation.h"
#include "content/browser/file_system_access/file_system_access_watch_scope.h"
@ -56,67 +57,6 @@ storage::FileSystemURL ToFileSystemURL(storage::FileSystemContext& context,
} // namespace
FileSystemAccessWatcherManager::Observation::Change::Change(
storage::FileSystemURL url,
FileSystemAccessChangeSource::ChangeInfo change_info)
: url(std::move(url)), change_info(std::move(change_info)) {}
FileSystemAccessWatcherManager::Observation::Change::~Change() = default;
FileSystemAccessWatcherManager::Observation::Change::Change(
const FileSystemAccessWatcherManager::Observation::Change& other)
: url(other.url), change_info(std::move(other.change_info)) {}
FileSystemAccessWatcherManager::Observation::Change::Change(
FileSystemAccessWatcherManager::Observation::Change&&) noexcept = default;
FileSystemAccessWatcherManager::Observation::Change&
FileSystemAccessWatcherManager::Observation::Change::operator=(
const FileSystemAccessWatcherManager::Observation::Change&) = default;
FileSystemAccessWatcherManager::Observation::Change&
FileSystemAccessWatcherManager::Observation::Change::operator=(
FileSystemAccessWatcherManager::Observation::Change&&) noexcept = default;
FileSystemAccessWatcherManager::Observation::Observation(
FileSystemAccessWatcherManager* watcher_manager,
FileSystemAccessWatchScope scope,
base::PassKey<FileSystemAccessWatcherManager> /*pass_key*/)
: scope_(std::move(scope)) {
CHECK(watcher_manager);
obs_.Observe(watcher_manager);
}
FileSystemAccessWatcherManager::Observation::~Observation() = default;
void FileSystemAccessWatcherManager::Observation::SetCallback(
OnChangesCallback on_change_callback) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
CHECK(!on_change_callback_);
on_change_callback_ = std::move(on_change_callback);
}
void FileSystemAccessWatcherManager::Observation::SetUsageCallback(
OnUsageChangesCallback on_usage_change_callback) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
CHECK(!on_usage_change_callback_);
on_usage_change_callback_ = std::move(on_usage_change_callback);
}
void FileSystemAccessWatcherManager::Observation::NotifyOfChanges(
const std::optional<std::list<Change>>& changes_or_error,
base::PassKey<FileSystemAccessWatcherManager> pass_key) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
if (on_change_callback_) {
on_change_callback_.Run(std::move(changes_or_error));
}
}
void FileSystemAccessWatcherManager::Observation::NotifyOfUsageChange(
size_t old_usage,
size_t new_usage) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
if (on_usage_change_callback_) {
on_usage_change_callback_.Run(old_usage, new_usage);
}
}
FileSystemAccessWatcherManager::FileSystemAccessWatcherManager(
FileSystemAccessManagerImpl* manager,
base::PassKey<FileSystemAccessManagerImpl> /*pass_key*/)
@ -222,11 +162,12 @@ void FileSystemAccessWatcherManager::OnRawChange(
change_info.moved_from_path.value()))
: std::nullopt;
const std::optional<std::list<Observation::Change>> changes_or_error =
error ? std::nullopt
: std::make_optional(
std::list<Observation::Change>({{changed_url, change_info}}));
for (auto& observation : observations_) {
const std::optional<std::list<Change>> changes_or_error =
error
? std::nullopt
: std::make_optional<std::list<Change>>({{changed_url, change_info}});
for (auto& observation_group : observation_groups_) {
// TODO(crbug.com/376134535): We identify a change source by its scope.
// Observations that have the same scope belong to that change source. The
// bucket file system being the exception.
@ -235,7 +176,7 @@ void FileSystemAccessWatcherManager::OnRawChange(
// ChangeSource. However we will have to do some refactoring because of the
// bucket file system exception.
if (scope.GetWatchType() != WatchType::kAllBucketFileSystems &&
observation.scope() != scope) {
observation_group.scope() != scope) {
continue;
}
@ -243,13 +184,14 @@ void FileSystemAccessWatcherManager::OnRawChange(
// observations based on their scope but based on the source the
// observations are tied to.
if (error) {
observation.NotifyOfChanges(
changes_or_error, base::PassKey<FileSystemAccessWatcherManager>());
observation_group.NotifyOfChanges(changes_or_error);
continue;
}
bool modified_url_in_scope = observation.scope().Contains(changed_url);
bool modified_url_in_scope =
observation_group.scope().Contains(changed_url);
bool moved_from_url_in_scope =
is_move_event && observation.scope().Contains(moved_from_url.value());
is_move_event &&
observation_group.scope().Contains(moved_from_url.value());
if (!modified_url_in_scope && !moved_from_url_in_scope) {
continue;
@ -263,10 +205,8 @@ void FileSystemAccessWatcherManager::OnRawChange(
change_info.file_path_type,
FileSystemAccessChangeSource::ChangeType::kCreated,
change_info.modified_path);
observation.NotifyOfChanges(
std::list<Observation::Change>(
{{changed_url, std::move(updated_change_info)}}),
base::PassKey<FileSystemAccessWatcherManager>());
observation_group.NotifyOfChanges(
std::list<Change>({{changed_url, std::move(updated_change_info)}}));
continue;
}
if (!modified_url_in_scope) {
@ -276,18 +216,15 @@ void FileSystemAccessWatcherManager::OnRawChange(
change_info.file_path_type,
FileSystemAccessChangeSource::ChangeType::kDeleted,
change_info.moved_from_path.value());
observation.NotifyOfChanges(
std::list<Observation::Change>(
{{moved_from_url.value(), std::move(updated_change_info)}}),
base::PassKey<FileSystemAccessWatcherManager>());
observation_group.NotifyOfChanges(std::list<Change>(
{{moved_from_url.value(), std::move(updated_change_info)}}));
continue;
}
}
// The default case, including move within scope, should notify the changes
// as is.
observation.NotifyOfChanges(
changes_or_error, base::PassKey<FileSystemAccessWatcherManager>());
observation_group.NotifyOfChanges(changes_or_error);
}
}
@ -300,7 +237,7 @@ void FileSystemAccessWatcherManager::OnUsageChange(
// The bucket file system's usage should not change.
CHECK(scope.GetWatchType() != WatchType::kAllBucketFileSystems);
for (auto& observation : observations_) {
for (auto& observation_group : observation_groups_) {
// TODO(crbug.com/376134535): We identify a change source by its scope.
// Observations that have the same scope belong to that change source. The
// bucket file system being the exception.
@ -308,11 +245,11 @@ void FileSystemAccessWatcherManager::OnUsageChange(
// Eventually we will want Observations to directly watch their
// ChangeSource. However we will have to do some refactoring because of the
// bucket file system exception.
if (observation.scope() != scope) {
if (observation_group.scope() != scope) {
continue;
}
observation.NotifyOfUsageChange(old_usage, new_usage);
observation_group.NotifyOfUsageChange(old_usage, new_usage);
}
}
@ -333,17 +270,19 @@ void FileSystemAccessWatcherManager::RegisterSource(
all_sources_.emplace_back(*source);
}
void FileSystemAccessWatcherManager::AddObserver(Observation* observation) {
void FileSystemAccessWatcherManager::AddObserver(
FileSystemAccessObservationGroup* observation_group) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
observations_.AddObserver(observation);
observation_groups_.AddObserver(observation_group);
}
void FileSystemAccessWatcherManager::RemoveObserver(Observation* observation) {
void FileSystemAccessWatcherManager::RemoveObserver(
FileSystemAccessObservationGroup* observation_group) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
const auto newly_unobserved_scope = observation->scope();
observations_.RemoveObserver(observation);
const auto newly_unobserved_scope = observation_group->scope();
observation_groups_.RemoveObserver(observation_group);
// Remove the respective source if we own it and it was the only observer
// for this scope.
@ -353,12 +292,19 @@ void FileSystemAccessWatcherManager::RemoveObserver(Observation* observation) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
return source->scope().Contains(newly_unobserved_scope) &&
base::ranges::none_of(
observations_, [&source](const auto& observation) {
observation_groups_, [&source](const auto& observation) {
return source->scope().Contains(observation.scope());
});
});
}
void FileSystemAccessWatcherManager::RemoveObservationGroup(
const FileSystemAccessWatchScope& scope) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
watch_scope_obs_groups_map_.erase(scope);
}
bool FileSystemAccessWatcherManager::HasSourceContainingScopeForTesting(
const FileSystemAccessWatchScope& scope) const {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
@ -441,6 +387,26 @@ void FileSystemAccessWatcherManager::DidInitializeSource(
std::move(on_source_initialized).Run(std::move(result));
}
FileSystemAccessObservationGroup&
FileSystemAccessWatcherManager::GetOrCreateObservationGroup(
FileSystemAccessWatchScope scope) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
auto observation_group_iter = watch_scope_obs_groups_map_.find(scope);
if (observation_group_iter != watch_scope_obs_groups_map_.end()) {
return observation_group_iter->second;
}
auto [created_observation_group_iter, inserted] =
watch_scope_obs_groups_map_.emplace(
std::piecewise_construct, std::forward_as_tuple(scope),
std::forward_as_tuple(
*this, scope, base::PassKey<FileSystemAccessWatcherManager>()));
CHECK(inserted);
return created_observation_group_iter->second;
}
void FileSystemAccessWatcherManager::PrepareObservationForScope(
FileSystemAccessWatchScope scope,
GetObservationCallback get_observation_callback,
@ -455,9 +421,7 @@ void FileSystemAccessWatcherManager::PrepareObservationForScope(
}
std::move(get_observation_callback)
.Run(std::make_unique<Observation>(
this, std::move(scope),
base::PassKey<FileSystemAccessWatcherManager>()));
.Run(GetOrCreateObservationGroup(std::move(scope)).CreateObserver());
}
std::unique_ptr<FileSystemAccessChangeSource>

@ -18,6 +18,7 @@
#include "base/types/pass_key.h"
#include "content/browser/file_system_access/file_system_access_bucket_path_watcher.h"
#include "content/browser/file_system_access/file_system_access_change_source.h"
#include "content/browser/file_system_access/file_system_access_observation_group.h"
#include "content/browser/file_system_access/file_system_access_watch_scope.h"
#include "content/common/content_export.h"
#include "content/public/browser/file_system_access_entry_factory.h"
@ -33,83 +34,27 @@ namespace content {
class FileSystemAccessManagerImpl;
class FileSystemAccessObserverHost;
// Manages all watches to file system changes for a StoragePartition.
//
// Raw changes from the underlying file system are plumbed through this class,
// to be filtered, batched, and transformed before being relayed to the
// appropriate `Observer`s.
// Manages browser side resources for the `FileSystemObserver` API.
//
// Instances of this class must be accessed exclusively on the UI thread. Owned
// by the FileSystemAccessManagerImpl.
// by the `FileSystemAccessManagerImpl`.
//
// TODO(crbug.com/376134535): Currently, raw changes from the underlying file
// system are plumbed through this class, to be filtered, batched, and
// transformed before being relayed to the appropriate
// `FileSystemAccessObservationGroup`s. Once `FileSystemAccessObservationGroup`s
// observe their change source directly, all this logic should be moved to the
// `FileSystemAccessObservationGroup`.
class CONTENT_EXPORT FileSystemAccessWatcherManager
: public FileSystemAccessChangeSource::RawChangeObserver {
public:
// Notifies of changes to a file system which occur in the given `scope`.
// These events may be consumed by other components.
class CONTENT_EXPORT Observation : public base::CheckedObserver {
public:
// Describes a change to some location in a file system.
struct CONTENT_EXPORT Change {
Change(storage::FileSystemURL url,
FileSystemAccessChangeSource::ChangeInfo change_info);
~Change();
// Copyable and movable.
Change(const Change&);
Change(Change&&) noexcept;
Change& operator=(const Change&);
Change& operator=(Change&&) noexcept;
storage::FileSystemURL url;
FileSystemAccessChangeSource::ChangeInfo change_info;
bool operator==(const Change& other) const {
return url == other.url && change_info == other.change_info;
}
};
using OnChangesCallback = base::RepeatingCallback<void(
const std::optional<std::list<Change>>& changes_or_error)>;
using OnUsageChangesCallback = FilePathWatcher::UsageChangeCallback;
Observation(FileSystemAccessWatcherManager* watcher_manager,
FileSystemAccessWatchScope scope,
base::PassKey<FileSystemAccessWatcherManager> pass_key);
~Observation() override;
// Set the callback to which changes will be reported.
// It is illegal to call this method more than once.
void SetCallback(OnChangesCallback on_change_callback);
// Set the callback to which usage changes will be reported.
// It is illegal to call this method more than once.
void SetUsageCallback(OnUsageChangesCallback on_usage_change_callback);
const FileSystemAccessWatchScope& scope() const { return scope_; }
void NotifyOfChanges(
const std::optional<std::list<Change>>& changes_or_error,
base::PassKey<FileSystemAccessWatcherManager> pass_key);
void NotifyOfUsageChange(size_t old_usage, size_t new_usage);
private:
SEQUENCE_CHECKER(sequence_checker_);
OnChangesCallback on_change_callback_ GUARDED_BY_CONTEXT(sequence_checker_);
OnUsageChangesCallback on_usage_change_callback_
GUARDED_BY_CONTEXT(sequence_checker_);
const FileSystemAccessWatchScope scope_;
base::ScopedObservation<FileSystemAccessWatcherManager, Observation> obs_
GUARDED_BY_CONTEXT(sequence_checker_){this};
};
using Change = FileSystemAccessObservationGroup::Change;
using BindingContext = FileSystemAccessEntryFactory::BindingContext;
using GetObservationCallback = base::OnceCallback<void(
base::expected<std::unique_ptr<Observation>,
blink::mojom::FileSystemAccessErrorPtr>)>;
base::expected<
std::unique_ptr<FileSystemAccessObservationGroup::Observer>,
blink::mojom::FileSystemAccessErrorPtr>)>;
using OnUsageChangeCallback = FilePathWatcher::UsageChangeCallback;
FileSystemAccessWatcherManager(
FileSystemAccessManagerImpl* manager,
@ -152,18 +97,14 @@ class CONTENT_EXPORT FileSystemAccessWatcherManager
// Subscriber this instance to raw changes from `source`.
void RegisterSource(FileSystemAccessChangeSource* source);
// For use with `ScopedObservation`. Do not otherwise call these methods
// directly.
void AddObserver(Observation* observation);
void RemoveObserver(Observation* observation);
bool HasObservationsForTesting() const {
bool HasObservationGroupsForTesting() const {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
return !observations_.empty();
return !observation_groups_.empty();
}
bool HasObservationForTesting(Observation* observation) const {
bool HasObservationGroupForTesting(
const FileSystemAccessObservationGroup& observation_group) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
return observations_.HasObserver(observation);
return observation_groups_.HasObserver(&observation_group);
}
bool HasSourceForTesting(FileSystemAccessChangeSource* source) const {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
@ -175,6 +116,23 @@ class CONTENT_EXPORT FileSystemAccessWatcherManager
FileSystemAccessManagerImpl* manager() { return manager_; }
private:
friend FileSystemAccessObservationGroup;
// Called by `FileSystemAccessObservationGroup` to receive raw change events.
//
// TODO(crbug.com/376134535): Remove this once observation groups directly
// observe their change source.
void AddObserver(FileSystemAccessObservationGroup* observation_group);
// Called by `FileSystemAccessObservationGroup` to stop receiving raw change
// events.
//
// TODO(crbug.com/376134535): Remove this once observation groups directly
// observe their change source.
void RemoveObserver(FileSystemAccessObservationGroup* observation_group);
// Called by `FileSystemAccessObservationGroup` to destroy itself.
void RemoveObservationGroup(const FileSystemAccessWatchScope& scope);
// Attempts to create a change source for `scope` if it does not exist.
void EnsureSourceIsInitializedForScope(
FileSystemAccessWatchScope scope,
@ -186,6 +144,9 @@ class CONTENT_EXPORT FileSystemAccessWatcherManager
on_source_initialized,
blink::mojom::FileSystemAccessErrorPtr result);
FileSystemAccessObservationGroup& GetOrCreateObservationGroup(
FileSystemAccessWatchScope scope);
void PrepareObservationForScope(
FileSystemAccessWatchScope scope,
GetObservationCallback callback,
@ -213,9 +174,15 @@ class CONTENT_EXPORT FileSystemAccessWatcherManager
// TODO(crbug.com/321980367): Make more efficient mappings to observers
// and sources. For now, most actions requires iterating through lists.
std::map<FileSystemAccessWatchScope, FileSystemAccessObservationGroup>
watch_scope_obs_groups_map_ GUARDED_BY_CONTEXT(sequence_checker_);
// Observations to which this instance will notify of changes within their
// respective scope.
base::ObserverList<Observation> observations_
//
// TODO(crbug.com/376134535): Remove this once observation groups directly
// observe their change source.
base::ObserverList<FileSystemAccessObservationGroup> observation_groups_
GUARDED_BY_CONTEXT(sequence_checker_);
// Sources created by this instance in response to a request to observe a

@ -21,6 +21,7 @@
#include "content/browser/blob_storage/chrome_blob_storage_context.h"
#include "content/browser/file_system_access/file_system_access_change_source.h"
#include "content/browser/file_system_access/file_system_access_manager_impl.h"
#include "content/browser/file_system_access/file_system_access_observation_group.h"
#include "content/browser/file_system_access/file_system_access_watch_scope.h"
#include "content/public/browser/web_contents.h"
#include "content/public/test/browser_task_environment.h"
@ -44,8 +45,8 @@
namespace content {
using Change = FileSystemAccessWatcherManager::Observation::Change;
using Observation = FileSystemAccessWatcherManager::Observation;
using Change = FileSystemAccessObservationGroup::Change;
using Observation = FileSystemAccessObservationGroup::Observer;
using ChangeInfo = FileSystemAccessChangeSource::ChangeInfo;
using ChangeType = FileSystemAccessChangeSource::ChangeType;
using FilePathType = FileSystemAccessChangeSource::FilePathType;
@ -159,11 +160,12 @@ class ChangeAccumulator {
GUARDED_BY_CONTEXT(sequence_checker_){this};
};
// Accumulates usage changes it receives from the given `observation`.
// Accumulates usage changes it receives from the given `observation_group`.
class UsageChangeAccumulator {
public:
explicit UsageChangeAccumulator(std::unique_ptr<Observation>& observation) {
observation->SetUsageCallback(base::BindRepeating(
explicit UsageChangeAccumulator(
FileSystemAccessObservationGroup& observation_group) {
observation_group.SetOnUsageCallbackForTesting(base::BindRepeating(
&UsageChangeAccumulator::OnUsageChanges, weak_factory_.GetWeakPtr()));
}
UsageChangeAccumulator(const ChangeAccumulator&) = delete;
@ -414,13 +416,18 @@ class FileSystemAccessWatcherManagerTest : public testing::Test {
base::expected<std::unique_ptr<Observation>,
blink::mojom::FileSystemAccessErrorPtr>>&
get_observation_future) {
if (!get_observation_future.Get().has_value()) {
auto& observation_or_error = get_observation_future.Get();
if (!observation_or_error.has_value()) {
return;
}
Observation* observation = get_observation_future.Get()->get();
const std::unique_ptr<Observation>& observation =
observation_or_error.value();
CHECK(watcher_manager().HasObservationForTesting(observation));
const FileSystemAccessObservationGroup& observation_group =
observation->GetObservationGroupForTesting();
CHECK(watcher_manager().HasObservationGroupForTesting(observation_group));
}
protected:
@ -458,7 +465,7 @@ TEST_F(FileSystemAccessWatcherManagerTest, BasicRegistration) {
base::FilePath dir_path = dir_.GetPath().AppendASCII("dir");
auto dir_url = manager_->CreateFileSystemURLFromPath(PathInfo(dir_path));
EXPECT_FALSE(watcher_manager().HasObservationsForTesting());
EXPECT_FALSE(watcher_manager().HasObservationGroupsForTesting());
std::optional<FileSystemAccessWatchScope> observation_scope = std::nullopt;
{
@ -471,21 +478,28 @@ TEST_F(FileSystemAccessWatcherManagerTest, BasicRegistration) {
ASSERT_TRUE(get_observation_future.Get().has_value());
// An observation should have been created.
auto observation = get_observation_future.Take();
std::unique_ptr<content::FileSystemAccessObservationGroup::Observer>
observation = get_observation_future.Take().value();
FileSystemAccessObservationGroup& observation_group =
observation->GetObservationGroupForTesting();
// An observation group should exist for the scope of the observation.
EXPECT_TRUE(
watcher_manager().HasObservationForTesting(observation.value().get()));
watcher_manager().HasObservationGroupForTesting(observation_group));
// A source should have been created to cover the scope of the observation
observation_scope = observation.value()->scope();
observation_scope = observation->scope();
EXPECT_TRUE(watcher_manager().HasSourceContainingScopeForTesting(
*observation_scope));
}
// Destroying an observation unregisters it with the manager.
EXPECT_FALSE(watcher_manager().HasObservationsForTesting());
// Destroying an observation unregisters it with the observation group. When
// the observation group has no more observations, the observation group is
// unregistered with the manager and destroyed.
EXPECT_FALSE(watcher_manager().HasObservationGroupsForTesting());
// The watcher manager removes a source when there are no observations for
// that source.
// The watcher manager removes a source when there are no observation groups
// for that source.
EXPECT_FALSE(
watcher_manager().HasSourceContainingScopeForTesting(*observation_scope));
}
@ -552,8 +566,8 @@ TEST_F(FileSystemAccessWatcherManagerTest, SourceFailsInitialization) {
EXPECT_EQ(observation_or_error.error()->status,
blink::mojom::FileSystemAccessStatus::kOperationFailed);
// No observations should have been created.
ASSERT_FALSE(watcher_manager().HasObservationsForTesting());
// No observation groups should have been created.
ASSERT_FALSE(watcher_manager().HasObservationGroupsForTesting());
}
TEST_F(FileSystemAccessWatcherManagerTest, IgnoreSwapFileChanges) {
@ -634,8 +648,8 @@ TEST_F(FileSystemAccessWatcherManagerTest, RemoveObservation) {
{
ChangeAccumulator accumulator(std::move(observation_or_error));
EXPECT_TRUE(
watcher_manager().HasObservationForTesting(accumulator.observation()));
EXPECT_TRUE(watcher_manager().HasObservationGroupForTesting(
accumulator.observation()->GetObservationGroupForTesting()));
source.Signal();
@ -648,7 +662,7 @@ TEST_F(FileSystemAccessWatcherManagerTest, RemoveObservation) {
// Signaling changes after the observation was removed should not crash.
source.Signal();
EXPECT_FALSE(watcher_manager().HasObservationsForTesting());
EXPECT_FALSE(watcher_manager().HasObservationGroupsForTesting());
}
TEST_F(FileSystemAccessWatcherManagerTest, ObserveBucketFS) {
@ -1191,6 +1205,47 @@ TEST_F(FileSystemAccessWatcherManagerTest, OutOfScope) {
#endif // BUILDFLAG(IS_ANDROID) || BUILDFLAG(IS_IOS) || BUILDFLAG(IS_FUCHSIA)
}
TEST_F(FileSystemAccessWatcherManagerTest,
ObservationGroupsAreSharedByAllObservationsWithSameScope) {
base::FilePath file_path = dir_.GetPath().AppendASCII("foo");
auto file_url = manager_->CreateFileSystemURLFromPath(PathInfo(file_path));
FakeChangeSource file_source(
FileSystemAccessWatchScope::GetScopeForFileWatch(file_url),
file_system_context_);
watcher_manager().RegisterSource(&file_source);
base::FilePath dir_path = dir_.GetPath().AppendASCII("bar");
auto dir_url = manager_->CreateFileSystemURLFromPath(PathInfo(dir_path));
FakeChangeSource dir_source(
FileSystemAccessWatchScope::GetScopeForDirectoryWatch(
dir_url, /*is_recursive=*/false),
file_system_context_);
watcher_manager().RegisterSource(&dir_source);
std::unique_ptr<Observation> file_observation1 =
std::move(ObserveFile(file_url)).value();
FileSystemAccessObservationGroup& file_observation1_group =
file_observation1->GetObservationGroupForTesting();
std::unique_ptr<Observation> file_observation2 =
std::move(ObserveFile(file_url)).value();
FileSystemAccessObservationGroup& file_observation2_group =
file_observation2->GetObservationGroupForTesting();
std::unique_ptr<Observation> dir_observation =
std::move(ObserveDirectory(dir_url, /*is_recursive=*/false)).value();
FileSystemAccessObservationGroup& dir_observation_group =
dir_observation->GetObservationGroupForTesting();
// Both file observations have the same scope, so they should be a part of the
// observation group.
EXPECT_EQ(&file_observation1_group, &file_observation2_group);
// The file and dir observations have different scopes, so they should be in
// different observation groups.
EXPECT_NE(&dir_observation_group, &file_observation1_group);
}
TEST_F(FileSystemAccessWatcherManagerTest, UsageChange) {
base::FilePath file_path = dir_.GetPath().AppendASCII("foo");
auto file_url = manager_->CreateFileSystemURLFromPath(PathInfo(file_path));
@ -1205,10 +1260,12 @@ TEST_F(FileSystemAccessWatcherManagerTest, UsageChange) {
auto observation_or_error = ObserveFile(file_url);
ASSERT_TRUE(observation_or_error.has_value());
std::unique_ptr<Observation>& observation = observation_or_error.value();
FileSystemAccessObservationGroup& observation_group =
observation_or_error.value()->GetObservationGroupForTesting();
ASSERT_TRUE(
watcher_manager().HasObservationGroupForTesting(observation_group));
UsageChangeAccumulator accumulator(observation);
EXPECT_TRUE(watcher_manager().HasObservationForTesting(observation.get()));
UsageChangeAccumulator accumulator(observation_group);
source.SignalUsageChange(50, 100);
source.SignalUsageChange(100, 80);

@ -266,6 +266,8 @@ class COMPONENT_EXPORT(STORAGE_BROWSER) FileSystemURL {
bool operator==(const FileSystemURL& that) const;
bool operator!=(const FileSystemURL& that) const { return !(*this == that); }
std::weak_ordering operator<=>(const FileSystemURL& that) const;
struct COMPONENT_EXPORT(STORAGE_BROWSER) Comparator {