[FS] Add a GetPath method to NFSPermissionGrant.

Rather than relying on the root of an isolated file system to determine
the scope of permission grants, explicitly expose the scope of
permission grants. This will make it easier to support special volumes
on Chrome OS.

Bug: 1093653
Change-Id: I17ef72864af6441a9f5f4392d537f7d7f7328cb3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2391384Reviewed-by: Victor Costan <pwnall@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Auto-Submit: Marijn Kruisselbrink <mek@chromium.org>
Commit-Queue: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#804073}

chrome/browser/native_file_system/origin_scoped_native_file_system_permission_context.cc

@@ -50,11 +50,21 @@ class OriginScopedNativeFileSystemPermissionContext::PermissionGrantImpl
         type_(type) {}
 
   // NativeFileSystemPermissionGrant:
-  PermissionStatus GetStatus() override { return status_; }
+  PermissionStatus GetStatus() override {
+    DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+    return status_;
+  }
+  base::FilePath GetPath() override {
+    DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+    return path_;
+  }
+
   void RequestPermission(
       content::GlobalFrameRoutingId frame_id,
       UserActivationState user_activation_state,
       base::OnceCallback<void(PermissionRequestOutcome)> callback) override {
+    DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
     // Check if a permission request has already been processed previously. This
     // check is done first because we don't want to reset the status of a
     // permission if it has already been granted.

content/browser/native_file_system/file_system_chooser_browsertest.cc

@@ -432,7 +432,7 @@ IN_PROC_BROWSER_TEST_F(FileSystemChooserBrowserTest, OpenDirectory_DenyAccess) {
   auto read_grant = base::MakeRefCounted<
       testing::StrictMock<MockNativeFileSystemPermissionGrant>>();
   auto write_grant = base::MakeRefCounted<FixedNativeFileSystemPermissionGrant>(
-      PermissionStatus::ASK);
+      PermissionStatus::ASK, base::FilePath());
 
   EXPECT_CALL(permission_context,
               CanObtainReadPermission(url::Origin::Create(

content/browser/native_file_system/fixed_native_file_system_permission_grant.cc

@@ -7,8 +7,9 @@
 namespace content {
 
 FixedNativeFileSystemPermissionGrant::FixedNativeFileSystemPermissionGrant(
-    PermissionStatus status)
-    : status_(status) {}
+    PermissionStatus status,
+    base::FilePath path)
+    : status_(status), path_(std::move(path)) {}
 
 FixedNativeFileSystemPermissionGrant::~FixedNativeFileSystemPermissionGrant() =
     default;
@@ -18,6 +19,10 @@ FixedNativeFileSystemPermissionGrant::GetStatus() {
   return status_;
 }
 
+base::FilePath FixedNativeFileSystemPermissionGrant::GetPath() {
+  return path_;
+}
+
 void FixedNativeFileSystemPermissionGrant::RequestPermission(
     GlobalFrameRoutingId frame_id,
     UserActivationState user_activation_state,

content/browser/native_file_system/fixed_native_file_system_permission_grant.h

@@ -19,10 +19,12 @@ namespace content {
 class CONTENT_EXPORT FixedNativeFileSystemPermissionGrant
     : public NativeFileSystemPermissionGrant {
  public:
-  explicit FixedNativeFileSystemPermissionGrant(PermissionStatus status);
+  explicit FixedNativeFileSystemPermissionGrant(PermissionStatus status,
+                                                base::FilePath path);
 
   // NativeFileSystemPermissionGrant:
   PermissionStatus GetStatus() override;
+  base::FilePath GetPath() override;
   void RequestPermission(
       GlobalFrameRoutingId frame_id,
       UserActivationState user_activation_state,
@@ -33,6 +35,7 @@ class CONTENT_EXPORT FixedNativeFileSystemPermissionGrant
 
  private:
   const PermissionStatus status_;
+  const base::FilePath path_;
 };
 
 }  // namespace content

content/browser/native_file_system/mock_native_file_system_permission_grant.h

@@ -17,6 +17,7 @@ class MockNativeFileSystemPermissionGrant
   MockNativeFileSystemPermissionGrant();
 
   MOCK_METHOD0(GetStatus, PermissionStatus());
+  MOCK_METHOD0(GetPath, base::FilePath());
   void RequestPermission(
       GlobalFrameRoutingId frame_id,
       UserActivationState user_activation_state,

content/browser/native_file_system/native_file_system_file_handle_impl_unittest.cc

@@ -115,7 +115,8 @@ class NativeFileSystemFileHandleImplTest : public testing::Test {
 
   scoped_refptr<FixedNativeFileSystemPermissionGrant> allow_grant_ =
       base::MakeRefCounted<FixedNativeFileSystemPermissionGrant>(
-          FixedNativeFileSystemPermissionGrant::PermissionStatus::GRANTED);
+          FixedNativeFileSystemPermissionGrant::PermissionStatus::GRANTED,
+          base::FilePath());
   std::unique_ptr<NativeFileSystemFileHandleImpl> handle_;
 };
 

content/browser/native_file_system/native_file_system_file_writer_impl_unittest.cc

@@ -340,7 +340,8 @@ class NativeFileSystemFileWriterImplTest : public testing::Test {
 
   scoped_refptr<FixedNativeFileSystemPermissionGrant> permission_grant_ =
       base::MakeRefCounted<FixedNativeFileSystemPermissionGrant>(
-          FixedNativeFileSystemPermissionGrant::PermissionStatus::GRANTED);
+          FixedNativeFileSystemPermissionGrant::PermissionStatus::GRANTED,
+          base::FilePath());
   std::unique_ptr<NativeFileSystemFileWriterImpl> handle_;
 };
 

content/browser/native_file_system/native_file_system_manager_impl.cc

@@ -448,9 +448,10 @@ void NativeFileSystemManagerImpl::DidResolveForSerializeHandle(
   switch (url.type()) {
     case storage::kFileSystemTypeNativeLocal: {
       DCHECK_EQ(url.mount_type(), storage::kFileSystemTypeIsolated);
-      base::FilePath root_path;
-      storage::IsolatedContext::GetInstance()->GetRegisteredPath(
-          url.filesystem_id(), &root_path);
+      base::FilePath root_path = resolved_token->GetWriteGrant()->GetPath();
+      if (root_path.empty())
+        root_path = url.path();
+
       data.mutable_native()->set_root_path(SerializePath(root_path));
 
       base::FilePath relative_path;
@@ -512,7 +513,7 @@ void NativeFileSystemManagerImpl::DeserializeHandle(
 
       auto permission_grant =
           base::MakeRefCounted<FixedNativeFileSystemPermissionGrant>(
-              PermissionStatus::GRANTED);
+              PermissionStatus::GRANTED, base::FilePath());
       CreateTransferTokenImpl(
           url, origin,
           SharedHandleState(permission_grant, permission_grant, {}),
@@ -754,7 +755,7 @@ void NativeFileSystemManagerImpl::DidOpenSandboxedFileSystem(
 
   auto permission_grant =
       base::MakeRefCounted<FixedNativeFileSystemPermissionGrant>(
-          PermissionStatus::GRANTED);
+          PermissionStatus::GRANTED, base::FilePath());
 
   std::move(callback).Run(
       native_file_system_error::Ok(),
@@ -1037,7 +1038,8 @@ NativeFileSystemManagerImpl::GetSharedHandleStateForPath(
         base::CommandLine::ForCurrentProcess()->HasSwitch(
             switches::kEnableExperimentalWebPlatformFeatures)
             ? PermissionStatus::GRANTED
-            : PermissionStatus::DENIED);
+            : PermissionStatus::DENIED,
+        path);
     if (user_action ==
         NativeFileSystemPermissionContext::UserAction::kLoadFromStorage) {
       read_grant = write_grant;
@@ -1045,7 +1047,7 @@ NativeFileSystemManagerImpl::GetSharedHandleStateForPath(
       // Grant read permission even without a permission_context_, as the picker
       // itself is enough UI to assume user intent.
       read_grant = base::MakeRefCounted<FixedNativeFileSystemPermissionGrant>(
-          PermissionStatus::GRANTED);
+          PermissionStatus::GRANTED, path);
     }
   }
   return SharedHandleState(std::move(read_grant), std::move(write_grant),

content/browser/native_file_system/native_file_system_manager_impl_unittest.cc

@@ -150,16 +150,19 @@ class NativeFileSystemManagerImplTest : public testing::Test {
 
   mojo::Remote<blink::mojom::NativeFileSystemDirectoryHandle>
   GetHandleForDirectory(const base::FilePath& path) {
+    auto grant = base::MakeRefCounted<FixedNativeFileSystemPermissionGrant>(
+        FixedNativeFileSystemPermissionGrant::PermissionStatus::GRANTED, path);
+
     EXPECT_CALL(permission_context_,
                 GetReadPermissionGrant(
                     kTestOrigin, path, HandleType::kDirectory,
                     NativeFileSystemPermissionContext::UserAction::kOpen))
-        .WillOnce(testing::Return(allow_grant_));
+        .WillOnce(testing::Return(grant));
     EXPECT_CALL(permission_context_,
                 GetWritePermissionGrant(
                     kTestOrigin, path, HandleType::kDirectory,
                     NativeFileSystemPermissionContext::UserAction::kOpen))
-        .WillOnce(testing::Return(allow_grant_));
+        .WillOnce(testing::Return(grant));
 
     blink::mojom::NativeFileSystemEntryPtr entry =
         manager_->CreateDirectoryEntryFromPath(kBindingContext, path);
@@ -219,13 +222,16 @@ class NativeFileSystemManagerImplTest : public testing::Test {
 
   scoped_refptr<FixedNativeFileSystemPermissionGrant> ask_grant_ =
       base::MakeRefCounted<FixedNativeFileSystemPermissionGrant>(
-          FixedNativeFileSystemPermissionGrant::PermissionStatus::ASK);
+          FixedNativeFileSystemPermissionGrant::PermissionStatus::ASK,
+          base::FilePath());
   scoped_refptr<FixedNativeFileSystemPermissionGrant> ask_grant2_ =
       base::MakeRefCounted<FixedNativeFileSystemPermissionGrant>(
-          FixedNativeFileSystemPermissionGrant::PermissionStatus::ASK);
+          FixedNativeFileSystemPermissionGrant::PermissionStatus::ASK,
+          base::FilePath());
   scoped_refptr<FixedNativeFileSystemPermissionGrant> allow_grant_ =
       base::MakeRefCounted<FixedNativeFileSystemPermissionGrant>(
-          FixedNativeFileSystemPermissionGrant::PermissionStatus::GRANTED);
+          FixedNativeFileSystemPermissionGrant::PermissionStatus::GRANTED,
+          base::FilePath());
 };
 
 TEST_F(NativeFileSystemManagerImplTest, GetSandboxedFileSystem_Permissions) {
@@ -454,17 +460,21 @@ TEST_F(NativeFileSystemManagerImplTest, SerializeHandle_SandboxedDirectory) {
 TEST_F(NativeFileSystemManagerImplTest, SerializeHandle_Native_SingleFile) {
   const base::FilePath kTestPath(dir_.GetPath().AppendASCII("foo"));
 
+  auto grant = base::MakeRefCounted<FixedNativeFileSystemPermissionGrant>(
+      FixedNativeFileSystemPermissionGrant::PermissionStatus::GRANTED,
+      kTestPath);
+
   // Expect calls to get grants when creating the initial handle.
   EXPECT_CALL(permission_context_,
               GetReadPermissionGrant(
                   kTestOrigin, kTestPath, HandleType::kFile,
                   NativeFileSystemPermissionContext::UserAction::kOpen))
-      .WillOnce(testing::Return(allow_grant_));
+      .WillOnce(testing::Return(grant));
   EXPECT_CALL(permission_context_,
               GetWritePermissionGrant(
                   kTestOrigin, kTestPath, HandleType::kFile,
                   NativeFileSystemPermissionContext::UserAction::kOpen))
-      .WillOnce(testing::Return(allow_grant_));
+      .WillOnce(testing::Return(grant));
 
   blink::mojom::NativeFileSystemEntryPtr entry =
       manager_->CreateFileEntryFromPath(kBindingContext, kTestPath);

content/public/browser/native_file_system_permission_grant.h

@@ -34,6 +34,11 @@ class CONTENT_EXPORT NativeFileSystemPermissionGrant
 
   virtual PermissionStatus GetStatus() = 0;
 
+  // Returns the path this permission grant is associated with. Can return an
+  // empty FilePath if the permission grant isn't associated with any specific
+  // path.
+  virtual base::FilePath GetPath() = 0;
+
   // These values are persisted to logs. Entries should not be renumbered and
   // numeric values should never be reused.
   enum class PermissionRequestOutcome {