[NativeFS] Update permissions API following spec change.

This implements the changes from https://github.com/WICG/native-file-system/pull/200.

Change-Id: I9702f9888c4f0fff2084143ac6077b8484dfce16
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2298296Reviewed-by: Trent Apted <tapted@chromium.org>
Reviewed-by: Will Harris <wfh@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#792004}

chrome/browser/ui/views/native_file_system/native_file_system_browsertest.cc

@@ -537,10 +537,17 @@ IN_PROC_BROWSER_TEST_F(NativeFileSystemBrowserTest,
                             "  self.entry = e.data.entry;"
                             "  return self.entry.name; })()"));
 
+  // Try to request permission in iframe, should reject.
+  EXPECT_EQ("SecurityError",
+            content::EvalJs(third_party_iframe,
+                            "self.entry.requestPermission({mode: "
+                            "'readwrite'}).catch(e => e.name)"));
+
   // Have top-level page in first window request write permission.
-  EXPECT_EQ("granted",
-            content::EvalJs(first_party_web_contents,
-                            "self.entry.requestPermission({writable: true})"));
+  EXPECT_EQ(
+      "granted",
+      content::EvalJs(first_party_web_contents,
+                      "self.entry.requestPermission({mode: 'readwrite'})"));
 
   // And write to file from iframe.
   const std::string initial_file_contents = "file contents to write";
@@ -569,7 +576,7 @@ IN_PROC_BROWSER_TEST_F(NativeFileSystemBrowserTest,
   // Permission should still be granted in iframe.
   EXPECT_EQ("granted",
             content::EvalJs(third_party_iframe,
-                            "self.entry.queryPermission({writable: true})"));
+                            "self.entry.queryPermission({mode: 'readwrite'})"));
 
   // Even after triggering the timer in the permission context.
   static_cast<OriginScopedNativeFileSystemPermissionContext*>(
@@ -577,7 +584,7 @@ IN_PROC_BROWSER_TEST_F(NativeFileSystemBrowserTest,
       ->TriggerTimersForTesting();
   EXPECT_EQ("granted",
             content::EvalJs(third_party_iframe,
-                            "self.entry.queryPermission({writable: true})"));
+                            "self.entry.queryPermission({mode: 'readwrite'})"));
 
   // Now navigate away from b.com in third window as well.
   ui_test_utils::NavigateToURL(third_window,
@@ -586,7 +593,7 @@ IN_PROC_BROWSER_TEST_F(NativeFileSystemBrowserTest,
   // Permission should still be granted in iframe.
   EXPECT_EQ("granted",
             content::EvalJs(third_party_iframe,
-                            "self.entry.queryPermission({writable: true})"));
+                            "self.entry.queryPermission({mode: 'readwrite'})"));
 
   // But after triggering the timer in the permission context ...
   static_cast<OriginScopedNativeFileSystemPermissionContext*>(
@@ -596,10 +603,10 @@ IN_PROC_BROWSER_TEST_F(NativeFileSystemBrowserTest,
   // ... permission should have been revoked.
   EXPECT_EQ("prompt",
             content::EvalJs(third_party_iframe,
-                            "self.entry.queryPermission({writable: true})"));
+                            "self.entry.queryPermission({mode: 'readwrite'})"));
   EXPECT_EQ("prompt",
             content::EvalJs(third_party_iframe,
-                            "self.entry.queryPermission({writable: false})"));
+                            "self.entry.queryPermission({mode: 'read'})"));
 }
 
 // Tests that permissions are revoked after all top-level frames have been
@@ -671,15 +678,22 @@ IN_PROC_BROWSER_TEST_F(NativeFileSystemBrowserTest,
                             "  self.entry = e.data.entry;"
                             "  return self.entry.name; })()"));
 
+  // Try to request permission in iframe, should reject.
+  EXPECT_EQ("SecurityError",
+            content::EvalJs(third_party_iframe,
+                            "self.entry.requestPermission({mode: "
+                            "'readwrite'}).catch(e => e.name)"));
+
   // Have top-level page in first window request write permission.
