Don't pass blobs over IPC as UUIDs in blink FileWriter implementation.

Instead pass the mojo::Blob remote over IPC and resolve to
a UUID in the browser process. This is both more reliable
(there is no longer any way the blob uuid can become invalid
before the browser process has a chance to look up the uuid)
and reduces reliance on blob registration being sync.

Bug: 740744
Change-Id: Ie17bd48001ee888314bd597ab8806ab7b699a866
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4582370
Reviewed-by: Austin Sullivan <asully@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1153973}

content/browser/file_system/file_system_manager_impl.cc

@@ -692,7 +692,7 @@ void FileSystemManagerImpl::ContinueReadDirectorySync(
 
 void FileSystemManagerImpl::Write(
     const GURL& file_path,
-    const std::string& blob_uuid,
+    mojo::PendingRemote<blink::mojom::Blob> blob,
     int64_t position,
     mojo::PendingReceiver<blink::mojom::FileSystemCancellableOperation>
         op_receiver,
@@ -713,26 +713,40 @@ void FileSystemManagerImpl::Write(
       // security_policy_ is a singleton so refcounting is unnecessary
       base::BindOnce(&ChildProcessSecurityPolicyImpl::CanWriteFileSystemFile,
                      base::Unretained(security_policy_), process_id_, url),
-      base::BindOnce(&FileSystemManagerImpl::ContinueWrite,
-                     weak_factory_.GetWeakPtr(), url, blob_uuid, position,
-                     std::move(op_receiver), std::move(listener)));
+      base::BindOnce(
+          &FileSystemManagerImpl::ResolveBlobForWrite,
+          weak_factory_.GetWeakPtr(), std::move(blob),
+          base::BindOnce(&FileSystemManagerImpl::ContinueWrite,
+                         weak_factory_.GetWeakPtr(), url, position,
+                         std::move(op_receiver), std::move(listener))));
+}
+
+void FileSystemManagerImpl::ResolveBlobForWrite(
+    mojo::PendingRemote<blink::mojom::Blob> blob,
+    base::OnceCallback<void(std::unique_ptr<storage::BlobDataHandle>)> callback,
+    bool security_check_success) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  if (!security_check_success) {
+    std::move(callback).Run(nullptr);
+    return;
+  }
+
+  blob_storage_context_->context()->GetBlobDataFromBlobRemote(
+      std::move(blob), std::move(callback));
 }
 
 void FileSystemManagerImpl::ContinueWrite(
     const storage::FileSystemURL& url,
-    const std::string& blob_uuid,
     int64_t position,
     mojo::PendingReceiver<blink::mojom::FileSystemCancellableOperation>
         op_receiver,
     mojo::Remote<blink::mojom::FileSystemOperationListener> listener,
-    bool security_check_success) {
+    std::unique_ptr<storage::BlobDataHandle> blob) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  if (!security_check_success) {
+  if (!blob) {
     listener->ErrorOccurred(base::File::FILE_ERROR_SECURITY);
     return;
   }
-  std::unique_ptr<storage::BlobDataHandle> blob =
-      blob_storage_context_->context()->GetBlobDataFromUUID(blob_uuid);
 
   OperationListenerID listener_id = AddOpListener(std::move(listener));
 
@@ -753,10 +767,11 @@ void FileSystemManagerImpl::ContinueWrite(
       std::move(op_receiver));
 }
 
