[FileAPI] Change how snapshots are captured by blink::File.

Instead of going through mojom::blink::FileUtilitiesHost and passing a path
from the renderer back to the browser, add a new method to mojom::blink::Blob
to do the same thing. Since the blob already knows what file a blob is
referencing, this both makes things slightly more secure (or at least gets
us closer to a place where we can get rid of things like
ChildProcessSecurityPolicyImpl::CanReadFile) and makes this
work in a couple more cases after a File is for example postMessaged to a
worker or frame in a different process as well.

Additionally this gets us closer to unifying what the renderer and browser
processes consider to be "the" snapshot state of any particular blob. In
a follow-up CL the browser side code will change to return its snapshot
state (if any), rather than always returning the current state on disk.

This is part of a series of changes to improve consistency surrounding how
snapshot state of files/blobs is implemented.

Bug: 844874
Change-Id: Ia05cfd21a3d78955ff70933995e93bbe59e3bb3b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1970301
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Cr-Commit-Position: refs/heads/master@{#726995}

content/renderer/service_worker/service_worker_subresource_loader_unittest.cc

@@ -77,6 +77,9 @@ class FakeBlob final : public blink::mojom::Blob {
   void ReadSideData(ReadSideDataCallback callback) override {
     std::move(callback).Run(side_data_);
   }
+  void CaptureSnapshot(CaptureSnapshotCallback callback) override {
+    std::move(callback).Run(body_.size(), base::nullopt);
+  }
   void GetInternalUUID(GetInternalUUIDCallback callback) override {
     NOTREACHED();
   }

storage/browser/blob/blob_impl.cc

@@ -11,6 +11,8 @@
 
 #include "base/bind.h"
 #include "base/containers/span.h"
+#include "base/files/file_util.h"
+#include "base/task/post_task.h"
 #include "mojo/public/cpp/bindings/remote.h"
 #include "net/base/io_buffer.h"
 #include "storage/browser/blob/blob_data_handle.h"
@@ -128,6 +130,7 @@ void BlobImpl::ReadSideData(ReadSideDataCallback callback) {
       [](BlobDataHandle handle, ReadSideDataCallback callback,
          BlobStatus status) {
         if (status != BlobStatus::DONE) {
+          DCHECK(BlobStatusIsError(status));
           std::move(callback).Run(base::nullopt);
           return;
         }
@@ -165,6 +168,68 @@ void BlobImpl::ReadSideData(ReadSideDataCallback callback) {
       *handle_, std::move(callback)));
 }
 
+void BlobImpl::CaptureSnapshot(CaptureSnapshotCallback callback) {
+  handle_->RunOnConstructionComplete(base::BindOnce(
+      [](base::WeakPtr<BlobImpl> blob_impl, CaptureSnapshotCallback callback,
+         BlobStatus status) {
+        if (!blob_impl) {
+          // No need to call callback, since blob_impl is only destroyed if the
+          // mojo pipe is disconnected.
+          return;
+        }
+
+        auto* handle = blob_impl->handle_.get();
+
+        if (status != BlobStatus::DONE) {
+          DCHECK(BlobStatusIsError(status));
+          std::move(callback).Run(0, base::nullopt);
+          return;
+        }
+
+        auto snapshot = handle->CreateSnapshot();
+        // Only blobs consisting of a single file can have a modification
+        // time.
+        const auto& items = snapshot->items();
+        if (items.size() != 1) {
+          std::move(callback).Run(handle->size(), base::nullopt);
+          return;
+        }
+
+        const auto& item = items[0];
+        if (item->type() != BlobDataItem::Type::kFile) {
+          std::move(callback).Run(handle->size(), base::nullopt);
+          return;
+        }
+
+        // TODO(mek): Use cached snapshot state, rather than always looking up
+        // the current size and modification time.
+
+        struct SizeAndTime {
+          uint64_t size;
+          base::Optional<base::Time> time;
+        };
+        base::PostTaskAndReplyWithResult(
+            FROM_HERE,
+            {base::ThreadPool(), base::MayBlock(),
+             base::TaskPriority::USER_VISIBLE},
+            base::BindOnce(
+                [](const base::FilePath& path) {
+                  base::File::Info info;
+                  if (!base::GetFileInfo(path, &info))
+                    return SizeAndTime{0, base::nullopt};
+                  return SizeAndTime{info.size, info.last_modified};
+                },
+                item->path()),
+            base::BindOnce(
+                [](CaptureSnapshotCallback callback,
+                   const SizeAndTime& result) {
+                  std::move(callback).Run(result.size, result.time);
+                },
+                std::move(callback)));
+      },
+      weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
+}
+
 void BlobImpl::GetInternalUUID(GetInternalUUIDCallback callback) {
   std::move(callback).Run(handle_->uuid());
 }

storage/browser/blob/blob_impl.h

@@ -43,6 +43,7 @@ class COMPONENT_EXPORT(STORAGE_BROWSER) BlobImpl
       mojo::ScopedDataPipeProducerHandle handle,
       mojo::PendingRemote<blink::mojom::BlobReaderClient> client) override;
   void ReadSideData(ReadSideDataCallback callback) override;
+  void CaptureSnapshot(CaptureSnapshotCallback callback) override;
   void GetInternalUUID(GetInternalUUIDCallback callback) override;
 
   // network::mojom::DataPipeGetter:

storage/browser/test/fake_blob.cc

@@ -41,6 +41,10 @@ void FakeBlob::ReadSideData(ReadSideDataCallback) {
   NOTREACHED();
 }
 
+void FakeBlob::CaptureSnapshot(CaptureSnapshotCallback) {
+  NOTREACHED();
+}
+
 void FakeBlob::GetInternalUUID(GetInternalUUIDCallback callback) {
   std::move(callback).Run(uuid_);
 }

storage/browser/test/fake_blob.h

@@ -27,6 +27,7 @@ class FakeBlob : public blink::mojom::Blob {
   void ReadAll(mojo::ScopedDataPipeProducerHandle,
                mojo::PendingRemote<blink::mojom::BlobReaderClient>) override;
   void ReadSideData(ReadSideDataCallback) override;
+  void CaptureSnapshot(CaptureSnapshotCallback) override;
   void GetInternalUUID(GetInternalUUIDCallback callback) override;
 
  private:

third_party/blink/public/mojom/blob/blob.mojom

@@ -5,6 +5,7 @@
 module blink.mojom;
 
 import "mojo/public/mojom/base/big_buffer.mojom";
+import "mojo/public/mojom/base/time.mojom";
 import "services/network/public/mojom/data_pipe_getter.mojom";
 import "services/network/public/mojom/http_request_headers.mojom";
 
@@ -53,6 +54,10 @@ interface Blob {
   // this blob through a blob URL.
   ReadSideData() => (mojo_base.mojom.BigBuffer? data);
 
+  // If this blob represents a single file on disk, this will capture the
+  // current snapshot state of that file.
+  [Sync] CaptureSnapshot() => (uint64 length, mojo_base.mojom.Time? modification_time);
+
   // This method is an implementation detail of the blob system. You should not
   // ever need to call it directly.
   // This returns the internal UUID of the blob, used by the blob system to

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

@@ -298,10 +298,13 @@ base::Time File::LastModifiedTime() const {
   if (HasValidSnapshotMetadata() && snapshot_modification_time_)
     return *snapshot_modification_time_;
 
+  uint64_t size;
   base::Optional<base::Time> modification_time;
-  if (HasBackingFile() && GetFileModificationTime(path_, modification_time) &&
-      modification_time)
+  if (HasBackingFile() &&
+      GetBlobDataHandle()->CaptureSnapshot(&size, &modification_time) &&
+      modification_time) {
     return *modification_time;
+  }
 
   // lastModified / lastModifiedDate getters should return the current time
   // when the last modification time isn't known.
@@ -327,10 +330,13 @@ uint64_t File::size() const {
 
   // FIXME: JavaScript cannot represent sizes as large as uint64_t, we need
   // to come up with an exception to throw if file size is not representable.
-  int64_t size;
-  if (!HasBackingFile() || !GetFileSize(path_, size))
+  uint64_t size;
+  base::Optional<base::Time> modification_time;
+  if (!HasBackingFile() ||
+      !GetBlobDataHandle()->CaptureSnapshot(&size, &modification_time)) {
     return 0;
-  return static_cast<uint64_t>(size);
+  }
+  return size;
 }
 
 Blob* File::slice(int64_t start,
@@ -370,14 +376,12 @@ void File::CaptureSnapshot(
   // If we fail to retrieve the size or modification time, probably due to that
   // the file has been deleted, 0 size is returned.
   FileMetadata metadata;
-  if (!HasBackingFile() || !GetFileMetadata(path_, metadata)) {
+  if (!HasBackingFile() || !GetBlobDataHandle()->CaptureSnapshot(
+                               &snapshot_size, &snapshot_modification_time)) {
     snapshot_size = 0;
     snapshot_modification_time = base::nullopt;
     return;
   }
-
-  snapshot_size = static_cast<uint64_t>(metadata.length);
-  snapshot_modification_time = metadata.modification_time;
 }
 
 void File::AppendTo(BlobData& blob_data) const {

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

@@ -4,53 +4,57 @@
 
 #include "third_party/blink/renderer/core/fileapi/file.h"
 
+#include "base/task/post_task.h"
 #include "mojo/public/cpp/bindings/receiver_set.h"
+#include "mojo/public/cpp/bindings/self_owned_receiver.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/blink/public/common/thread_safe_browser_interface_broker_proxy.h"
 #include "third_party/blink/public/mojom/file/file_utilities.mojom-blink.h"
 #include "third_party/blink/public/platform/platform.h"
+#include "third_party/blink/renderer/platform/blob/testing/fake_blob.h"
 #include "third_party/blink/renderer/platform/file_metadata.h"
 #include "third_party/blink/renderer/platform/heap/heap.h"
+#include "third_party/blink/renderer/platform/scheduler/public/post_cross_thread_task.h"
+#include "third_party/blink/renderer/platform/wtf/cross_thread_functional.h"
 
 namespace blink {
 
 namespace {
 
-class MockFileUtilitiesHost : public mojom::blink::FileUtilitiesHost {
+class MockBlob : public FakeBlob {
  public:
-  MockFileUtilitiesHost()
-      : broker_(*Platform::Current()->GetBrowserInterfaceBroker()) {
-    broker_.SetBinderForTesting(
-        FileUtilitiesHost::Name_,
-        WTF::BindRepeating(&MockFileUtilitiesHost::BindReceiver,
-                           WTF::Unretained(this)));
-    RebindFileUtilitiesForTesting();
+  static void Create(File* file, base::Time modified_time) {
+    mojo::PendingRemote<mojom::blink::Blob> remote;
+    PostCrossThreadTask(
+        *base::CreateSequencedTaskRunner({base::ThreadPool()}), FROM_HERE,
+        CrossThreadBindOnce(
+            [](const String& uuid,
+               mojo::PendingReceiver<mojom::blink::Blob> receiver,
+               base::Time modified_time) {
+              mojo::MakeSelfOwnedReceiver(
+                  std::make_unique<MockBlob>(uuid, modified_time),
+                  std::move(receiver));
+            },
+            file->Uuid(), remote.InitWithNewPipeAndPassReceiver(),
+            modified_time));
+    file->GetBlobDataHandle()->SetBlobRemoteForTesting(std::move(remote));
   }
 
-  ~MockFileUtilitiesHost() override {
-    broker_.SetBinderForTesting(FileUtilitiesHost::Name_, {});
-    RebindFileUtilitiesForTesting();
-  }
-
-  void SetFileInfoToBeReturned(const base::File::Info info) {
-    file_info_ = info;
-  }
+  MockBlob(const String& uuid, base::Time modified_time)
+      : FakeBlob(uuid), modified_time_(modified_time) {}
 
- private:
-  void BindReceiver(mojo::ScopedMessagePipeHandle handle) {
-    receivers_.Add(this,
-                   mojo::PendingReceiver<FileUtilitiesHost>(std::move(handle)));
+  void Clone(mojo::PendingReceiver<mojom::blink::Blob> receiver) override {
+    mojo::MakeSelfOwnedReceiver(
+        std::make_unique<MockBlob>(uuid_, modified_time_), std::move(receiver));
   }
 
-  // FileUtilitiesHost function:
-  void GetFileInfo(const base::FilePath& path,
-                   GetFileInfoCallback callback) override {
-    std::move(callback).Run(file_info_);
+  void CaptureSnapshot(CaptureSnapshotCallback callback) override {
+    std::move(callback).Run(
+        /*size=*/0, NullableTimeToOptionalTime(modified_time_));
   }
 
-  ThreadSafeBrowserInterfaceBrokerProxy& broker_;
-  mojo::ReceiverSet<FileUtilitiesHost> receivers_;
-  base::File::Info file_info_;
+ private:
+  base::Time modified_time_;
 };
 
 void ExpectTimestampIsNow(const File& file) {
@@ -65,12 +69,9 @@ void ExpectTimestampIsNow(const File& file) {
 }  // namespace
 
 TEST(FileTest, NativeFileWithoutTimestamp) {
-  MockFileUtilitiesHost host;
-  base::File::Info info;
-  info.last_modified = base::Time();
-  host.SetFileInfoToBeReturned(info);
-
   auto* const file = MakeGarbageCollected<File>("/native/path");
+  MockBlob::Create(file, base::Time());
+
   EXPECT_TRUE(file->HasBackingFile());
   EXPECT_EQ("/native/path", file->GetPath());
   EXPECT_TRUE(file->FileSystemURL().IsEmpty());
@@ -78,24 +79,18 @@ TEST(FileTest, NativeFileWithoutTimestamp) {
 }
 
 TEST(FileTest, NativeFileWithUnixEpochTimestamp) {
-  MockFileUtilitiesHost host;
-  base::File::Info info;
-  info.last_modified = base::Time::UnixEpoch();
-  host.SetFileInfoToBeReturned(info);
-
   auto* const file = MakeGarbageCollected<File>("/native/path");
+  MockBlob::Create(file, base::Time::UnixEpoch());
+
   EXPECT_TRUE(file->HasBackingFile());
   EXPECT_EQ(0, file->lastModified());
   EXPECT_EQ(base::Time::UnixEpoch(), file->LastModifiedTime());
 }
 
 TEST(FileTest, NativeFileWithApocalypseTimestamp) {
-  MockFileUtilitiesHost host;
-  base::File::Info info;
-  info.last_modified = base::Time::Max();
-  host.SetFileInfoToBeReturned(info);
-
   auto* const file = MakeGarbageCollected<File>("/native/path");
+  MockBlob::Create(file, base::Time::Max());
+
   EXPECT_TRUE(file->HasBackingFile());
 
   EXPECT_EQ((base::Time::Max() - base::Time::UnixEpoch()).InMilliseconds(),

third_party/blink/renderer/platform/blob/blob_data.cc

@@ -415,6 +415,16 @@ void BlobDataHandle::ReadRange(
   blob_remote_ = blob.Unbind();
 }
 
+bool BlobDataHandle::CaptureSnapshot(
+    uint64_t* snapshot_size,
+    base::Optional<base::Time>* snapshot_modification_time) {
+  // This method operates on a cloned blob remote; this lets us avoid holding
+  // the |blob_remote_mutex_| locked during the duration of the (synchronous)
+  // CaptureSnapshot call.
+  mojo::Remote<mojom::blink::Blob> remote(CloneBlobRemote());
+  return remote->CaptureSnapshot(snapshot_size, snapshot_modification_time);
+}
+
 // static
 mojom::blink::BlobRegistry* BlobDataHandle::GetBlobRegistry() {
   return GetThreadSpecificRegistry();

third_party/blink/renderer/platform/blob/blob_data.h

@@ -219,6 +219,16 @@ class PLATFORM_EXPORT BlobDataHandle
                  mojo::ScopedDataPipeProducerHandle,
                  mojo::PendingRemote<mojom::blink::BlobReaderClient>);
 
+  // This does synchronous IPC, and possibly synchronous file operations. Think
+  // twice before calling this function.
+  bool CaptureSnapshot(uint64_t* snapshot_size,
+                       base::Optional<base::Time>* snapshot_modification_time);
+
+  void SetBlobRemoteForTesting(mojo::PendingRemote<mojom::blink::Blob> remote) {
+    MutexLocker locker(blob_remote_mutex_);
+    blob_remote_ = std::move(remote);
+  }
+
   static mojom::blink::BlobRegistry* GetBlobRegistry();
   static void SetBlobRegistryForTesting(mojom::blink::BlobRegistry*);
 

third_party/blink/renderer/platform/blob/testing/fake_blob.cc

@@ -80,6 +80,10 @@ void FakeBlob::ReadSideData(ReadSideDataCallback callback) {
   NOTREACHED();
 }
 
+void FakeBlob::CaptureSnapshot(CaptureSnapshotCallback callback) {
+  std::move(callback).Run(body_.length(), base::nullopt);
+}
+
 void FakeBlob::GetInternalUUID(GetInternalUUIDCallback callback) {
   std::move(callback).Run(uuid_);
 }

third_party/blink/renderer/platform/blob/testing/fake_blob.h

@@ -32,8 +32,10 @@ class FakeBlob : public mojom::blink::Blob {
   void ReadAll(mojo::ScopedDataPipeProducerHandle,
                mojo::PendingRemote<mojom::blink::BlobReaderClient>) override;
   void ReadSideData(ReadSideDataCallback) override;
+  void CaptureSnapshot(CaptureSnapshotCallback) override;
   void GetInternalUUID(GetInternalUUIDCallback) override;
- private:
+
+ protected:
   String uuid_;
   String body_;
   State* state_;