-  EXPECT_EQ("granted",
-            content::EvalJs(first_party_web_contents,
-                            "self.entry.requestPermission({writable: true})"));
+  EXPECT_EQ(
+      "granted",
+      content::EvalJs(first_party_web_contents,
+                      "self.entry.requestPermission({mode: 'readwrite'})"));
 
   // Permission should also be granted in iframe.
   EXPECT_EQ("granted",
             content::EvalJs(third_party_iframe,
-                            "self.entry.queryPermission({writable: true})"));
+                            "self.entry.queryPermission({mode: 'readwrite'})"));
 
   // Now close first window.
   CloseBrowserSynchronously(browser());
@@ -687,7 +701,7 @@ IN_PROC_BROWSER_TEST_F(NativeFileSystemBrowserTest,
   // Permission should still be granted in iframe.
   EXPECT_EQ("granted",
             content::EvalJs(third_party_iframe,
-                            "self.entry.queryPermission({writable: true})"));
+                            "self.entry.queryPermission({mode: 'readwrite'})"));
 
   // But after triggering the timer in the permission context ...
   static_cast<OriginScopedNativeFileSystemPermissionContext*>(
@@ -697,10 +711,10 @@ IN_PROC_BROWSER_TEST_F(NativeFileSystemBrowserTest,
   // ... permission should have been revoked.
   EXPECT_EQ("prompt",
             content::EvalJs(third_party_iframe,
-                            "self.entry.queryPermission({writable: true})"));
+                            "self.entry.queryPermission({mode: 'readwrite'})"));
   EXPECT_EQ("prompt",
             content::EvalJs(third_party_iframe,
-                            "self.entry.queryPermission({writable: false})"));
+                            "self.entry.queryPermission({mode: 'read'})"));
 }
 
 // TODO(mek): Add more end-to-end test including other bits of UI.

chromeos/components/media_app_ui/resources/js/web_app_file_handling.externs.js

@@ -65,7 +65,7 @@ class FileSystemWritableFileStream {
   async seek(offset) {}
 }
 
-/** @typedef {{writable: boolean}} */
+/** @typedef {{mode: string}} */
 let FileSystemHandlePermissionDescriptor;
 
 /** @interface */

content/browser/native_file_system/native_file_system_directory_handle_impl.cc

