[NativeFS] Make it possible to request read&write access at the same time.

When restoring a handle from IDB it won't be readable. If a website then
requests write access we need to prompt for both at the same time. This CL
implements this by removing a now invalid DCHECK in NativeFileSystemHandleBase
and adding support to NativeFileSystemPermissionRequestManager to combine
two requests into one prompt.

Bug: 984769
Change-Id: Ib1574c8142943adc412e7310505b63fc061f4557
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2092189
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Reviewed-by: Olivier Yiptong <oyiptong@chromium.org>
Cr-Commit-Position: refs/heads/master@{#747991}

chrome/browser/native_file_system/native_file_system_permission_request_manager.cc

@@ -20,6 +20,13 @@ bool RequestsAreIdentical(
          a.is_directory == b.is_directory && a.access == b.access;
 }
 
+bool RequestsAreForSamePath(
+    const NativeFileSystemPermissionRequestManager::RequestData& a,
+    const NativeFileSystemPermissionRequestManager::RequestData& b) {
+  return a.origin == b.origin && a.path == b.path &&
+         a.is_directory == b.is_directory;
+}
+
 }  // namespace
 
 struct NativeFileSystemPermissionRequestManager::Request {
@@ -30,7 +37,7 @@ struct NativeFileSystemPermissionRequestManager::Request {
     callbacks.push_back(std::move(callback));
   }
 
-  const RequestData data;
+  RequestData data;
   std::vector<base::OnceCallback<void(permissions::PermissionAction result)>>
       callbacks;
 };
@@ -57,6 +64,13 @@ void NativeFileSystemPermissionRequestManager::AddRequest(
       request->callbacks.push_back(std::move(callback));
       return;
     }
+    if (RequestsAreForSamePath(request->data, data)) {
+      // This means access levels are different. Change the existing request
+      // to kReadWrite, and add the new callback.
+      request->data.access = Access::kReadWrite;
+      request->callbacks.push_back(std::move(callback));
+      return;
+    }
   }
 
   queued_requests_.push_back(

chrome/browser/native_file_system/origin_scoped_native_file_system_permission_context.cc

@@ -143,8 +143,10 @@ class OriginScopedNativeFileSystemPermissionContext::PermissionGrantImpl
             ? NativeFileSystemPermissionRequestManager::Access::kRead
             : NativeFileSystemPermissionRequestManager::Access::kWrite;
 
-    // TODO(mek): We need to somehow deal with the case where both read and
-    // write access are being asked for at the same time.
+    // If a website wants both read and write access, code in content will
+    // request those as two separate requests. The |request_manager| will then
+    // detect this and combine the two requests into one prompt. As such this
+    // code does not have to have any way to request Access::kReadWrite.
 
     request_manager->AddRequest(
         {origin_, path_, is_directory_, access},

content/browser/native_file_system/native_file_system_handle_base.cc

@@ -144,10 +144,16 @@ void NativeFileSystemHandleBase::DoRequestPermission(
     return;
   }
 
-  // TODO(https://crbug.com/971401): Today we can't prompt for read permission,
-  // and should never be in state "ASK". Since we already checked for DENIED
-  // above current status here should always be GRANTED.
-  DCHECK_EQ(GetReadPermissionStatus(), PermissionStatus::GRANTED);
+  // Ask for both read and write permission at the same time, the permission
+  // context should coalesce these into one prompt.
+  if (GetReadPermissionStatus() == PermissionStatus::ASK) {
+    // Ignore callback for the read permission request; if the request fails,
+    // the write permission request probably fails the same way. And we check
+    // the final permission status after the permission request completes
+    // anyway.
+    handle_state_.read_grant->RequestPermission(
+        context().process_id, context().frame_id, base::DoNothing());
+  }
 
   handle_state_.write_grant->RequestPermission(
       context().process_id, context().frame_id,