FSA: Use the ExceptionState to throw exceptions

Ensures every ScriptPromiseResolver for methods that resolve their
promises asynchronously retain the passed-in ExceptionState

Prior to this, the passed-in ExceptionState could only be used to
throw exceptions synchronously (such as TypeErrors for invalid input)

This allows us to remove the V8ThrowDOMException() hack in
file_system_access_error.cc and just call RejectWithDOMException()
directly

Bug: 1413551
Change-Id: I3c1c340c0f96c8785d812eea46fe3bffa83b1c02
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4226755
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Zain Afzal <zafzal@google.com>
Commit-Queue: Austin Sullivan <asully@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1104524}

ash/webui/media_app_ui/resources/js/launch.js

@@ -954,7 +954,10 @@ async function getFileHandleFromCurrentDirectory(
       // Some filenames (e.g. "thumbs.db") can't be opened (or deleted) by
       // filename. TypeError doesn't give a good error message in the app, so
       // convert to a new Error.
-      if (e.name === 'TypeError' && e.message === 'Name is not allowed.') {
+      if (e.name === 'TypeError' &&
+          e.message ===
+              'Failed to execute \'getFileHandle\' on ' +
+                  '\'FileSystemDirectoryHandle\': Name is not allowed.') {
         console.warn(e);  // Warn so a crash report is not generated.
         throw new DOMException(
             'File has a reserved name and can not be opened',

third_party/blink/renderer/bindings/core/v8/script_promise_resolver.h

@@ -12,6 +12,7 @@
 #include "third_party/blink/renderer/core/core_export.h"
 #include "third_party/blink/renderer/core/execution_context/execution_context.h"
 #include "third_party/blink/renderer/core/execution_context/execution_context_lifecycle_observer.h"
+#include "third_party/blink/renderer/platform/bindings/exception_context.h"
 #include "third_party/blink/renderer/platform/bindings/scoped_persistent.h"
 #include "third_party/blink/renderer/platform/bindings/script_forbidden_scope.h"
 #include "third_party/blink/renderer/platform/bindings/script_state.h"
@@ -117,6 +118,10 @@ class CORE_EXPORT ScriptPromiseResolver
 
   ScriptState* GetScriptState() const { return script_state_; }
 
+  const ExceptionContext& GetExceptionContext() const {
+    return exception_context_;
+  }
+
   // Note that an empty ScriptPromise will be returned after resolve or
   // reject is called.
   ScriptPromise Promise() {

third_party/blink/renderer/modules/file_system_access/data_transfer_item_file_system_access.cc

@@ -17,6 +17,7 @@
 #include "third_party/blink/renderer/core/clipboard/data_transfer_item.h"
 #include "third_party/blink/renderer/core/execution_context/execution_context.h"
 #include "third_party/blink/renderer/modules/file_system_access/file_system_access_error.h"
+#include "third_party/blink/renderer/platform/bindings/exception_state.h"
 #include "third_party/blink/renderer/platform/bindings/script_state.h"
 #include "third_party/blink/renderer/platform/heap/garbage_collected.h"
 #include "third_party/blink/renderer/platform/wtf/functional.h"
@@ -26,7 +27,8 @@ namespace blink {
 // static
 ScriptPromise DataTransferItemFileSystemAccess::getAsFileSystemHandle(
     ScriptState* script_state,
-    DataTransferItem& data_transfer_item) {
+    DataTransferItem& data_transfer_item,
+    ExceptionState& exception_state) {
   if (!data_transfer_item.GetDataTransfer()->CanReadData()) {
     return ScriptPromise::CastUndefined(script_state);
   }
@@ -50,7 +52,8 @@ ScriptPromise DataTransferItemFileSystemAccess::getAsFileSystemHandle(
   mojo::PendingRemote<mojom::blink::FileSystemAccessDataTransferToken>
       token_remote = data_object_item.CloneFileSystemAccessEntryToken();
 
-  auto* resolver = MakeGarbageCollected<ScriptPromiseResolver>(script_state);
+  auto* resolver = MakeGarbageCollected<ScriptPromiseResolver>(
+      script_state, exception_state.GetContext());
   ScriptPromise result = resolver->Promise();
 
   // We need to move `fsa_manager` into GetEntryFromDataTransferToken in order

third_party/blink/renderer/modules/file_system_access/data_transfer_item_file_system_access.h

@@ -10,14 +10,16 @@
 #include "third_party/blink/renderer/modules/file_system_access/file_system_handle.h"
 
 namespace blink {
-
+class ExceptionState;
 class ScriptState;
 
 class DataTransferItemFileSystemAccess {
   STATIC_ONLY(DataTransferItemFileSystemAccess);
 
  public:
-  static ScriptPromise getAsFileSystemHandle(ScriptState*, DataTransferItem&);
+  static ScriptPromise getAsFileSystemHandle(ScriptState*,
+                                             DataTransferItem&,
+                                             ExceptionState&);
 };
 
 }  // namespace blink

third_party/blink/renderer/modules/file_system_access/data_transfer_item_file_system_access.idl

@@ -7,6 +7,9 @@
     ImplementedAs=DataTransferItemFileSystemAccess,
     RuntimeEnabled=FileSystemAccessLocal
 ] partial interface DataTransferItem {
-    [CallWith=ScriptState, MeasureAs=FileSystemAccessDragAndDrop]
-    Promise<FileSystemHandle?> getAsFileSystemHandle();
+    [
+        CallWith=ScriptState,
+        RaisesException,
+        MeasureAs=FileSystemAccessDragAndDrop
+    ] Promise<FileSystemHandle?> getAsFileSystemHandle();
 };

third_party/blink/renderer/modules/file_system_access/file_system_access_error.cc

@@ -4,16 +4,13 @@
 
 #include "third_party/blink/renderer/modules/file_system_access/file_system_access_error.h"
 
+#include "base/files/file.h"
 #include "third_party/blink/public/mojom/file_system_access/file_system_access_error.mojom-blink.h"
 #include "third_party/blink/renderer/bindings/core/v8/script_promise_resolver.h"
-#include "third_party/blink/renderer/bindings/core/v8/v8_throw_dom_exception.h"
 #include "third_party/blink/renderer/core/dom/dom_exception.h"
 #include "third_party/blink/renderer/core/fileapi/file_error.h"
-#include "third_party/blink/renderer/platform/bindings/v8_throw_exception.h"
-#include "third_party/blink/renderer/platform/heap/garbage_collected.h"
 
-namespace blink {
-namespace file_system_access_error {
+namespace blink::file_system_access_error {
 
 void Reject(ScriptPromiseResolver* resolver,
             const mojom::blink::FileSystemAccessError& error) {
@@ -26,7 +23,6 @@ void ResolveOrReject(ScriptPromiseResolver* resolver,
   // Early exit if the resolver's context has been destroyed already.
   if (!resolver->GetScriptState()->ContextIsValid())
     return;
-  auto* const isolate = resolver->GetScriptState()->GetIsolate();
   ScriptState::Scope scope(resolver->GetScriptState());
 
   // Convert empty message to a null string, to make sure we get the default
@@ -38,37 +34,34 @@ void ResolveOrReject(ScriptPromiseResolver* resolver,
       resolver->Resolve();
       break;
     case mojom::blink::FileSystemAccessStatus::kPermissionDenied:
-      resolver->Reject(V8ThrowDOMException::CreateOrEmpty(
-          isolate, DOMExceptionCode::kNotAllowedError, message));
+      resolver->RejectWithDOMException(DOMExceptionCode::kNotAllowedError,
+                                       message);
       break;
     case mojom::blink::FileSystemAccessStatus::kNoModificationAllowedError:
-      resolver->Reject(V8ThrowDOMException::CreateOrEmpty(
-          isolate, DOMExceptionCode::kNoModificationAllowedError, message));
+      resolver->RejectWithDOMException(
+          DOMExceptionCode::kNoModificationAllowedError, message);
       break;
     case mojom::blink::FileSystemAccessStatus::kInvalidModificationError:
-      resolver->Reject(V8ThrowDOMException::CreateOrEmpty(
-          isolate, DOMExceptionCode::kInvalidModificationError, message));
+      resolver->RejectWithDOMException(
+          DOMExceptionCode::kInvalidModificationError, message);
       break;
     case mojom::blink::FileSystemAccessStatus::kSecurityError:
-      resolver->Reject(V8ThrowDOMException::CreateOrEmpty(
-          isolate, DOMExceptionCode::kSecurityError, message));
+      resolver->RejectWithSecurityError(message, message);
       break;
     case mojom::blink::FileSystemAccessStatus::kNotSupportedError:
-      resolver->Reject(V8ThrowDOMException::CreateOrEmpty(
-          isolate, DOMExceptionCode::kNotSupportedError, message));
+      resolver->RejectWithDOMException(DOMExceptionCode::kNotSupportedError,
+                                       message);
       break;
     case mojom::blink::FileSystemAccessStatus::kInvalidState:
-      resolver->Reject(V8ThrowDOMException::CreateOrEmpty(
-          isolate, DOMExceptionCode::kInvalidStateError, message));
+      resolver->RejectWithDOMException(DOMExceptionCode::kInvalidStateError,
+                                       message);
       break;
     case mojom::blink::FileSystemAccessStatus::kInvalidArgument:
-      resolver->Reject(V8ThrowException::CreateTypeError(
-          resolver->GetScriptState()->GetIsolate(), message));
+      resolver->RejectWithTypeError(message);
       break;
     case mojom::blink::FileSystemAccessStatus::kOperationFailed:
     case mojom::blink::FileSystemAccessStatus::kOperationAborted:
-      resolver->Reject(V8ThrowDOMException::CreateOrEmpty(
-          isolate, DOMExceptionCode::kAbortError, message));
+      resolver->RejectWithDOMException(DOMExceptionCode::kAbortError, message);
       break;
     case mojom::blink::FileSystemAccessStatus::kFileError:
       // TODO(mek): We might want to support custom messages for these cases.
@@ -77,5 +70,4 @@ void ResolveOrReject(ScriptPromiseResolver* resolver,
   }
 }
 
-}  // namespace file_system_access_error
-}  // namespace blink
+}  // namespace blink::file_system_access_error

third_party/blink/renderer/modules/file_system_access/file_system_access_error.h

@@ -5,7 +5,6 @@
 #ifndef THIRD_PARTY_BLINK_RENDERER_MODULES_FILE_SYSTEM_ACCESS_FILE_SYSTEM_ACCESS_ERROR_H_
 #define THIRD_PARTY_BLINK_RENDERER_MODULES_FILE_SYSTEM_ACCESS_FILE_SYSTEM_ACCESS_ERROR_H_
 
-#include "base/files/file.h"
 #include "third_party/blink/public/mojom/file_system_access/file_system_access_error.mojom-blink-forward.h"
 
 namespace blink {

third_party/blink/renderer/modules/file_system_access/file_system_directory_handle.cc

@@ -5,27 +5,17 @@
 #include "third_party/blink/renderer/modules/file_system_access/file_system_directory_handle.h"
 
 #include "mojo/public/cpp/bindings/pending_remote.h"
-#include "mojo/public/cpp/bindings/remote.h"
-#include "services/network/public/mojom/web_sandbox_flags.mojom-blink.h"
-#include "third_party/blink/public/common/browser_interface_broker_proxy.h"
 #include "third_party/blink/public/mojom/file_system_access/file_system_access_error.mojom-blink.h"
-#include "third_party/blink/public/mojom/file_system_access/file_system_access_manager.mojom-blink.h"
 #include "third_party/blink/public/platform/task_type.h"
 #include "third_party/blink/renderer/bindings/core/v8/script_promise_resolver.h"
-#include "third_party/blink/renderer/bindings/core/v8/v8_throw_dom_exception.h"
-#include "third_party/blink/renderer/bindings/modules/v8/v8_file_system_directory_handle.h"
 #include "third_party/blink/renderer/bindings/modules/v8/v8_file_system_get_directory_options.h"
 #include "third_party/blink/renderer/bindings/modules/v8/v8_file_system_get_file_options.h"
 #include "third_party/blink/renderer/bindings/modules/v8/v8_file_system_remove_options.h"
-#include "third_party/blink/renderer/core/dom/dom_exception.h"
 #include "third_party/blink/renderer/core/execution_context/execution_context.h"
-#include "third_party/blink/renderer/core/execution_context/security_context.h"
-#include "third_party/blink/renderer/core/fileapi/file_error.h"
 #include "third_party/blink/renderer/modules/file_system_access/file_system_access_error.h"
 #include "third_party/blink/renderer/modules/file_system_access/file_system_directory_iterator.h"
 #include "third_party/blink/renderer/modules/file_system_access/file_system_file_handle.h"
 #include "third_party/blink/renderer/platform/bindings/exception_state.h"
-#include "third_party/blink/renderer/platform/weborigin/security_origin.h"
 #include "third_party/blink/renderer/platform/wtf/functional.h"
 #include "third_party/blink/renderer/platform/wtf/text/wtf_string.h"
 
@@ -96,7 +86,8 @@ ScriptPromise FileSystemDirectoryHandle::getFileHandle(
     return ScriptPromise();
   }
 
-  auto* resolver = MakeGarbageCollected<ScriptPromiseResolver>(script_state);
+  auto* resolver = MakeGarbageCollected<ScriptPromiseResolver>(
+      script_state, exception_state.GetContext());
   ScriptPromise result = resolver->Promise();
 
   mojo_ptr_->GetFile(
@@ -134,7 +125,8 @@ ScriptPromise FileSystemDirectoryHandle::getDirectoryHandle(
     return ScriptPromise();
   }
 
-  auto* resolver = MakeGarbageCollected<ScriptPromiseResolver>(script_state);
+  auto* resolver = MakeGarbageCollected<ScriptPromiseResolver>(
+      script_state, exception_state.GetContext());
   ScriptPromise result = resolver->Promise();
 
   mojo_ptr_->GetDirectory(
@@ -172,7 +164,8 @@ ScriptPromise FileSystemDirectoryHandle::removeEntry(
     return ScriptPromise();
   }
 
-  auto* resolver = MakeGarbageCollected<ScriptPromiseResolver>(script_state);
+  auto* resolver = MakeGarbageCollected<ScriptPromiseResolver>(
+      script_state, exception_state.GetContext());
   ScriptPromise result = resolver->Promise();
 
   mojo_ptr_->RemoveEntry(
@@ -199,7 +192,8 @@ ScriptPromise FileSystemDirectoryHandle::resolve(
     return ScriptPromise();
   }
 
-  auto* resolver = MakeGarbageCollected<ScriptPromiseResolver>(script_state);
+  auto* resolver = MakeGarbageCollected<ScriptPromiseResolver>(
+      script_state, exception_state.GetContext());
   ScriptPromise result = resolver->Promise();
 
   mojo_ptr_->Resolve(
@@ -324,6 +318,8 @@ void FileSystemDirectoryHandle::IsSameEntryImpl(
 void FileSystemDirectoryHandle::GetUniqueIdImpl(
     base::OnceCallback<void(const WTF::String&)> callback) {
   if (!mojo_ptr_.is_bound()) {
+    // TODO(crbug.com/1413551): Consider throwing a kInvalidState exception here
+    // rather than returning an empty string.
     std::move(callback).Run("");
     return;
   }

third_party/blink/renderer/modules/file_system_access/file_system_directory_iterator.cc

@@ -7,12 +7,9 @@
 #include "third_party/blink/public/platform/task_type.h"
 #include "third_party/blink/renderer/bindings/core/v8/iterable.h"
 #include "third_party/blink/renderer/bindings/core/v8/script_promise_resolver.h"
-#include "third_party/blink/renderer/core/dom/dom_exception.h"
-#include "third_party/blink/renderer/core/fileapi/file_error.h"
 #include "third_party/blink/renderer/modules/file_system_access/file_system_access_error.h"
 #include "third_party/blink/renderer/modules/file_system_access/file_system_directory_handle.h"
-#include "third_party/blink/renderer/modules/file_system_access/file_system_file_handle.h"
-#include "third_party/blink/renderer/platform/wtf/functional.h"
+#include "third_party/blink/renderer/platform/bindings/exception_state.h"
 
 namespace blink {
 
@@ -28,15 +25,23 @@ FileSystemDirectoryIterator::FileSystemDirectoryIterator(
       execution_context->GetTaskRunner(TaskType::kStorage)));
 }
 
-ScriptPromise FileSystemDirectoryIterator::next(ScriptState* script_state) {
+ScriptPromise FileSystemDirectoryIterator::next(
+    ScriptState* script_state,
+    ExceptionState& exception_state) {
+  return nextImpl(script_state, exception_state.GetContext());
+}
+
+ScriptPromise FileSystemDirectoryIterator::nextImpl(
+    ScriptState* script_state,
+    const ExceptionContext& exception_context) {
   // TODO(crbug.com/1087157): The bindings layer should implement async
   // iterable. Until it gets implemented, this class (and especially this
   // member function) implements the behavior of async iterable in its own way.
   // Use of bindings internal code (use of bindings:: internal namespace) should
   // be gone once https://crbug.com/1087157 gets resolved.
-
   if (error_) {
-    auto* resolver = MakeGarbageCollected<ScriptPromiseResolver>(script_state);
+    auto* resolver = MakeGarbageCollected<ScriptPromiseResolver>(
+        script_state, exception_context);
     auto result = resolver->Promise();
     file_system_access_error::Reject(resolver, *error_);
     return result;
@@ -72,7 +77,8 @@ ScriptPromise FileSystemDirectoryIterator::next(ScriptState* script_state) {
 
   if (waiting_for_more_entries_) {
     DCHECK(!pending_next_);
-    pending_next_ = MakeGarbageCollected<ScriptPromiseResolver>(script_state);
+    pending_next_ = MakeGarbageCollected<ScriptPromiseResolver>(
+        script_state, exception_context);
     return pending_next_->Promise();
   }
 
@@ -116,8 +122,9 @@ void FileSystemDirectoryIterator::DidReadDirectory(
   waiting_for_more_entries_ = has_more_entries;
   if (pending_next_) {
     ScriptState::Scope scope(pending_next_->GetScriptState());
-    pending_next_->Resolve(
-        next(pending_next_->GetScriptState()).AsScriptValue());
+    pending_next_->Resolve(nextImpl(pending_next_->GetScriptState(),
+                                    pending_next_->GetExceptionContext())
+                               .AsScriptValue());
     pending_next_ = nullptr;
   }
 }

third_party/blink/renderer/modules/file_system_access/file_system_directory_iterator.h

@@ -5,17 +5,17 @@
 #ifndef THIRD_PARTY_BLINK_RENDERER_MODULES_FILE_SYSTEM_ACCESS_FILE_SYSTEM_DIRECTORY_ITERATOR_H_
 #define THIRD_PARTY_BLINK_RENDERER_MODULES_FILE_SYSTEM_ACCESS_FILE_SYSTEM_DIRECTORY_ITERATOR_H_
 
-#include "base/files/file.h"
 #include "third_party/blink/public/mojom/file_system_access/file_system_access_directory_handle.mojom-blink.h"
 #include "third_party/blink/public/mojom/file_system_access/file_system_access_error.mojom-blink.h"
 #include "third_party/blink/renderer/bindings/core/v8/active_script_wrappable.h"
 #include "third_party/blink/renderer/core/execution_context/execution_context_lifecycle_observer.h"
 #include "third_party/blink/renderer/platform/bindings/script_wrappable.h"
 #include "third_party/blink/renderer/platform/heap/collection_support/heap_deque.h"
 #include "third_party/blink/renderer/platform/mojo/heap_mojo_receiver.h"
-#include "third_party/blink/renderer/platform/mojo/heap_mojo_wrapper_mode.h"
 
 namespace blink {
+class ExceptionContext;
+class ExceptionState;
 class FileSystemDirectoryHandle;
 class FileSystemHandle;
 class ScriptPromise;
@@ -37,14 +37,16 @@ class FileSystemDirectoryIterator final
                               Mode mode,
                               ExecutionContext* execution_context);
 
-  ScriptPromise next(ScriptState*);
+  ScriptPromise next(ScriptState*, ExceptionState&);
 
   // ScriptWrappable:
   bool HasPendingActivity() const final;
 
   void Trace(Visitor*) const override;
 
  private:
+  ScriptPromise nextImpl(ScriptState*, const ExceptionContext&);
+
   void DidReadDirectory(mojom::blink::FileSystemAccessErrorPtr result,
                         Vector<mojom::blink::FileSystemAccessEntryPtr> entries,
                         bool has_more_entries) override;

third_party/blink/renderer/modules/file_system_access/file_system_directory_iterator.idl

@@ -10,5 +10,5 @@
     ActiveScriptWrappable,
     RuntimeEnabled=FileSystemAccess
 ] interface FileSystemDirectoryIterator {
-  [CallWith=ScriptState] Promise<any> next();
+  [CallWith=ScriptState, RaisesException] Promise<any> next();
 };