@@ -102,11 +102,9 @@ void NativeFileSystemDirectoryHandleImpl::GetFile(const std::string& basename,
         base::BindOnce(
             &NativeFileSystemDirectoryHandleImpl::GetFileWithWritePermission,
             weak_factory_.GetWeakPtr(), child_url),
-        base::BindOnce([](GetFileCallback callback) {
-          std::move(callback).Run(
-              native_file_system_error::FromStatus(
-                  NativeFileSystemStatus::kPermissionDenied),
-              mojo::NullRemote());
+        base::BindOnce([](blink::mojom::NativeFileSystemErrorPtr result,
+                          GetFileCallback callback) {
+          std::move(callback).Run(std::move(result), mojo::NullRemote());
         }),
         std::move(callback));
   } else {
@@ -149,11 +147,9 @@ void NativeFileSystemDirectoryHandleImpl::GetDirectory(
         base::BindOnce(&NativeFileSystemDirectoryHandleImpl::
                            GetDirectoryWithWritePermission,
                        weak_factory_.GetWeakPtr(), child_url),
-        base::BindOnce([](GetDirectoryCallback callback) {
-          std::move(callback).Run(
-              native_file_system_error::FromStatus(
-                  NativeFileSystemStatus::kPermissionDenied),
-              mojo::NullRemote());
+        base::BindOnce([](blink::mojom::NativeFileSystemErrorPtr result,
+                          GetDirectoryCallback callback) {
+          std::move(callback).Run(std::move(result), mojo::NullRemote());
         }),
         std::move(callback));
   } else {
@@ -206,9 +202,9 @@ void NativeFileSystemDirectoryHandleImpl::RemoveEntry(
   RunWithWritePermission(
       base::BindOnce(&NativeFileSystemDirectoryHandleImpl::RemoveEntryImpl,
                      weak_factory_.GetWeakPtr(), child_url, recurse),
-      base::BindOnce([](RemoveEntryCallback callback) {
-        std::move(callback).Run(native_file_system_error::FromStatus(
-            NativeFileSystemStatus::kPermissionDenied));
+      base::BindOnce([](blink::mojom::NativeFileSystemErrorPtr result,
+                        RemoveEntryCallback callback) {
+        std::move(callback).Run(std::move(result));
       }),
       std::move(callback));
 }

content/browser/native_file_system/native_file_system_file_handle_impl.cc

@@ -82,10 +82,9 @@ void NativeFileSystemFileHandleImpl::CreateFileWriter(
   RunWithWritePermission(
       base::BindOnce(&NativeFileSystemFileHandleImpl::CreateFileWriterImpl,
                      weak_factory_.GetWeakPtr(), keep_existing_data),
-      base::BindOnce([](CreateFileWriterCallback callback) {
-        std::move(callback).Run(native_file_system_error::FromStatus(
-                                    NativeFileSystemStatus::kPermissionDenied),
-                                mojo::NullRemote());
+      base::BindOnce([](blink::mojom::NativeFileSystemErrorPtr result,
+                        CreateFileWriterCallback callback) {
+        std::move(callback).Run(std::move(result), mojo::NullRemote());
       }),
       std::move(callback));
 }

content/browser/native_file_system/native_file_system_file_writer_impl.cc

@@ -176,9 +176,9 @@ void NativeFileSystemFileWriterImpl::Write(
   RunWithWritePermission(
       base::BindOnce(&NativeFileSystemFileWriterImpl::WriteImpl,
                      weak_factory_.GetWeakPtr(), offset, std::move(data)),
-      base::BindOnce([](WriteCallback callback) {
-        std::move(callback).Run(native_file_system_error::FromStatus(
-                                    NativeFileSystemStatus::kPermissionDenied),
+      base::BindOnce([](blink::mojom::NativeFileSystemErrorPtr result,
+                        WriteCallback callback) {
+        std::move(callback).Run(std::move(result),
                                 /*bytes_written=*/0);
       }),
       std::move(callback));
@@ -193,9 +193,9 @@ void NativeFileSystemFileWriterImpl::WriteStream(
   RunWithWritePermission(
       base::BindOnce(&NativeFileSystemFileWriterImpl::WriteStreamImpl,
                      weak_factory_.GetWeakPtr(), offset, std::move(stream)),
-      base::BindOnce([](WriteStreamCallback callback) {
-        std::move(callback).Run(native_file_system_error::FromStatus(
-                                    NativeFileSystemStatus::kPermissionDenied),
+      base::BindOnce([](blink::mojom::NativeFileSystemErrorPtr result,
+                        WriteStreamCallback callback) {
+        std::move(callback).Run(std::move(result),
                                 /*bytes_written=*/0);
       }),
       std::move(callback));
@@ -208,9 +208,9 @@ void NativeFileSystemFileWriterImpl::Truncate(uint64_t length,
   RunWithWritePermission(
       base::BindOnce(&NativeFileSystemFileWriterImpl::TruncateImpl,
                      weak_factory_.GetWeakPtr(), length),
-      base::BindOnce([](TruncateCallback callback) {
-        std::move(callback).Run(native_file_system_error::FromStatus(
-            NativeFileSystemStatus::kPermissionDenied));
+      base::BindOnce([](blink::mojom::NativeFileSystemErrorPtr result,
+                        TruncateCallback callback) {
+        std::move(callback).Run(std::move(result));
       }),
       std::move(callback));
 }
@@ -221,9 +221,9 @@ void NativeFileSystemFileWriterImpl::Close(CloseCallback callback) {
   RunWithWritePermission(
       base::BindOnce(&NativeFileSystemFileWriterImpl::CloseImpl,
                      weak_factory_.GetWeakPtr()),
-      base::BindOnce([](CloseCallback callback) {
-        std::move(callback).Run(native_file_system_error::FromStatus(
-            NativeFileSystemStatus::kPermissionDenied));
+      base::BindOnce([](blink::mojom::NativeFileSystemErrorPtr result,
+                        CloseCallback callback) {
+        std::move(callback).Run(std::move(result));
       }),
       std::move(callback));
 }

content/browser/native_file_system/native_file_system_handle_base.cc

@@ -147,14 +147,14 @@ void NativeFileSystemHandleBase::DidRequestPermission(
     case Outcome::kThirdPartyContext:
       std::move(callback).Run(
           native_file_system_error::FromStatus(
-              blink::mojom::NativeFileSystemStatus::kPermissionDenied,
+              blink::mojom::NativeFileSystemStatus::kSecurityError,
               "Not allowed to request permissions in this context."),
           writable ? GetWritePermissionStatus() : GetReadPermissionStatus());
       return;
     case Outcome::kNoUserActivation:
       std::move(callback).Run(
           native_file_system_error::FromStatus(
-              blink::mojom::NativeFileSystemStatus::kPermissionDenied,
+              blink::mojom::NativeFileSystemStatus::kSecurityError,
               "User activation is required to request permissions."),
           writable ? GetWritePermissionStatus() : GetReadPermissionStatus());
       return;

content/browser/native_file_system/native_file_system_handle_base.h

@@ -74,7 +74,8 @@ class CONTENT_EXPORT NativeFileSystemHandleBase : public WebContentsObserver {
   template <typename CallbackArgType>
   void RunWithWritePermission(
       base::OnceCallback<void(CallbackArgType)> callback,
-      base::OnceCallback<void(CallbackArgType)> no_permission_callback,
+      base::OnceCallback<void(blink::mojom::NativeFileSystemErrorPtr,
+                              CallbackArgType)> no_permission_callback,
       CallbackArgType callback_arg);
 
  protected:
@@ -205,14 +206,16 @@ class CONTENT_EXPORT NativeFileSystemHandleBase : public WebContentsObserver {
 template <typename CallbackArgType>
 void NativeFileSystemHandleBase::RunWithWritePermission(
     base::OnceCallback<void(CallbackArgType)> callback,
-    base::OnceCallback<void(CallbackArgType)> no_permission_callback,
+    base::OnceCallback<void(blink::mojom::NativeFileSystemErrorPtr,
+                            CallbackArgType)> no_permission_callback,
     CallbackArgType callback_arg) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DoRequestPermission(
       /*writable=*/true,
       base::BindOnce(
           [](base::OnceCallback<void(CallbackArgType)> callback,
-             base::OnceCallback<void(CallbackArgType)> no_permission_callback,
+             base::OnceCallback<void(blink::mojom::NativeFileSystemErrorPtr,
+                                     CallbackArgType)> no_permission_callback,
              CallbackArgType callback_arg,
              blink::mojom::NativeFileSystemErrorPtr result,
              blink::mojom::PermissionStatus status) {
@@ -220,7 +223,12 @@ void NativeFileSystemHandleBase::RunWithWritePermission(
               std::move(callback).Run(std::move(callback_arg));
               return;
             }
-            std::move(no_permission_callback).Run(std::move(callback_arg));
+            if (result->status == blink::mojom::NativeFileSystemStatus::kOk) {
+              result->status =
+                  blink::mojom::NativeFileSystemStatus::kPermissionDenied;
+            }
+            std::move(no_permission_callback)
+                .Run(std::move(result), std::move(callback_arg));
           },
           std::move(callback), std::move(no_permission_callback),
           std::move(callback_arg)));

third_party/blink/public/mojom/native_file_system/native_file_system_error.mojom

@@ -11,6 +11,8 @@ enum NativeFileSystemStatus {
   kOk,
   // The website doesn't/didn't have permission to do what it tried to do.
   kPermissionDenied,
+  // The website wasn't allowed to ask for permission in the current context.
+  kSecurityError,
   // The object being operated on was in an invalid state for the operation.
   kInvalidState,
   // An invalid argument was passed to a method.

third_party/blink/renderer/modules/native_file_system/file_system_handle_permission_descriptor.idl

@@ -2,7 +2,13 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+// http://wicg.github.io/native-file-system/#enumdef-filesystempermissionmode
+enum FileSystemPermissionMode {
+  "read",
+  "readwrite"
+};
+
 // http://wicg.github.io/native-file-system/#dictdef-filesystemhandlepermissiondescriptor
 dictionary FileSystemHandlePermissionDescriptor {
-  boolean writable = false;
+  FileSystemPermissionMode mode = "read";
 };

third_party/blink/renderer/modules/native_file_system/native_file_system_error.cc

@@ -41,6 +41,10 @@ void ResolveOrReject(ScriptPromiseResolver* resolver,
       resolver->Reject(V8ThrowDOMException::CreateOrEmpty(
           isolate, DOMExceptionCode::kNotAllowedError, message));
       break;
+    case mojom::blink::NativeFileSystemStatus::kSecurityError:
+      resolver->Reject(V8ThrowDOMException::CreateOrEmpty(
+          isolate, DOMExceptionCode::kSecurityError, message));
+      break;
     case mojom::blink::NativeFileSystemStatus::kInvalidState:
       resolver->Reject(V8ThrowDOMException::CreateOrEmpty(
           isolate, DOMExceptionCode::kInvalidStateError, message));

third_party/blink/renderer/modules/native_file_system/native_file_system_handle.cc

@@ -49,6 +49,7 @@ String MojoPermissionStatusToString(mojom::blink::PermissionStatus status) {
   NOTREACHED();
   return "denied";
 }
+
 }  // namespace
 
 ScriptPromise NativeFileSystemHandle::queryPermission(
@@ -58,7 +59,7 @@ ScriptPromise NativeFileSystemHandle::queryPermission(
   ScriptPromise result = resolver->Promise();
 
   QueryPermissionImpl(
-      descriptor->writable(),
+      descriptor->mode() == "readwrite",
       WTF::Bind(
           [](ScriptPromiseResolver* resolver,
              mojom::blink::PermissionStatus result) {
@@ -76,7 +77,7 @@ ScriptPromise NativeFileSystemHandle::requestPermission(
   ScriptPromise result = resolver->Promise();
 
   RequestPermissionImpl(
-      descriptor->writable(),
+      descriptor->mode() == "readwrite",
       WTF::Bind(
           [](ScriptPromiseResolver* resolver, NativeFileSystemErrorPtr result,
              mojom::blink::PermissionStatus status) {

third_party/blink/web_tests/external/wpt/native-file-system/resources/messaging-serialize-helpers.js

@@ -31,10 +31,10 @@ async function serialize_handle(handle) {
 // FileSystemDirectoryHandle.
 async function serialize_file_system_handle(handle) {
   const read_permission =
-    await handle.queryPermission({ writable: false });
+    await handle.queryPermission({ mode: 'read' });
 
   const write_permission =
-    await handle.queryPermission({ writable: true })
+    await handle.queryPermission({ mode: 'readwrite' })
 
   return {
     kind: handle.kind,

third_party/blink/web_tests/external/wpt/native-file-system/resources/native-fs-test-helpers.js

@@ -41,14 +41,14 @@ function directory_test(func, description) {
 }
 
 directory_test(async (t, dir) => {
-  assert_equals(await dir.queryPermission({writable: false}), 'granted');
+  assert_equals(await dir.queryPermission({mode: 'read'}), 'granted');
 }, 'User succesfully selected an empty directory.');
 
 directory_test(async (t, dir) => {
-  const status = await dir.queryPermission({writable: true});
+  const status = await dir.queryPermission({mode: 'readwrite'});
   if (status == 'granted')
     return;
 
   await window.test_driver.bless('ask for write permission');
-  assert_equals(await dir.requestPermission({writable: true}), 'granted');
+  assert_equals(await dir.requestPermission({mode: 'readwrite'}), 'granted');
 }, 'User granted write access.');

third_party/blink/web_tests/external/wpt/native-file-system/showSaveFilePicker-manual.https.html

@@ -26,7 +26,7 @@
 
     promise_test(async t => {
       assert_equals(await file.queryPermission(), 'granted');
-      assert_equals(await file.queryPermission({ writable: true }), 'granted');
+      assert_equals(await file.queryPermission({ mode: 'readwrite' }), 'granted');
     }, 'showSaveFilePicker returns correct permissions');
   }, 'showSaveFilePicker works');