Reland "FSA: Provide a better error if observe() fails"
This reverts commit 8c34999fbd695045b08f9e8dcd0bf0a285461293.
Reason for revert: Fixed failures on iOS
Original change's description:
> Revert "FSA: Provide a better error if observe() fails"
>
> This reverts commit 7a76e40f19498035d5ba24f2ceb5ec7ff7f3e34e.
>
> Reason for revert: causing failures on ios-blink-dbg-fyi
>
> Original change's description:
> > FSA: Provide a better error if observe() fails
> >
> > Before this CL, observe() would always throw a NotSupportedError, which
> > is generally reserved for unlaunched/unsupported features. Now, we'll
> > throw an OperationFailedError if observing the given file is supported,
> > but initializing the corresponding change source fails
> >
> > Bug: 1019297
> > Change-Id: I7e5bb3be14bbad4caf755ecc47ef6797df1bfacc
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4883812
> > Auto-Submit: Austin Sullivan <asully@chromium.org>
> > Reviewed-by: Christine Smith <christinesm@chromium.org>
> > Commit-Queue: Christine Smith <christinesm@chromium.org>
> > Cr-Commit-Position: refs/heads/main@{#1200214}
>
> Bug: 1019297
> Change-Id: I208e894ea76b68b5a5ae5e747636e25bf92a1cb1
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4885557
> Reviewed-by: Austin Sullivan <asully@chromium.org>
> Commit-Queue: Austin Sullivan <asully@chromium.org>
> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
> Cr-Commit-Position: refs/heads/main@{#1200388}
Bug: 1019297
Change-Id: I35176a5069a9b8c852f08f250e364083a0f58056
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4885458
Reviewed-by: Christine Smith <christinesm@chromium.org>
Auto-Submit: Austin Sullivan <asully@chromium.org>
Commit-Queue: Christine Smith <christinesm@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1201115}
diff --git a/content/browser/file_system_access/file_system_access_change_source.cc b/content/browser/file_system_access/file_system_access_change_source.cc
index e23d8a3..bc7cbfc 100644
--- a/content/browser/file_system_access/file_system_access_change_source.cc
+++ b/content/browser/file_system_access/file_system_access_change_source.cc
@@ -28,12 +28,13 @@
}
void FileSystemAccessChangeSource::EnsureInitialized(
- base::OnceCallback<void(bool)> on_source_initialized) {
+ base::OnceCallback<void(blink::mojom::FileSystemAccessErrorPtr)>
+ on_source_initialized) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
if (initialization_result_.has_value()) {
CHECK(initialization_callbacks_.empty());
- std::move(on_source_initialized).Run(*initialization_result_);
+ std::move(on_source_initialized).Run(initialization_result_->Clone());
return;
}
@@ -46,18 +47,19 @@
weak_factory_.GetWeakPtr()));
}
-void FileSystemAccessChangeSource::DidInitialize(bool result) {
+void FileSystemAccessChangeSource::DidInitialize(
+ blink::mojom::FileSystemAccessErrorPtr result) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
CHECK(!initialization_result_.has_value());
CHECK(!initialization_callbacks_.empty());
- initialization_result_ = result;
+ initialization_result_ = std::move(result);
// Move the callbacks to the stack since they may cause |this| to be deleted.
auto initialization_callbacks = std::move(initialization_callbacks_);
initialization_callbacks_.clear();
for (auto& callback : initialization_callbacks) {
- std::move(callback).Run(result);
+ std::move(callback).Run(initialization_result_->Clone());
}
}
diff --git a/content/browser/file_system_access/file_system_access_change_source.h b/content/browser/file_system_access/file_system_access_change_source.h
index f2ee0d3..01eb7d0 100644
--- a/content/browser/file_system_access/file_system_access_change_source.h
+++ b/content/browser/file_system_access/file_system_access_change_source.h
@@ -17,6 +17,7 @@
#include "content/browser/file_system_access/file_system_access_watch_scope.h"
#include "content/common/content_export.h"
#include "third_party/abseil-cpp/absl/types/optional.h"
+#include "third_party/blink/public/mojom/file_system_access/file_system_access_error.mojom.h"
namespace content {
@@ -46,11 +47,13 @@
// Ensures that this change source is ready to watch for changes within its
// `scope_`. This may fail if the scope cannot be watched.
- // `on_source_initialized` is run with a bool indicating whether setting up
- // this source succeeds.
+ // `on_source_initialized` is run with a error status indicating whether
+ // setting up this source succeeds.
// TODO(https://crbug.com/1019297): Assert that this is called before
// notifying of changes.
- void EnsureInitialized(base::OnceCallback<void(bool)> on_source_initialized);
+ void EnsureInitialized(
+ base::OnceCallback<void(blink::mojom::FileSystemAccessErrorPtr)>
+ on_source_initialized);
base::WeakPtr<FileSystemAccessChangeSource> AsWeakPtr();
@@ -58,7 +61,8 @@
protected:
virtual void Initialize(
- base::OnceCallback<void(bool)> on_source_initialized) = 0;
+ base::OnceCallback<void(blink::mojom::FileSystemAccessErrorPtr)>
+ on_source_initialized) = 0;
// Called by subclasses to record changes to watched paths.
void NotifyOfChange(const base::FilePath& relative_path, bool error);
@@ -66,13 +70,13 @@
SEQUENCE_CHECKER(sequence_checker_);
private:
- void DidInitialize(bool result);
+ void DidInitialize(blink::mojom::FileSystemAccessErrorPtr result);
const FileSystemAccessWatchScope scope_;
- absl::optional<bool> initialization_result_;
- std::list<base::OnceCallback<void(bool)>> initialization_callbacks_
- GUARDED_BY_CONTEXT(sequence_checker_);
+ absl::optional<blink::mojom::FileSystemAccessErrorPtr> initialization_result_;
+ std::list<base::OnceCallback<void(blink::mojom::FileSystemAccessErrorPtr)>>
+ initialization_callbacks_ GUARDED_BY_CONTEXT(sequence_checker_);
base::ObserverList<RawChangeObserver> observers_
GUARDED_BY_CONTEXT(sequence_checker_);
diff --git a/content/browser/file_system_access/file_system_access_local_path_watcher.cc b/content/browser/file_system_access/file_system_access_local_path_watcher.cc
index 1879441..602a1b2 100644
--- a/content/browser/file_system_access/file_system_access_local_path_watcher.cc
+++ b/content/browser/file_system_access/file_system_access_local_path_watcher.cc
@@ -8,8 +8,10 @@
#include "base/files/file_path_watcher.h"
#include "base/task/bind_post_task.h"
#include "base/task/thread_pool.h"
+#include "content/browser/file_system_access/file_system_access_error.h"
#include "content/browser/file_system_access/file_system_access_watcher_manager.h"
#include "storage/browser/file_system/file_system_url.h"
+#include "third_party/blink/public/mojom/file_system_access/file_system_access_error.mojom-shared.h"
namespace content {
@@ -40,12 +42,15 @@
FileSystemAccessLocalPathWatcher::~FileSystemAccessLocalPathWatcher() = default;
void FileSystemAccessLocalPathWatcher::Initialize(
- base::OnceCallback<void(bool)> on_source_initialized) {
+ base::OnceCallback<void(blink::mojom::FileSystemAccessErrorPtr)>
+ on_source_initialized) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
if (scope().IsRecursive() &&
!base::FilePathWatcher::RecursiveWatchAvailable()) {
- std::move(on_source_initialized).Run(false);
+ std::move(on_source_initialized)
+ .Run(file_system_access_error::FromStatus(
+ blink::mojom::FileSystemAccessStatus::kNotSupportedError));
return;
}
@@ -71,7 +76,17 @@
.WithArgs(
scope().root_url().path(), std::move(watch_options),
base::BindPostTaskToCurrentDefault(std::move(on_change_callback)))
- .Then(std::move(on_source_initialized));
+ .Then(base::BindOnce(
+ [](base::OnceCallback<void(blink::mojom::FileSystemAccessErrorPtr)>
+ callback,
+ bool result) {
+ std::move(callback).Run(
+ result ? file_system_access_error::Ok()
+ : file_system_access_error::FromStatus(
+ blink::mojom::FileSystemAccessStatus::
+ kOperationFailed));
+ },
+ std::move(on_source_initialized)));
}
void FileSystemAccessLocalPathWatcher::OnFilePathChanged(
diff --git a/content/browser/file_system_access/file_system_access_local_path_watcher.h b/content/browser/file_system_access/file_system_access_local_path_watcher.h
index b429292..b8a18f7 100644
--- a/content/browser/file_system_access/file_system_access_local_path_watcher.h
+++ b/content/browser/file_system_access/file_system_access_local_path_watcher.h
@@ -31,7 +31,8 @@
// FileSystemAccessChangeSource:
void Initialize(
- base::OnceCallback<void(bool)> on_source_initialized) override;
+ base::OnceCallback<void(blink::mojom::FileSystemAccessErrorPtr)>
+ on_source_initialized) override;
private:
void OnFilePathChanged(const base::FilePath& changed_path, bool error);
diff --git a/content/browser/file_system_access/file_system_access_observer_host.cc b/content/browser/file_system_access/file_system_access_observer_host.cc
index ccd8696..3bcf079 100644
--- a/content/browser/file_system_access/file_system_access_observer_host.cc
+++ b/content/browser/file_system_access/file_system_access_observer_host.cc
@@ -150,14 +150,14 @@
absl::variant<std::unique_ptr<FileSystemAccessDirectoryHandleImpl>,
std::unique_ptr<FileSystemAccessFileHandleImpl>> handle,
ObserveCallback callback,
- std::unique_ptr<FileSystemAccessWatcherManager::Observation> observation) {
+ base::expected<std::unique_ptr<FileSystemAccessWatcherManager::Observation>,
+ blink::mojom::FileSystemAccessErrorPtr>
+ observation_or_error) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
- if (!observation) {
- std::move(callback).Run(
- file_system_access_error::FromStatus(
- blink::mojom::FileSystemAccessStatus::kNotSupportedError),
- mojo::NullReceiver());
+ if (!observation_or_error.has_value()) {
+ std::move(callback).Run(std::move(observation_or_error.error()),
+ mojo::NullReceiver());
return;
}
@@ -167,8 +167,8 @@
auto observer_observation =
std::make_unique<FileSystemAccessObserverObservation>(
- this, std::move(observation), std::move(observer_remote),
- std::move(handle));
+ this, std::move(observation_or_error.value()),
+ std::move(observer_remote), std::move(handle));
observations_.insert(std::move(observer_observation));
std::move(callback).Run(file_system_access_error::Ok(),
diff --git a/content/browser/file_system_access/file_system_access_observer_host.h b/content/browser/file_system_access/file_system_access_observer_host.h
index 4851314..c0bf6ba 100644
--- a/content/browser/file_system_access/file_system_access_observer_host.h
+++ b/content/browser/file_system_access/file_system_access_observer_host.h
@@ -72,7 +72,9 @@
absl::variant<std::unique_ptr<FileSystemAccessDirectoryHandleImpl>,
std::unique_ptr<FileSystemAccessFileHandleImpl>> handle,
ObserveCallback callback,
- std::unique_ptr<FileSystemAccessWatcherManager::Observation> observation);
+ base::expected<
+ std::unique_ptr<FileSystemAccessWatcherManager::Observation>,
+ blink::mojom::FileSystemAccessErrorPtr> observation_or_error);
void OnHostReceiverDisconnect();
diff --git a/content/browser/file_system_access/file_system_access_watcher_manager.cc b/content/browser/file_system_access/file_system_access_watcher_manager.cc
index 74a3443d..51af152 100644
--- a/content/browser/file_system_access/file_system_access_watcher_manager.cc
+++ b/content/browser/file_system_access/file_system_access_watcher_manager.cc
@@ -17,6 +17,7 @@
#include "build/buildflag.h"
#include "components/services/storage/public/cpp/buckets/bucket_locator.h"
#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_observer_host.h"
#include "content/browser/file_system_access/file_system_access_observer_observation.h"
@@ -24,6 +25,7 @@
#include "mojo/public/cpp/bindings/pending_receiver.h"
#include "storage/browser/file_system/file_system_url.h"
#include "storage/common/file_system/file_system_types.h"
+#include "third_party/blink/public/mojom/file_system_access/file_system_access_error.mojom-shared.h"
#if !BUILDFLAG(IS_ANDROID) && !BUILDFLAG(IS_FUCHSIA)
#include "content/browser/file_system_access/file_system_access_local_path_watcher.h"
@@ -202,7 +204,8 @@
void FileSystemAccessWatcherManager::EnsureSourceIsInitializedForScope(
FileSystemAccessWatchScope scope,
- base::OnceCallback<void(bool)> on_source_initialized) {
+ base::OnceCallback<void(blink::mojom::FileSystemAccessErrorPtr)>
+ on_source_initialized) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
// TODO(https://crbug.com/1019297): Handle overlapping scopes and initializing
@@ -219,7 +222,9 @@
auto owned_change_source = CreateOwnedSourceForScope(scope);
if (!owned_change_source) {
// TODO(https://crbug.com/1019297): Watching `scope` is not supported.
- std::move(on_source_initialized).Run(false);
+ std::move(on_source_initialized)
+ .Run(file_system_access_error::FromStatus(
+ blink::mojom::FileSystemAccessStatus::kNotSupportedError));
return;
}
raw_change_source = owned_change_source.get();
@@ -235,16 +240,20 @@
void FileSystemAccessWatcherManager::DidInitializeSource(
base::WeakPtr<FileSystemAccessChangeSource> source,
- base::OnceCallback<void(bool)> on_source_initialized,
- bool success) {
+ base::OnceCallback<void(blink::mojom::FileSystemAccessErrorPtr)>
+ on_source_initialized,
+ blink::mojom::FileSystemAccessErrorPtr result) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
if (!source) {
- std::move(on_source_initialized).Run(false);
+ // `source` was destroyed as we tried to initialize it. Abort.
+ std::move(on_source_initialized)
+ .Run(file_system_access_error::FromStatus(
+ blink::mojom::FileSystemAccessStatus::kOperationFailed));
return;
}
- if (!success) {
+ if (result->status != blink::mojom::FileSystemAccessStatus::kOk) {
// If we owned this source, remove it. A source which is not initialized
// will not notify of changes, so there's no use keeping it around.
//
@@ -258,17 +267,19 @@
});
}
- std::move(on_source_initialized).Run(success);
+ std::move(on_source_initialized).Run(std::move(result));
}
void FileSystemAccessWatcherManager::PrepareObservationForScope(
FileSystemAccessWatchScope scope,
GetObservationCallback get_observation_callback,
- bool success) {
+ blink::mojom::FileSystemAccessErrorPtr source_initialization_result) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
- if (!success) {
- std::move(get_observation_callback).Run(nullptr);
+ if (source_initialization_result->status !=
+ blink::mojom::FileSystemAccessStatus::kOk) {
+ std::move(get_observation_callback)
+ .Run(base::unexpected(std::move(source_initialization_result)));
return;
}
diff --git a/content/browser/file_system_access/file_system_access_watcher_manager.h b/content/browser/file_system_access/file_system_access_watcher_manager.h
index 087cb39..9732b903 100644
--- a/content/browser/file_system_access/file_system_access_watcher_manager.h
+++ b/content/browser/file_system_access/file_system_access_watcher_manager.h
@@ -14,6 +14,7 @@
#include "base/scoped_multi_source_observation.h"
#include "base/scoped_observation.h"
#include "base/sequence_checker.h"
+#include "base/types/expected.h"
#include "base/types/pass_key.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"
@@ -22,6 +23,7 @@
#include "mojo/public/cpp/bindings/pending_receiver.h"
#include "storage/browser/file_system/file_system_context.h"
#include "storage/browser/file_system/file_system_url.h"
+#include "third_party/blink/public/mojom/file_system_access/file_system_access_error.mojom.h"
#include "third_party/blink/public/mojom/file_system_access/file_system_access_observer_host.mojom.h"
namespace content {
@@ -85,8 +87,9 @@
};
using BindingContext = FileSystemAccessEntryFactory::BindingContext;
- using GetObservationCallback =
- base::OnceCallback<void(std::unique_ptr<Observation>)>;
+ using GetObservationCallback = base::OnceCallback<void(
+ base::expected<std::unique_ptr<Observation>,
+ blink::mojom::FileSystemAccessErrorPtr>)>;
FileSystemAccessWatcherManager(
FileSystemAccessManagerImpl* manager,
@@ -108,8 +111,8 @@
// `FileSystemAccessChangeSource` if one does not already cover the scope of
// the requested observation.
//
- // `get_observation_callback` returns an `Observation`, or nullptr if the
- // given file or directory cannot be watched as requested.
+ // `get_observation_callback` returns an `Observation`, or an appropriate
+ // error if the given file or directory cannot be watched as requested.
void GetFileObservation(const storage::FileSystemURL& file_url,
GetObservationCallback get_observation_callback);
void GetDirectoryObservation(const storage::FileSystemURL& directory_url,
@@ -153,15 +156,21 @@
// Attempts to create a change source for `scope` if it does not exist.
void EnsureSourceIsInitializedForScope(
FileSystemAccessWatchScope scope,
- base::OnceCallback<void(bool)> on_source_initialized);
- void DidInitializeSource(base::WeakPtr<FileSystemAccessChangeSource> source,
- base::OnceCallback<void(bool)> on_source_initialized,
- bool success);
+ base::OnceCallback<void(blink::mojom::FileSystemAccessErrorPtr)>
+ on_source_initialized);
+ void DidInitializeSource(
+ base::WeakPtr<FileSystemAccessChangeSource> source,
+ base::OnceCallback<void(blink::mojom::FileSystemAccessErrorPtr)>
+ on_source_initialized,
+ blink::mojom::FileSystemAccessErrorPtr result);
- void PrepareObservationForScope(FileSystemAccessWatchScope scope,
- GetObservationCallback callback,
- bool success);
+ void PrepareObservationForScope(
+ FileSystemAccessWatchScope scope,
+ GetObservationCallback callback,
+ blink::mojom::FileSystemAccessErrorPtr source_initialization_result);
+ // Creates and returns a new (uninitialized) change source for the given
+ // scope, or nullptr if watching this scope is not supported.
std::unique_ptr<FileSystemAccessChangeSource> CreateOwnedSourceForScope(
FileSystemAccessWatchScope scope);
diff --git a/content/browser/file_system_access/file_system_access_watcher_manager_unittest.cc b/content/browser/file_system_access/file_system_access_watcher_manager_unittest.cc
index ca4fc850..ec6e442b 100644
--- a/content/browser/file_system_access/file_system_access_watcher_manager_unittest.cc
+++ b/content/browser/file_system_access/file_system_access_watcher_manager_unittest.cc
@@ -32,6 +32,7 @@
#include "storage/browser/test/test_file_system_context.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/blink/public/mojom/file_system_access/file_system_access_error.mojom-shared.h"
namespace content {
@@ -110,9 +111,11 @@
~FakeChangeSource() override = default;
// FileSystemAccessChangeSource:
- void Initialize(base::OnceCallback<void(bool)> on_initialized) override {
+ void Initialize(
+ base::OnceCallback<void(blink::mojom::FileSystemAccessErrorPtr)>
+ on_initialized) override {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
- std::move(on_initialized).Run(initialization_result_);
+ std::move(on_initialized).Run(initialization_result_->Clone());
}
void Signal(base::FilePath relative_path, bool error) {
@@ -120,13 +123,19 @@
NotifyOfChange(std::move(relative_path), error);
}
- void set_initialization_result(bool result) {
+ void set_initialization_result(
+ blink::mojom::FileSystemAccessErrorPtr result) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
- initialization_result_ = result;
+ initialization_result_ = std::move(result);
}
private:
- bool initialization_result_ GUARDED_BY_CONTEXT(sequence_checker_) = true;
+ blink::mojom::FileSystemAccessErrorPtr initialization_result_
+ GUARDED_BY_CONTEXT(sequence_checker_) =
+ blink::mojom::FileSystemAccessError::New(
+ blink::mojom::FileSystemAccessStatus::kOk,
+ base::File::FILE_OK,
+ "");
};
} // namespace
@@ -197,15 +206,18 @@
EXPECT_FALSE(watcher_manager().HasSourcesForTesting());
{
- base::test::TestFuture<std::unique_ptr<Observation>> get_observation_future;
+ base::test::TestFuture<base::expected<
+ std::unique_ptr<Observation>, blink::mojom::FileSystemAccessErrorPtr>>
+ get_observation_future;
watcher_manager().GetDirectoryObservation(
dir_url,
/*is_recursive=*/false, get_observation_future.GetCallback());
- ASSERT_TRUE(get_observation_future.Get());
+ ASSERT_TRUE(get_observation_future.Get().has_value());
// An observation should have been created.
auto observation = get_observation_future.Take();
- EXPECT_TRUE(watcher_manager().HasObservationForTesting(observation.get()));
+ EXPECT_TRUE(
+ watcher_manager().HasObservationForTesting(observation.value().get()));
// A source should have been created to cover the scope of the observation.
EXPECT_TRUE(watcher_manager().HasSourcesForTesting());
@@ -245,12 +257,14 @@
EXPECT_TRUE(watcher_manager().HasSourceForTesting(&source));
// Attempting to observe a scope covered by `source` will use `source`.
- base::test::TestFuture<std::unique_ptr<Observation>> get_observation_future;
+ base::test::TestFuture<base::expected<std::unique_ptr<Observation>,
+ blink::mojom::FileSystemAccessErrorPtr>>
+ get_observation_future;
watcher_manager().GetFileObservation(file_url,
get_observation_future.GetCallback());
- ASSERT_TRUE(get_observation_future.Get());
+ ASSERT_TRUE(get_observation_future.Get().has_value());
- ChangeAccumulator accumulator(get_observation_future.Take());
+ ChangeAccumulator accumulator(get_observation_future.Take().value());
EXPECT_TRUE(
watcher_manager().HasObservationForTesting(accumulator.observation()));
@@ -273,13 +287,19 @@
watcher_manager().RegisterSource(&source);
EXPECT_TRUE(watcher_manager().HasSourceForTesting(&source));
- source.set_initialization_result(false);
+ source.set_initialization_result(blink::mojom::FileSystemAccessError::New(
+ blink::mojom::FileSystemAccessStatus::kOperationFailed,
+ base::File::FILE_OK, ""));
// Attempting to observe a scope covered by `source` will use `source`.
- base::test::TestFuture<std::unique_ptr<Observation>> get_observation_future;
+ base::test::TestFuture<base::expected<std::unique_ptr<Observation>,
+ blink::mojom::FileSystemAccessErrorPtr>>
+ get_observation_future;
watcher_manager().GetFileObservation(file_url,
get_observation_future.GetCallback());
- EXPECT_FALSE(get_observation_future.Get());
+ ASSERT_FALSE(get_observation_future.Get().has_value());
+ EXPECT_EQ(get_observation_future.Get().error()->status,
+ blink::mojom::FileSystemAccessStatus::kOperationFailed);
// TODO(https://crbug.com/1019297): Determine what should happen on failure to
// initialize a source, then add better test coverage.
@@ -296,13 +316,15 @@
EXPECT_TRUE(watcher_manager().HasSourceForTesting(&source));
// Attempting to observe a scope covered by `source` will use `source`.
- base::test::TestFuture<std::unique_ptr<Observation>> get_observation_future;
+ base::test::TestFuture<base::expected<std::unique_ptr<Observation>,
+ blink::mojom::FileSystemAccessErrorPtr>>
+ get_observation_future;
watcher_manager().GetFileObservation(file_url,
get_observation_future.GetCallback());
- ASSERT_TRUE(get_observation_future.Get());
+ ASSERT_TRUE(get_observation_future.Get().has_value());
{
- ChangeAccumulator accumulator(get_observation_future.Take());
+ ChangeAccumulator accumulator(get_observation_future.Take().value());
EXPECT_TRUE(
watcher_manager().HasObservationForTesting(accumulator.observation()));
@@ -326,10 +348,14 @@
GURL("filesystem:http://chromium.org/temporary/i/has/a.bucket"));
// Attempting to observe the given file will fail.
- base::test::TestFuture<std::unique_ptr<Observation>> get_observation_future;
+ base::test::TestFuture<base::expected<std::unique_ptr<Observation>,
+ blink::mojom::FileSystemAccessErrorPtr>>
+ get_observation_future;
watcher_manager().GetFileObservation(temporary_url,
get_observation_future.GetCallback());
- EXPECT_FALSE(get_observation_future.Get());
+ ASSERT_FALSE(get_observation_future.Get().has_value());
+ EXPECT_EQ(get_observation_future.Get().error()->status,
+ blink::mojom::FileSystemAccessStatus::kNotSupportedError);
}
// TODO(https://crbug.com/1019297): Add tests covering more edge cases regarding
@@ -354,12 +380,14 @@
watcher_manager().RegisterSource(&source_for_dir);
EXPECT_TRUE(watcher_manager().HasSourceForTesting(&source_for_dir));
- base::test::TestFuture<std::unique_ptr<Observation>> get_observation_future;
+ base::test::TestFuture<base::expected<std::unique_ptr<Observation>,
+ blink::mojom::FileSystemAccessErrorPtr>>
+ get_observation_future;
watcher_manager().GetFileObservation(file_url,
get_observation_future.GetCallback());
- ASSERT_TRUE(get_observation_future.Get());
+ ASSERT_TRUE(get_observation_future.Get().has_value());
- ChangeAccumulator accumulator(get_observation_future.Take());
+ ChangeAccumulator accumulator(get_observation_future.Take().value());
EXPECT_TRUE(
watcher_manager().HasObservationForTesting(accumulator.observation()));
@@ -391,23 +419,26 @@
watcher_manager().RegisterSource(&source);
EXPECT_TRUE(watcher_manager().HasSourceForTesting(&source));
- base::test::TestFuture<std::unique_ptr<Observation>>
+ base::test::TestFuture<base::expected<std::unique_ptr<Observation>,
+ blink::mojom::FileSystemAccessErrorPtr>>
get_dir_observation_future;
watcher_manager().GetDirectoryObservation(
dir_url, /*is_recursive=*/true, get_dir_observation_future.GetCallback());
- EXPECT_TRUE(get_dir_observation_future.Get());
+ EXPECT_TRUE(get_dir_observation_future.Get().has_value());
- ChangeAccumulator dir_accumulator(get_dir_observation_future.Take());
+ ChangeAccumulator dir_accumulator(get_dir_observation_future.Take().value());
EXPECT_TRUE(watcher_manager().HasObservationForTesting(
dir_accumulator.observation()));
- base::test::TestFuture<std::unique_ptr<Observation>>
+ base::test::TestFuture<base::expected<std::unique_ptr<Observation>,
+ blink::mojom::FileSystemAccessErrorPtr>>
get_file_observation_future;
watcher_manager().GetFileObservation(
file_url, get_file_observation_future.GetCallback());
- EXPECT_TRUE(get_file_observation_future.Get());
+ EXPECT_TRUE(get_file_observation_future.Get().has_value());
- ChangeAccumulator file_accumulator(get_file_observation_future.Take());
+ ChangeAccumulator file_accumulator(
+ get_file_observation_future.Take().value());
EXPECT_TRUE(watcher_manager().HasObservationForTesting(
file_accumulator.observation()));
@@ -438,12 +469,14 @@
EXPECT_TRUE(watcher_manager().HasSourceForTesting(&source));
// Attempting to observe a scope covered by `source` will use `source`.
- base::test::TestFuture<std::unique_ptr<Observation>> get_observation_future;
+ base::test::TestFuture<base::expected<std::unique_ptr<Observation>,
+ blink::mojom::FileSystemAccessErrorPtr>>
+ get_observation_future;
watcher_manager().GetFileObservation(file_url,
get_observation_future.GetCallback());
- ASSERT_TRUE(get_observation_future.Get());
+ ASSERT_TRUE(get_observation_future.Get().has_value());
- ChangeAccumulator accumulator(get_observation_future.Take());
+ ChangeAccumulator accumulator(get_observation_future.Take().value());
EXPECT_TRUE(
watcher_manager().HasObservationForTesting(accumulator.observation()));
@@ -467,12 +500,14 @@
EXPECT_TRUE(watcher_manager().HasSourceForTesting(&source));
// Attempting to observe a scope covered by `source` will use `source`.
- base::test::TestFuture<std::unique_ptr<Observation>> get_observation_future;
+ base::test::TestFuture<base::expected<std::unique_ptr<Observation>,
+ blink::mojom::FileSystemAccessErrorPtr>>
+ get_observation_future;
watcher_manager().GetDirectoryObservation(
dir_url, /*is_recursive=*/true, get_observation_future.GetCallback());
- ASSERT_TRUE(get_observation_future.Get());
+ ASSERT_TRUE(get_observation_future.Get().has_value());
- ChangeAccumulator accumulator(get_observation_future.Take());
+ ChangeAccumulator accumulator(get_observation_future.Take().value());
EXPECT_TRUE(
watcher_manager().HasObservationForTesting(accumulator.observation()));
@@ -503,17 +538,21 @@
auto file_path = dir_path.AppendASCII("foo");
base::WriteFile(file_path, "watch me");
- base::test::TestFuture<std::unique_ptr<Observation>> get_observation_future;
+ base::test::TestFuture<base::expected<std::unique_ptr<Observation>,
+ blink::mojom::FileSystemAccessErrorPtr>>
+ get_observation_future;
watcher_manager().GetDirectoryObservation(
dir_url,
/*is_recursive=*/false, get_observation_future.GetCallback());
// Watching the local file system is not supported on Android or Fuchsia.
#if BUILDFLAG(IS_ANDROID) || BUILDFLAG(IS_FUCHSIA)
- EXPECT_FALSE(get_observation_future.Get());
+ ASSERT_FALSE(get_observation_future.Get().has_value());
+ EXPECT_EQ(get_observation_future.Get().error()->status,
+ blink::mojom::FileSystemAccessStatus::kNotSupportedError);
#else
- ASSERT_TRUE(get_observation_future.Get());
+ ASSERT_TRUE(get_observation_future.Get().has_value());
// Constructing an observation registers it with the manager.
- ChangeAccumulator accumulator(get_observation_future.Take());
+ ChangeAccumulator accumulator(get_observation_future.Take().value());
EXPECT_TRUE(
watcher_manager().HasObservationForTesting(accumulator.observation()));
@@ -545,17 +584,21 @@
auto file_path = dir_path.AppendASCII("subdir").AppendASCII("foo");
base::WriteFile(file_path, "watch me");
- base::test::TestFuture<std::unique_ptr<Observation>> get_observation_future;
+ base::test::TestFuture<base::expected<std::unique_ptr<Observation>,
+ blink::mojom::FileSystemAccessErrorPtr>>
+ get_observation_future;
watcher_manager().GetDirectoryObservation(
dir_url,
/*is_recursive=*/false, get_observation_future.GetCallback());
// Watching the local file system is not supported on Android or Fuchsia.
#if BUILDFLAG(IS_ANDROID) || BUILDFLAG(IS_FUCHSIA)
- EXPECT_FALSE(get_observation_future.Get());
+ ASSERT_FALSE(get_observation_future.Get().has_value());
+ EXPECT_EQ(get_observation_future.Get().error()->status,
+ blink::mojom::FileSystemAccessStatus::kNotSupportedError);
#else
- ASSERT_TRUE(get_observation_future.Get());
+ ASSERT_TRUE(get_observation_future.Get().has_value());
- ChangeAccumulator accumulator(get_observation_future.Take());
+ ChangeAccumulator accumulator(get_observation_future.Take().value());
EXPECT_TRUE(
watcher_manager().HasObservationForTesting(accumulator.observation()));
EXPECT_TRUE(watcher_manager().HasSourcesForTesting());
@@ -582,18 +625,22 @@
auto file_path = dir_path.AppendASCII("subdir").AppendASCII("foo");
base::WriteFile(file_path, "watch me");
- base::test::TestFuture<std::unique_ptr<Observation>> get_observation_future;
+ base::test::TestFuture<base::expected<std::unique_ptr<Observation>,
+ blink::mojom::FileSystemAccessErrorPtr>>
+ get_observation_future;
watcher_manager().GetDirectoryObservation(
dir_url,
/*is_recursive=*/true, get_observation_future.GetCallback());
// Watching the local file system is not supported on Android or Fuchsia.
// Recursive watching of the local file system is not supported on iOS.
#if BUILDFLAG(IS_ANDROID) || BUILDFLAG(IS_FUCHSIA) || BUILDFLAG(IS_IOS)
- EXPECT_FALSE(get_observation_future.Get());
+ ASSERT_FALSE(get_observation_future.Get().has_value());
+ EXPECT_EQ(get_observation_future.Get().error()->status,
+ blink::mojom::FileSystemAccessStatus::kNotSupportedError);
#else
- ASSERT_TRUE(get_observation_future.Get());
+ ASSERT_TRUE(get_observation_future.Get().has_value());
- ChangeAccumulator accumulator(get_observation_future.Take());
+ ChangeAccumulator accumulator(get_observation_future.Take().value());
EXPECT_TRUE(
watcher_manager().HasObservationForTesting(accumulator.observation()));
EXPECT_TRUE(watcher_manager().HasSourcesForTesting());
@@ -626,16 +673,20 @@
// Create the file to be watched.
base::WriteFile(file_path, "watch me");
- base::test::TestFuture<std::unique_ptr<Observation>> get_observation_future;
+ base::test::TestFuture<base::expected<std::unique_ptr<Observation>,
+ blink::mojom::FileSystemAccessErrorPtr>>
+ get_observation_future;
watcher_manager().GetFileObservation(file_url,
get_observation_future.GetCallback());
// Watching the local file system is not supported on Android or Fuchsia.
#if BUILDFLAG(IS_ANDROID) || BUILDFLAG(IS_FUCHSIA)
- EXPECT_FALSE(get_observation_future.Get());
+ ASSERT_FALSE(get_observation_future.Get().has_value());
+ EXPECT_EQ(get_observation_future.Get().error()->status,
+ blink::mojom::FileSystemAccessStatus::kNotSupportedError);
#else
- ASSERT_TRUE(get_observation_future.Get());
+ ASSERT_TRUE(get_observation_future.Get().has_value());
- ChangeAccumulator accumulator(get_observation_future.Take());
+ ChangeAccumulator accumulator(get_observation_future.Take().value());
EXPECT_TRUE(
watcher_manager().HasObservationForTesting(accumulator.observation()));
@@ -659,8 +710,9 @@
// Create the file to be watched.
base::WriteFile(file_path, "watch me");
- base::test::TestFuture<std::unique_ptr<Observation>> get_observation_future1,
- get_observation_future2, get_observation_future3;
+ base::test::TestFuture<base::expected<std::unique_ptr<Observation>,
+ blink::mojom::FileSystemAccessErrorPtr>>
+ get_observation_future1, get_observation_future2, get_observation_future3;
watcher_manager().GetFileObservation(file_url,
get_observation_future1.GetCallback());
watcher_manager().GetFileObservation(file_url,
@@ -669,17 +721,23 @@
get_observation_future3.GetCallback());
// Watching the local file system is not supported on Android or Fuchsia.
#if BUILDFLAG(IS_ANDROID) || BUILDFLAG(IS_FUCHSIA)
- EXPECT_FALSE(get_observation_future1.Get());
- EXPECT_FALSE(get_observation_future2.Get());
- EXPECT_FALSE(get_observation_future3.Get());
+ ASSERT_FALSE(get_observation_future1.Get().has_value());
+ ASSERT_FALSE(get_observation_future2.Get().has_value());
+ ASSERT_FALSE(get_observation_future3.Get().has_value());
+ EXPECT_EQ(get_observation_future1.Get().error()->status,
+ blink::mojom::FileSystemAccessStatus::kNotSupportedError);
+ EXPECT_EQ(get_observation_future2.Get().error()->status,
+ blink::mojom::FileSystemAccessStatus::kNotSupportedError);
+ EXPECT_EQ(get_observation_future3.Get().error()->status,
+ blink::mojom::FileSystemAccessStatus::kNotSupportedError);
#else
- ASSERT_TRUE(get_observation_future1.Get());
- ASSERT_TRUE(get_observation_future2.Get());
- ASSERT_TRUE(get_observation_future3.Get());
+ ASSERT_TRUE(get_observation_future1.Get().has_value());
+ ASSERT_TRUE(get_observation_future2.Get().has_value());
+ ASSERT_TRUE(get_observation_future3.Get().has_value());
- ChangeAccumulator accumulator1(get_observation_future1.Take());
- ChangeAccumulator accumulator2(get_observation_future2.Take());
- ChangeAccumulator accumulator3(get_observation_future3.Take());
+ ChangeAccumulator accumulator1(get_observation_future1.Take().value());
+ ChangeAccumulator accumulator2(get_observation_future2.Take().value());
+ ChangeAccumulator accumulator3(get_observation_future3.Take().value());
EXPECT_TRUE(
watcher_manager().HasObservationForTesting(accumulator1.observation()));
EXPECT_TRUE(
@@ -705,16 +763,20 @@
auto file_url = manager_->CreateFileSystemURLFromPath(
FileSystemAccessEntryFactory::PathType::kLocal, file_path);
- base::test::TestFuture<std::unique_ptr<Observation>> get_observation_future;
+ base::test::TestFuture<base::expected<std::unique_ptr<Observation>,
+ blink::mojom::FileSystemAccessErrorPtr>>
+ get_observation_future;
watcher_manager().GetFileObservation(file_url,
get_observation_future.GetCallback());
// Watching the local file system is not supported on Android or Fuchsia.
#if BUILDFLAG(IS_ANDROID) || BUILDFLAG(IS_FUCHSIA)
- EXPECT_FALSE(get_observation_future.Get());
+ ASSERT_FALSE(get_observation_future.Get().has_value());
+ EXPECT_EQ(get_observation_future.Get().error()->status,
+ blink::mojom::FileSystemAccessStatus::kNotSupportedError);
#else
- ASSERT_TRUE(get_observation_future.Get());
+ ASSERT_TRUE(get_observation_future.Get().has_value());
- ChangeAccumulator accumulator(get_observation_future.Take());
+ ChangeAccumulator accumulator(get_observation_future.Take().value());
EXPECT_TRUE(
watcher_manager().HasObservationForTesting(accumulator.observation()));