Storage Buckets: Reject getDirectory() promise for deleted bucket

Change-Id: I82a84c5ca2af53f591643bd4f4e327a6dc4969fd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4074546
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: Austin Sullivan <asully@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1080079}

content/browser/file_system_access/file_system_access_manager_impl_unittest.cc

@@ -501,15 +501,11 @@ TEST_F(FileSystemAccessManagerImplTest, GetSandboxedFileSystem_BadBucket) {
       handle_future;
   manager_->GetSandboxedFileSystem(binding_context, bucket,
                                    handle_future.GetCallback());
-  EXPECT_EQ(blink::mojom::FileSystemAccessStatus::kOk,
+  EXPECT_EQ(blink::mojom::FileSystemAccessStatus::kFileError,
             handle_future.Get<0>()->status);
-
   mojo::Remote<blink::mojom::FileSystemAccessDirectoryHandle> root(
       std::move(std::get<1>(handle_future.Take())));
-  // Currently we intentionally return a non-functional file/directory handle
-  // in the case of a bad bucket override, as there is currently no better way
-  // of representing a handle to a bucket that no longer exists.
-  ASSERT_TRUE(root);
+  EXPECT_FALSE(root);
 }
 
 TEST_F(FileSystemAccessManagerImplTest, GetSandboxedFileSystem_Permissions) {

storage/browser/file_system/file_system_context.cc

@@ -431,18 +431,31 @@ void FileSystemContext::OpenFileSystem(
   }
 
   // Quota manager isn't provided by all tests.
-  if (quota_manager_proxy() && !bucket.has_value()) {
+  if (!quota_manager_proxy()) {
+    ResolveURLOnOpenFileSystem(storage_key, bucket, type, mode,
+                               std::move(callback));
+    return;
+  }
+
+  auto got_bucket = base::BindOnce(&FileSystemContext::OnGetOrCreateBucket,
+                                   weak_factory_.GetWeakPtr(), storage_key,
+                                   type, mode, std::move(callback));
+  if (bucket.has_value()) {
+    if (!bucket->id) {
+      // This branch can be hit if the bucket has been deleted but `BucketHost`
+      // is still alive.
+      std::move(got_bucket).Run(QuotaError::kUnknownError);
+    } else {
+      quota_manager_proxy()->GetBucketById(bucket->id, io_task_runner_.get(),
+                                           std::move(got_bucket));
+    }
+  } else {
     // Ensure default bucket for `storage_key` exists so that Quota API
     // is aware of the usage.
     quota_manager_proxy()->GetOrCreateBucketDeprecated(
         BucketInitParams::ForDefaultBucket(storage_key),
         FileSystemTypeToQuotaStorageType(type), io_task_runner_.get(),
-        base::BindOnce(&FileSystemContext::OnGetOrCreateBucket,
-                       weak_factory_.GetWeakPtr(), storage_key, type, mode,
-                       std::move(callback)));
-  } else {
-    ResolveURLOnOpenFileSystem(storage_key, bucket, type, mode,
-                               std::move(callback));
+        std::move(got_bucket));
   }
 }
 

third_party/blink/web_tests/external/wpt/fs/FileSystemBaseHandle-buckets.https.any.js

@@ -0,0 +1,3 @@
+// META: script=resources/test-helpers.js
+// META: script=resources/sandboxed-fs-test-helpers.js
+// META: script=script-tests/FileSystemBaseHandle-buckets.js

third_party/blink/web_tests/external/wpt/fs/script-tests/FileSystemBaseHandle-buckets.js

@@ -0,0 +1,19 @@
+'use strict';
+
+directory_test(async (t, root_dir) => {
+  const inboxBucket = await navigator.storageBuckets.open('inbox');
+  const inboxRootDir = await inboxBucket.getDirectory();
+
+  assert_false(await inboxRootDir.isSameEntry(root_dir));
+
+  const handle1 = await createEmptyFile(t, 'mtime.txt', inboxRootDir);
+  const handle2 = await inboxRootDir.getFileHandle('mtime.txt');
+  assert_true(await handle1.isSameEntry(handle2));
+}, 'isSameEntry works as expected with buckets');
+
+directory_test(async (t, root_dir) => {
+  const inboxBucket = await navigator.storageBuckets.open('inbox');
+  await navigator.storageBuckets.delete('inbox');
+  const directoryPromise = inboxBucket.getDirectory();
+  await promise_rejects_dom(t, 'InvalidStateError', directoryPromise);
+}, 'getDirectory promise rejects if bucket has been deleted');

third_party/blink/web_tests/external/wpt/fs/script-tests/FileSystemBaseHandle-isSameEntry.js

@@ -105,14 +105,3 @@ directory_test(async (t, root_dir) => {
       await file_handle.isSameEntry(dir_handle),
       'a file and directory handle pointing at the same path should not be considered the same entry');
 }, 'isSameEntry comparing a file to a directory of the same path returns false');
-
-directory_test(async (t, root_dir) => {
-  const inboxBucket = await navigator.storageBuckets.open('inbox');
-  const inboxRootDir = await inboxBucket.getDirectory();
-
-  assert_false(await inboxRootDir.isSameEntry(root_dir));
-
-  const handle1 = await createEmptyFile(t, 'mtime.txt', inboxRootDir);
-  const handle2 = await inboxRootDir.getFileHandle('mtime.txt');
-  assert_true(await handle1.isSameEntry(handle2));
-}, 'isSameEntry works as expected with buckets');