-void FileSystemManagerImpl::WriteSync(const GURL& file_path,
-                                      const std::string& blob_uuid,
-                                      int64_t position,
-                                      WriteSyncCallback callback) {
+void FileSystemManagerImpl::WriteSync(
+    const GURL& file_path,
+    mojo::PendingRemote<blink::mojom::Blob> blob,
+    int64_t position,
+    WriteSyncCallback callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   FileSystemURL url(
       context_->CrackURL(file_path, receivers_.current_context()));
@@ -770,23 +785,23 @@ void FileSystemManagerImpl::WriteSync(const GURL& file_path,
       // security_policy_ is a singleton so refcounting is unnecessary
       base::BindOnce(&ChildProcessSecurityPolicyImpl::CanWriteFileSystemFile,
                      base::Unretained(security_policy_), process_id_, url),
-      base::BindOnce(&FileSystemManagerImpl::ContinueWriteSync,
-                     weak_factory_.GetWeakPtr(), url, blob_uuid, position,
-                     std::move(callback)));
+      base::BindOnce(&FileSystemManagerImpl::ResolveBlobForWrite,
+                     weak_factory_.GetWeakPtr(), std::move(blob),
+                     base::BindOnce(&FileSystemManagerImpl::ContinueWriteSync,
+                                    weak_factory_.GetWeakPtr(), url, position,
+                                    std::move(callback))));
 }
 
