[FileSystem] Browser side implementation of future FileWriter API.

Not hooked up yet, but implements the mojo interface in terms of the old
implementation.

Bug: 872460
Change-Id: Id33e4a8f732f4bda0a2ad866e85e210962b45a05
Reviewed-on: https://chromium-review.googlesource.com/1171518Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583864}

storage/browser/BUILD.gn

@@ -112,6 +112,8 @@ component("browser") {
     "fileapi/file_system_usage_cache.h",
     "fileapi/file_writer_delegate.cc",
     "fileapi/file_writer_delegate.h",
+    "fileapi/file_writer_impl.cc",
+    "fileapi/file_writer_impl.h",
     "fileapi/isolated_context.cc",
     "fileapi/isolated_context.h",
     "fileapi/isolated_file_system_backend.cc",
@@ -275,6 +277,7 @@ source_set("unittests") {
     "fileapi/file_system_url_unittest.cc",
     "fileapi/file_system_usage_cache_unittest.cc",
     "fileapi/file_writer_delegate_unittest.cc",
+    "fileapi/file_writer_impl_unittest.cc",
     "fileapi/isolated_context_unittest.cc",
     "fileapi/local_file_stream_reader_unittest.cc",
     "fileapi/local_file_stream_writer_unittest.cc",

storage/browser/blob/blob_storage_context.cc

@@ -25,6 +25,7 @@
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/trace_event/memory_dump_manager.h"
 #include "base/trace_event/trace_event.h"
+#include "mojo/public/cpp/bindings/callback_helpers.h"
 #include "storage/browser/blob/blob_data_builder.h"
 #include "storage/browser/blob/blob_data_item.h"
 #include "storage/browser/blob/blob_data_snapshot.h"
@@ -75,6 +76,26 @@ std::unique_ptr<BlobDataHandle> BlobStorageContext::GetBlobDataFromPublicURL(
   return CreateHandle(uuid, entry);
 }
 
+void BlobStorageContext::GetBlobDataFromBlobPtr(
+    blink::mojom::BlobPtr blob,
+    base::OnceCallback<void(std::unique_ptr<BlobDataHandle>)> callback) {
+  DCHECK(blob);
+  blink::mojom::Blob* raw_blob = blob.get();
+  raw_blob->GetInternalUUID(mojo::WrapCallbackWithDefaultInvokeIfNotRun(
+      base::BindOnce(
+          [](blink::mojom::BlobPtr, base::WeakPtr<BlobStorageContext> context,
+             base::OnceCallback<void(std::unique_ptr<BlobDataHandle>)> callback,
+             const std::string& uuid) {
+            if (!context || uuid.empty()) {
+              std::move(callback).Run(nullptr);
+              return;
+            }
+            std::move(callback).Run(context->GetBlobDataFromUUID(uuid));
+          },
+          std::move(blob), AsWeakPtr(), std::move(callback)),
+      ""));
+}
+
 std::unique_ptr<BlobDataHandle> BlobStorageContext::AddFinishedBlob(
     std::unique_ptr<BlobDataBuilder> external_builder) {
   TRACE_EVENT0("Blob", "Context::AddFinishedBlob");

storage/browser/blob/blob_storage_context.h

@@ -25,6 +25,7 @@
 #include "storage/browser/blob/blob_storage_registry.h"
 #include "storage/browser/storage_browser_export.h"
 #include "storage/common/blob_storage/blob_storage_constants.h"
+#include "third_party/blink/public/mojom/blob/blob.mojom.h"
 
 class GURL;
 
@@ -57,8 +58,15 @@ class STORAGE_EXPORT BlobStorageContext
                      scoped_refptr<base::TaskRunner> file_runner);
   ~BlobStorageContext() override;
 
+  // The following three methods all lookup a BlobDataHandle based on some
+  // input. If no blob matching the input exists these methods return null.
   std::unique_ptr<BlobDataHandle> GetBlobDataFromUUID(const std::string& uuid);
   std::unique_ptr<BlobDataHandle> GetBlobDataFromPublicURL(const GURL& url);
+  // If this BlobStorageContext is deleted before this method finishes, the
+  // callback will still be called with null.
+  void GetBlobDataFromBlobPtr(
+      blink::mojom::BlobPtr blob,
+      base::OnceCallback<void(std::unique_ptr<BlobDataHandle>)> callback);
 
   // Always returns a handle to a blob. Use BlobStatus::GetBlobStatus() and
   // BlobStatus::RunOnConstructionComplete(callback) to determine construction

storage/browser/fileapi/file_writer_impl.cc

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "storage/browser/fileapi/file_writer_impl.h"
+
+#include "base/callback_helpers.h"
+#include "storage/browser/blob/blob_data_handle.h"
+#include "storage/browser/blob/blob_storage_context.h"
+
+namespace storage {
+
+FileWriterImpl::FileWriterImpl(
+    FileSystemURL url,
+    std::unique_ptr<FileSystemOperationRunner> operation_runner,
+    base::WeakPtr<BlobStorageContext> blob_context)
+    : operation_runner_(std::move(operation_runner)),
+      blob_context_(std::move(blob_context)),
+      url_(std::move(url)) {
+  DCHECK(url_.is_valid());
+}
+
+FileWriterImpl::~FileWriterImpl() = default;
+
+void FileWriterImpl::Write(uint64_t position,
+                           blink::mojom::BlobPtr blob,
+                           WriteCallback callback) {
+  blob_context_->GetBlobDataFromBlobPtr(
+      std::move(blob),
+      base::BindOnce(&FileWriterImpl::DoWrite, base::Unretained(this),
+                     std::move(callback), position));
+}
+
+void FileWriterImpl::Truncate(uint64_t length, TruncateCallback callback) {
+  operation_runner_->Truncate(
+      url_, length,
+      base::BindRepeating(
+          &FileWriterImpl::DidTruncate, base::Unretained(this),
+          base::AdaptCallbackForRepeating(std::move(callback))));
+}
+
+void FileWriterImpl::DoWrite(WriteCallback callback,
+                             uint64_t position,
+                             std::unique_ptr<BlobDataHandle> blob) {
+  if (!blob) {
+    std::move(callback).Run(base::File::FILE_ERROR_FAILED, 0);
+    return;
+  }
+
+  // FileSystemOperationRunner assumes that positions passed to Write are always
+  // valid, and will NOTREACHED() if that is not the case, so first check the
+  // size of the file to make sure the position passed in from the renderer is
+  // in fact valid.
+  // Of course the file could still change between checking its size and the
+  // write operation being started, but this is at least a lot better than the
+  // old implementation where the renderer only checks against how big it thinks
+  // the file currently is.
+  operation_runner_->GetMetadata(
+      url_, FileSystemOperation::GET_METADATA_FIELD_SIZE,
+      base::BindRepeating(&FileWriterImpl::DoWriteWithFileInfo,
+                          base::Unretained(this),
+                          base::AdaptCallbackForRepeating(std::move(callback)),
+                          position, base::Passed(std::move(blob))));
+}
+
+void FileWriterImpl::DoWriteWithFileInfo(WriteCallback callback,
+                                         uint64_t position,
+                                         std::unique_ptr<BlobDataHandle> blob,
+                                         base::File::Error result,
+                                         const base::File::Info& file_info) {
+  if (file_info.size < 0 || position > static_cast<uint64_t>(file_info.size)) {
+    std::move(callback).Run(base::File::FILE_ERROR_FAILED, 0);
+    return;
+  }
+  operation_runner_->Write(
+      url_, std::move(blob), position,
+      base::BindRepeating(&FileWriterImpl::DidWrite, base::Unretained(this),
+                          base::AdaptCallbackForRepeating(std::move(callback)),
+                          base::Owned(new WriteState())));
+}
+
+void FileWriterImpl::DidWrite(WriteCallback callback,
+                              WriteState* state,
+                              base::File::Error result,
+                              int64_t bytes,
+                              bool complete) {
+  DCHECK(state);
+  state->bytes_written += bytes;
+  if (complete)
+    std::move(callback).Run(result, state->bytes_written);
+}
+
+void FileWriterImpl::DidTruncate(TruncateCallback callback,
+                                 base::File::Error result) {
+  std::move(callback).Run(result);
+}
+
+}  // namespace storage

storage/browser/fileapi/file_writer_impl.h

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef STORAGE_BROWSER_FILEAPI_FILE_WRITER_IMPL_H_
+#define STORAGE_BROWSER_FILEAPI_FILE_WRITER_IMPL_H_
+
+#include "storage/browser/fileapi/file_system_operation_runner.h"
+#include "storage/browser/fileapi/file_system_url.h"
+#include "storage/browser/storage_browser_export.h"
+#include "third_party/blink/public/mojom/filesystem/file_writer.mojom.h"
+
+namespace storage {
+
+// This class itself is stateless, but it uses the passed in
+// FileSystemOperationRunner and BlobStorageContext so all methods should be
+// called on the sequence those instances live on. In chromium that means all
+// usage has to be on the IO thread.
+class STORAGE_EXPORT FileWriterImpl : public blink::mojom::FileWriter {
+ public:
+  FileWriterImpl(FileSystemURL url,
+                 std::unique_ptr<FileSystemOperationRunner> operation_runner,
+                 base::WeakPtr<BlobStorageContext> blob_context);
+  ~FileWriterImpl() override;
+
+  void Write(uint64_t position,
+             blink::mojom::BlobPtr blob,
+             WriteCallback callback) override;
+  void Truncate(uint64_t length, TruncateCallback callback) override;
+
+ private:
+  void DoWrite(WriteCallback callback,
+               uint64_t position,
+               std::unique_ptr<BlobDataHandle> blob);
+  void DoWriteWithFileInfo(WriteCallback callback,
+                           uint64_t position,
+                           std::unique_ptr<BlobDataHandle> blob,
+                           base::File::Error result,
+                           const base::File::Info& file_info);
+
+  struct WriteState {
+    uint64_t bytes_written = 0;
+  };
+  void DidWrite(WriteCallback callback,
+                WriteState* state,
+                base::File::Error result,
+                int64_t bytes,
+                bool complete);
+  void DidTruncate(TruncateCallback callback, base::File::Error result);
+
+  const std::unique_ptr<FileSystemOperationRunner> operation_runner_;
+  const base::WeakPtr<BlobStorageContext> blob_context_;
+  const FileSystemURL url_;
+};
+
+}  // namespace storage
+
+#endif  // STORAGE_BROWSER_FILEAPI_FILE_WRITER_IMPL_H_

storage/browser/fileapi/file_writer_impl_unittest.cc

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "storage/browser/fileapi/file_writer_impl.h"
+
+#include <limits>
+#include "base/files/scoped_temp_dir.h"
+#include "base/guid.h"
+#include "base/test/bind_test_util.h"
+#include "base/test/scoped_task_environment.h"
+#include "net/base/io_buffer.h"
+#include "net/base/net_errors.h"
+#include "net/base/test_completion_callback.h"
+#include "storage/browser/blob/blob_data_builder.h"
+#include "storage/browser/blob/blob_impl.h"
+#include "storage/browser/blob/blob_storage_context.h"
+#include "storage/browser/fileapi/file_stream_reader.h"
+#include "storage/browser/test/async_file_test_helper.h"
+#include "storage/browser/test/test_file_system_context.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using content::AsyncFileTestHelper;
+using content::CreateFileSystemContextForTesting;
+
+namespace storage {
+
+class FileWriterImplTest : public testing::Test {
+ public:
+  FileWriterImplTest()
+      : scoped_task_environment_(
+            base::test::ScopedTaskEnvironment::MainThreadType::IO) {}
+
+  void SetUp() override {
+    ASSERT_TRUE(dir_.CreateUniqueTempDir());
+    file_system_context_ = CreateFileSystemContextForTesting(
+        /*quota_manager_proxy=*/nullptr, dir_.GetPath());
+
+    test_url_ = file_system_context_->CreateCrackedFileSystemURL(
+        GURL("http://example.com"), kFileSystemTypeTest,
+        base::FilePath::FromUTF8Unsafe("test"));
+
+    ASSERT_EQ(base::File::FILE_OK, AsyncFileTestHelper::CreateFile(
+                                       file_system_context_.get(), test_url_));
+
+    blob_context_ = std::make_unique<BlobStorageContext>();
+
+    writer_ = std::make_unique<FileWriterImpl>(
+        test_url_, file_system_context_->CreateFileSystemOperationRunner(),
+        blob_context_->AsWeakPtr());
+  }
+
+  blink::mojom::BlobPtr CreateBlob(const std::string& contents) {
+    auto builder =
+        std::make_unique<storage::BlobDataBuilder>(base::GenerateGUID());
+    builder->AppendData(contents);
+    auto handle = blob_context_->AddFinishedBlob(std::move(builder));
+    blink::mojom::BlobPtr result;
+    BlobImpl::Create(std::move(handle), MakeRequest(&result));
+    return result;
+  }
+
+  std::string ReadFile(const FileSystemURL& url) {
+    std::unique_ptr<FileStreamReader> reader =
+        file_system_context_->CreateFileStreamReader(
+            url, 0, std::numeric_limits<int64_t>::max(), base::Time());
+    net::TestCompletionCallback callback;
+    std::string result;
+    while (true) {
+      auto buf = base::MakeRefCounted<net::IOBufferWithSize>(4096);
+      int rv = reader->Read(buf.get(), buf->size(), callback.callback());
+      if (rv == net::ERR_IO_PENDING)
+        rv = callback.WaitForResult();
+      EXPECT_GE(rv, 0);
+      if (rv < 0)
+        return "(read failure)";
+      if (rv == 0)
+        return result;
+      result.append(buf->data(), rv);
+    }
+  }
+
+  base::File::Error WriteSync(uint64_t position,
+                              blink::mojom::BlobPtr blob,
+                              uint64_t* bytes_written_out) {
+    base::RunLoop loop;
+    base::File::Error result_out;
+    writer_->Write(position, std::move(blob),
+                   base::BindLambdaForTesting(
+                       [&](base::File::Error result, uint64_t bytes_written) {
+                         result_out = result;
+                         *bytes_written_out = bytes_written;
+                         loop.Quit();
+                       }));
+    loop.Run();
+    return result_out;
+  }
+
+  base::File::Error TruncateSync(uint64_t length) {
+    base::RunLoop loop;
+    base::File::Error result_out;
+    writer_->Truncate(length,
+                      base::BindLambdaForTesting([&](base::File::Error result) {
+                        result_out = result;
+                        loop.Quit();
+                      }));
+    loop.Run();
+    return result_out;
+  }
+
+ protected:
+  base::test::ScopedTaskEnvironment scoped_task_environment_;
+
+  base::ScopedTempDir dir_;
+  scoped_refptr<FileSystemContext> file_system_context_;
+  std::unique_ptr<BlobStorageContext> blob_context_;
+
+  FileSystemURL test_url_;
+
+  std::unique_ptr<FileWriterImpl> writer_;
+};
+
+TEST_F(FileWriterImplTest, WriteInvalidBlob) {
+  blink::mojom::BlobPtr blob;
+  MakeRequest(&blob);
+
+  uint64_t bytes_written;
+  base::File::Error result = WriteSync(0, std::move(blob), &bytes_written);
+  EXPECT_EQ(result, base::File::FILE_ERROR_FAILED);
+  EXPECT_EQ(bytes_written, 0u);
+
+  EXPECT_EQ("", ReadFile(test_url_));
+}
+
+TEST_F(FileWriterImplTest, WriteValidEmptyBlob) {
+  uint64_t bytes_written;
+  base::File::Error result = WriteSync(0, CreateBlob(""), &bytes_written);
+  EXPECT_EQ(result, base::File::FILE_OK);
+  EXPECT_EQ(bytes_written, 0u);
+
+  EXPECT_EQ("", ReadFile(test_url_));
+}
+
+TEST_F(FileWriterImplTest, WriteValidBlob) {
+  uint64_t bytes_written;
+  base::File::Error result =
+      WriteSync(0, CreateBlob("1234567890"), &bytes_written);
+  EXPECT_EQ(result, base::File::FILE_OK);
+  EXPECT_EQ(bytes_written, 10u);
+
+  EXPECT_EQ("1234567890", ReadFile(test_url_));
+}
+
+TEST_F(FileWriterImplTest, WriteWithOffsetInFile) {
+  uint64_t bytes_written;
+  base::File::Error result;
+
+  result = WriteSync(0, CreateBlob("1234567890"), &bytes_written);
+  EXPECT_EQ(result, base::File::FILE_OK);
+  EXPECT_EQ(bytes_written, 10u);
+
+  result = WriteSync(4, CreateBlob("abc"), &bytes_written);
+  EXPECT_EQ(result, base::File::FILE_OK);
+  EXPECT_EQ(bytes_written, 3u);
+
+  EXPECT_EQ("1234abc890", ReadFile(test_url_));
+}
+
+TEST_F(FileWriterImplTest, WriteWithOffsetPastFile) {
+  uint64_t bytes_written;
+  base::File::Error result = WriteSync(4, CreateBlob("abc"), &bytes_written);
+  EXPECT_EQ(result, base::File::FILE_ERROR_FAILED);
+  EXPECT_EQ(bytes_written, 0u);
+
+  EXPECT_EQ("", ReadFile(test_url_));
+}
+
+TEST_F(FileWriterImplTest, TruncateShrink) {
+  uint64_t bytes_written;
+  base::File::Error result;
+
+  result = WriteSync(0, CreateBlob("1234567890"), &bytes_written);
+  EXPECT_EQ(result, base::File::FILE_OK);
+  EXPECT_EQ(bytes_written, 10u);
+
+  result = TruncateSync(5);
+  EXPECT_EQ(result, base::File::FILE_OK);
+
+  EXPECT_EQ("12345", ReadFile(test_url_));
+}
+
+TEST_F(FileWriterImplTest, TruncateGrow) {
+  uint64_t bytes_written;
+  base::File::Error result;
+
+  result = WriteSync(0, CreateBlob("abc"), &bytes_written);
+  EXPECT_EQ(result, base::File::FILE_OK);
+  EXPECT_EQ(bytes_written, 3u);
+
+  result = TruncateSync(5);
+  EXPECT_EQ(result, base::File::FILE_OK);
+
+  EXPECT_EQ(std::string("abc\0\0", 5), ReadFile(test_url_));
+}
+
+// TODO(mek): More tests, particularly for error conditions.
+
+}  // namespace storage