-void FileSystemManagerImpl::ContinueWriteSync(const storage::FileSystemURL& url,
-                                              const std::string& blob_uuid,
-                                              int64_t position,
-                                              WriteSyncCallback callback,
-                                              bool security_check_success) {
+void FileSystemManagerImpl::ContinueWriteSync(
+    const storage::FileSystemURL& url,
+    int64_t position,
+    WriteSyncCallback callback,
+    std::unique_ptr<storage::BlobDataHandle> blob) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  if (!security_check_success) {
+  if (!blob) {
     std::move(callback).Run(0, base::File::FILE_ERROR_SECURITY);
     return;
   }
-  std::unique_ptr<storage::BlobDataHandle> blob =
-      blob_storage_context_->context()->GetBlobDataFromUUID(blob_uuid);
 
   storage::FileSystemOperationRunner* fs_op_runner = operation_runner();
   if (!fs_op_runner) {

content/browser/file_system/file_system_manager_impl.h

@@ -104,14 +104,14 @@ class CONTENT_EXPORT FileSystemManagerImpl
   void ReadDirectorySync(const GURL& path,
                          ReadDirectorySyncCallback callback) override;
   void Write(const GURL& file_path,
-             const std::string& blob_uuid,
+             mojo::PendingRemote<blink::mojom::Blob> blob,
              int64_t position,
              mojo::PendingReceiver<blink::mojom::FileSystemCancellableOperation>
                  op_receiver,
              mojo::PendingRemote<blink::mojom::FileSystemOperationListener>
                  pending_listener) override;
   void WriteSync(const GURL& file_path,
-                 const std::string& blob_uuid,
+                 mojo::PendingRemote<blink::mojom::Blob> blob,
                  int64_t position,
                  WriteSyncCallback callback) override;
   void Truncate(
@@ -182,19 +182,22 @@ class CONTENT_EXPORT FileSystemManagerImpl
   void ContinueReadDirectorySync(const storage::FileSystemURL& url,
                                  ReadDirectorySyncCallback callback,
                                  bool security_check_success);
+  void ResolveBlobForWrite(
+      mojo::PendingRemote<blink::mojom::Blob> blob,
+      base::OnceCallback<void(std::unique_ptr<storage::BlobDataHandle>)>
+          callback,
+      bool security_check_success);
   void ContinueWrite(
       const storage::FileSystemURL& url,
-      const std::string& blob_uuid,
       int64_t position,
       mojo::PendingReceiver<blink::mojom::FileSystemCancellableOperation>
           op_receiver,
       mojo::Remote<blink::mojom::FileSystemOperationListener> listener,
-      bool security_check_success);
+      std::unique_ptr<storage::BlobDataHandle> blob);
   void ContinueWriteSync(const storage::FileSystemURL& url,
-                         const std::string& blob_uuid,
                          int64_t position,
                          WriteSyncCallback callback,
-                         bool security_check_success);
+                         std::unique_ptr<storage::BlobDataHandle> blob);
   void ContinueTruncate(
       const storage::FileSystemURL& url,
       int64_t length,

content/public/test/browser_test_utils.cc

@@ -3575,9 +3575,9 @@ void PwnMessageHelper::FileSystemWrite(RenderProcessHost* process,
       &waiter, listener.InitWithNewPipeAndPassReceiver());
   mojo::Remote<blink::mojom::FileSystemCancellableOperation> op;
 
-  file_system_manager->Write(file_path, blob_uuid, position,
-                             op.BindNewPipeAndPassReceiver(),
-                             std::move(listener));
+  file_system_manager->Write(
+      file_path, process->GetBrowserContext()->GetBlobRemote(blob_uuid),
+      position, op.BindNewPipeAndPassReceiver(), std::move(listener));
   waiter.WaitForOperationToFinish();
 }
 

third_party/blink/public/mojom/filesystem/file_system.mojom

@@ -12,6 +12,7 @@ import "mojo/public/mojom/base/file_path.mojom";
 import "mojo/public/mojom/base/file_info.mojom";
 import "mojo/public/mojom/base/time.mojom";
 import "third_party/blink/public/mojom/blob/serialized_blob.mojom";
+import "third_party/blink/public/mojom/blob/blob.mojom";
 
 enum FileSystemType {
   kTemporary,
@@ -149,17 +150,17 @@ interface FileSystemManager {
     (array<filesystem.mojom.DirectoryEntry> entries,
      mojo_base.mojom.FileError error_code);
 
-  // Write data (indicated by |blob_uuid|) to the given file at |file_path|,
+  // Write data (indicated by |blob|) to the given file at |file_path|,
   // at |position|. Calls DidWrite on |listener| to provide progress updates on
   // the write, and |ErrorOccurred| if the operation fails. The operation can
   // also be cancelled using the remote associated with |op_receiver|.
   Write(url.mojom.Url file_path,
-        string blob_uuid,
+        pending_remote<blink.mojom.Blob> blob,
         int64 position,
         pending_receiver<FileSystemCancellableOperation> op_receiver,
         pending_remote<FileSystemOperationListener> listener);
   [Sync]
-  WriteSync(url.mojom.Url file_path, string blob_uuid, int64 position) =>
+  WriteSync(url.mojom.Url file_path, pending_remote<blink.mojom.Blob> blob, int64 position) =>
       (int64 byte_count, mojo_base.mojom.FileError error_code);
 
   // Changes the file length of the file at |file_path| to the |length|

third_party/blink/renderer/core/fileapi/file_test.cc

@@ -114,14 +114,14 @@ class MockFileSystemManager : public mojom::blink::FileSystemManager {
   void ReadDirectorySync(const KURL& path,
                          ReadDirectorySyncCallback callback) override {}
   void Write(const KURL& file_path,
-             const String& blob_uuid,
+             mojo::PendingRemote<mojom::blink::Blob> blob,
              int64_t position,
              mojo::PendingReceiver<mojom::blink::FileSystemCancellableOperation>
                  op_receiver,
              mojo::PendingRemote<mojom::blink::FileSystemOperationListener>
                  pending_listener) override {}
   void WriteSync(const KURL& file_path,
-                 const String& blob_uuid,
+                 mojo::PendingRemote<mojom::blink::Blob> blob,
                  int64_t position,
                  WriteSyncCallback callback) override {}
   void Truncate(

third_party/blink/renderer/modules/filesystem/file_system_dispatcher.cc

@@ -15,6 +15,7 @@
 #include "third_party/blink/public/platform/platform.h"
 #include "third_party/blink/public/platform/task_type.h"
 #include "third_party/blink/renderer/core/execution_context/execution_context.h"
+#include "third_party/blink/renderer/core/fileapi/blob.h"
 #include "third_party/blink/renderer/platform/wtf/functional.h"
 #include "third_party/blink/renderer/platform/wtf/text/wtf_string.h"
 
@@ -366,7 +367,7 @@ void FileSystemDispatcher::TruncateSync(const KURL& path,
 }
 
 void FileSystemDispatcher::Write(const KURL& path,
-                                 const String& blob_id,
+                                 const Blob& blob,
                                  int64_t offset,
                                  int* request_id_out,
                                  const WriteCallback& success_callback,
@@ -397,21 +398,21 @@ void FileSystemDispatcher::Write(const KURL& path,
                                       std::move(error_callback), operation_id)),
                     std::move(receiver), task_runner);
 
-  GetFileSystemManager().Write(path, blob_id, offset, std::move(op_receiver),
-                               std::move(listener));
+  GetFileSystemManager().Write(path, blob.AsMojoBlob(), offset,
+                               std::move(op_receiver), std::move(listener));
 
   if (request_id_out)
     *request_id_out = operation_id;
 }
 
 void FileSystemDispatcher::WriteSync(const KURL& path,
-                                     const String& blob_id,
+                                     const Blob& blob,
                                      int64_t offset,
                                      const WriteCallback& success_callback,
                                      StatusCallback error_callback) {
   int64_t byte_count;
   base::File::Error error_code = base::File::FILE_ERROR_FAILED;
-  GetFileSystemManager().WriteSync(path, blob_id, offset, &byte_count,
+  GetFileSystemManager().WriteSync(path, blob.AsMojoBlob(), offset, &byte_count,
                                    &error_code);
   if (error_code == base::File::FILE_OK)
     std::move(success_callback).Run(byte_count, /*complete=*/true);

third_party/blink/renderer/modules/filesystem/file_system_dispatcher.h

@@ -23,8 +23,9 @@ class String;
 
 namespace blink {
 
-class KURL;
+class Blob;
 class ExecutionContext;
+class KURL;
 class SecurityOrigin;
 
 // Sends messages via mojo to the blink::mojom::FileSystemManager service
@@ -124,13 +125,13 @@ class FileSystemDispatcher : public GarbageCollected<FileSystemDispatcher>,
   void TruncateSync(const KURL& path, int64_t offset, StatusCallback callback);
 
   void Write(const KURL& path,
-             const String& blob_id,
+             const Blob& blob,
              int64_t offset,
              int* request_id_out,
              const WriteCallback& success_callback,
              StatusCallback error_callback);
   void WriteSync(const KURL& path,
-                 const String& blob_id,
+                 const Blob& blob,
                  int64_t offset,
                  const WriteCallback& success_callback,
                  StatusCallback error_callback);

third_party/blink/renderer/modules/filesystem/file_writer.cc

@@ -231,12 +231,10 @@ void FileWriter::DoTruncate(const KURL& path, int64_t offset) {
           WTF::BindOnce(&FileWriter::DidFinish, WrapWeakPersistent(this)));
 }
 
-void FileWriter::DoWrite(const KURL& path,
-                         const String& blob_id,
-                         int64_t offset) {
+void FileWriter::DoWrite(const KURL& path, const Blob& blob, int64_t offset) {
   FileSystemDispatcher::From(GetExecutionContext())
       .Write(
-          path, blob_id, offset, &request_id_,
+          path, blob, offset, &request_id_,
           WTF::BindRepeating(&FileWriter::DidWrite, WrapWeakPersistent(this)),
           WTF::BindOnce(&FileWriter::DidFinish, WrapWeakPersistent(this)));
 }
@@ -263,7 +261,7 @@ void FileWriter::DoOperation(Operation operation) {
       DCHECK_EQ(kMaxTruncateLength, truncate_length_);
       DCHECK(blob_being_written_.Get());
       DCHECK_EQ(kWriting, ready_state_);
-      Write(position(), blob_being_written_->Uuid());
+      Write(position(), *blob_being_written_);
       break;
     case kOperationTruncate:
       DCHECK_EQ(kOperationNone, operation_in_progress_);

third_party/blink/renderer/modules/filesystem/file_writer.h

@@ -74,9 +74,7 @@ class FileWriter final : public EventTargetWithInlineData,
   void DidTruncateImpl() override;
   void DidFailImpl(base::File::Error error) override;
   void DoTruncate(const KURL& path, int64_t offset) override;
-  void DoWrite(const KURL& path,
-               const String& blob_id,
-               int64_t offset) override;
+  void DoWrite(const KURL& path, const Blob& blob, int64_t offset) override;
   void DoCancel() override;
 
   // ExecutionContextLifecycleObserver

third_party/blink/renderer/modules/filesystem/file_writer_base.cc

@@ -67,11 +67,11 @@ void FileWriterBase::Truncate(int64_t length) {
   DoTruncate(path_, length);
 }
 
-void FileWriterBase::Write(int64_t position, const String& id) {
+void FileWriterBase::Write(int64_t position, const Blob& blob) {
   DCHECK_EQ(kOperationNone, operation_);
   DCHECK_EQ(kCancelNotInProgress, cancel_state_);
   operation_ = kOperationWrite;
-  DoWrite(path_, id, position);
+  DoWrite(path_, blob, position);
 }
 
 // When we cancel a write/truncate, we always get back the result of the write

third_party/blink/renderer/modules/filesystem/file_writer_base.h

@@ -38,6 +38,8 @@
 
 namespace blink {
 
+class Blob;
+
 class MODULES_EXPORT FileWriterBase : public GarbageCollectedMixin {
  public:
   virtual ~FileWriterBase();
@@ -49,7 +51,7 @@ class MODULES_EXPORT FileWriterBase : public GarbageCollectedMixin {
   void Trace(Visitor* visitor) const override {}
 
   virtual void Truncate(int64_t length);
-  virtual void Write(int64_t position, const String& id);
+  virtual void Write(int64_t position, const Blob& blob);
   virtual void Cancel();
 
  protected:
@@ -71,9 +73,7 @@ class MODULES_EXPORT FileWriterBase : public GarbageCollectedMixin {
   // the requested operation, and they must call the appropriate DidSomething
   // method upon completion and as progress is made in the Write case.
   virtual void DoTruncate(const KURL& path, int64_t offset) = 0;
-  virtual void DoWrite(const KURL& path,
-                       const String& blob_id,
-                       int64_t offset) = 0;
+  virtual void DoWrite(const KURL& path, const Blob& blob, int64_t offset) = 0;
   virtual void DoCancel() = 0;
 
   // These are conditionally called by the Did* methods.

third_party/blink/renderer/modules/filesystem/file_writer_sync.cc

@@ -43,7 +43,7 @@ void FileWriterSync::write(Blob* data, ExceptionState& exception_state) {
   DCHECK(complete_);
 
   PrepareForWrite();
-  Write(position(), data->Uuid());
+  Write(position(), *data);
   DCHECK(complete_);
   if (error_) {
     file_error::ThrowDOMException(exception_state, error_);
@@ -107,13 +107,13 @@ void FileWriterSync::DoTruncate(const KURL& path, int64_t offset) {
 }
 
 void FileWriterSync::DoWrite(const KURL& path,
-                             const String& blob_id,
+                             const Blob& blob,
                              int64_t offset) {
   if (!GetExecutionContext())
     return;
   FileSystemDispatcher::From(GetExecutionContext())
       .WriteSync(
-          path, blob_id, offset,
+          path, blob, offset,
           WTF::BindRepeating(&FileWriterSync::DidWrite,
                              WrapWeakPersistent(this)),
           WTF::BindOnce(&FileWriterSync::DidFinish, WrapWeakPersistent(this)));

third_party/blink/renderer/modules/filesystem/file_writer_sync.h

@@ -61,9 +61,7 @@ class FileWriterSync final : public ScriptWrappable,
   void DidTruncateImpl() override;
   void DidFailImpl(base::File::Error error) override;
   void DoTruncate(const KURL& path, int64_t offset) override;
-  void DoWrite(const KURL& path,
-               const String& blob_id,
-               int64_t offset) override;
+  void DoWrite(const KURL& path, const Blob& blob, int64_t offset) override;
   void DoCancel() override;
 
  private: