Commit 74cf65e4 authored by Olivier Yiptong's avatar Olivier Yiptong Committed by Commit Bot

[Native File System] Create FileSystemFileWriterImpl to mirror the renderer side

This is the initial cl of the refactor of NativeFileSystemFileHandleImpl to
mirror the split on the renderer side with the handle and the writer as
two separate entities.

It is the initial implementation for FileSystemFileWriterImpl. It includes
only one no-op method for now: close().

This sets the stage for atomic writes as close() will be the explicit
termination of a write, which allows us to do the atomic operation.

Bug: 968550
Change-Id: I0af81efa46789f18c4a8b0bca3ab89c5703b798c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1662709
Commit-Queue: Olivier Yiptong <oyiptong@chromium.org>
Reviewed-by: default avatarDaniel Cheng <dcheng@chromium.org>
Reviewed-by: default avatarMarijn Kruisselbrink <mek@chromium.org>
Auto-Submit: Olivier Yiptong <oyiptong@chromium.org>
Cr-Commit-Position: refs/heads/master@{#671539}
parent 52a3d269
......@@ -1338,6 +1338,8 @@ jumbo_source_set("browser") {
"native_file_system/native_file_system_directory_handle_impl.h",
"native_file_system/native_file_system_file_handle_impl.cc",
"native_file_system/native_file_system_file_handle_impl.h",
"native_file_system/native_file_system_file_writer_impl.cc",
"native_file_system/native_file_system_file_writer_impl.h",
"native_file_system/native_file_system_handle_base.cc",
"native_file_system/native_file_system_handle_base.h",
"native_file_system/native_file_system_manager_impl.cc",
......
......@@ -127,6 +127,21 @@ void NativeFileSystemFileHandleImpl::Truncate(uint64_t length,
std::move(callback));
}
void NativeFileSystemFileHandleImpl::CreateFileWriter(
CreateFileWriterCallback callback) {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
RunWithWritePermission(
base::BindOnce(&NativeFileSystemFileHandleImpl::CreateFileWriterImpl,
weak_factory_.GetWeakPtr()),
base::BindOnce([](CreateFileWriterCallback callback) {
std::move(callback).Run(
NativeFileSystemError::New(base::File::FILE_ERROR_ACCESS_DENIED),
nullptr);
}),
std::move(callback));
}
void NativeFileSystemFileHandleImpl::Transfer(
blink::mojom::NativeFileSystemTransferTokenRequest token) {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
......@@ -260,6 +275,17 @@ void NativeFileSystemFileHandleImpl::TruncateImpl(uint64_t length,
std::move(callback)));
}
void NativeFileSystemFileHandleImpl::CreateFileWriterImpl(
CreateFileWriterCallback callback) {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
DCHECK_EQ(GetWritePermissionStatus(),
blink::mojom::PermissionStatus::GRANTED);
std::move(callback).Run(
NativeFileSystemError::New(base::File::FILE_OK),
manager()->CreateFileWriter(context(), url(), handle_state()));
}
base::WeakPtr<NativeFileSystemHandleBase>
NativeFileSystemFileHandleImpl::AsWeakPtr() {
return weak_factory_.GetWeakPtr();
......
......@@ -55,6 +55,7 @@ class CONTENT_EXPORT NativeFileSystemFileHandleImpl
mojo::ScopedDataPipeConsumerHandle stream,
WriteStreamCallback callback) override;
void Truncate(uint64_t length, TruncateCallback callback) override;
void CreateFileWriter(CreateFileWriterCallback callback) override;
void Transfer(
blink::mojom::NativeFileSystemTransferTokenRequest token) override;
......@@ -84,6 +85,7 @@ class CONTENT_EXPORT NativeFileSystemFileHandleImpl
bool complete);
void TruncateImpl(uint64_t length, TruncateCallback callback);
void CreateFileWriterImpl(CreateFileWriterCallback callback);
base::WeakPtr<NativeFileSystemHandleBase> AsWeakPtr() override;
......
// Copyright 2019 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 "content/browser/native_file_system/native_file_system_file_writer_impl.h"
#include "content/browser/native_file_system/native_file_system_manager_impl.h"
using blink::mojom::NativeFileSystemError;
namespace content {
NativeFileSystemFileWriterImpl::NativeFileSystemFileWriterImpl(
NativeFileSystemManagerImpl* manager,
const BindingContext& context,
const storage::FileSystemURL& url,
const SharedHandleState& handle_state)
: NativeFileSystemHandleBase(manager, context, url, handle_state) {}
NativeFileSystemFileWriterImpl::~NativeFileSystemFileWriterImpl() = default;
void NativeFileSystemFileWriterImpl::Close(CloseCallback callback) {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
RunWithWritePermission(
base::BindOnce(&NativeFileSystemFileWriterImpl::CloseImpl,
weak_factory_.GetWeakPtr()),
base::BindOnce([](CloseCallback callback) {
std::move(callback).Run(
NativeFileSystemError::New(base::File::FILE_ERROR_ACCESS_DENIED));
}),
std::move(callback));
}
void NativeFileSystemFileWriterImpl::CloseImpl(CloseCallback callback) {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
DCHECK_EQ(GetWritePermissionStatus(),
blink::mojom::PermissionStatus::GRANTED);
std::move(callback).Run(NativeFileSystemError::New(base::File::FILE_OK));
}
base::WeakPtr<NativeFileSystemHandleBase>
NativeFileSystemFileWriterImpl::AsWeakPtr() {
return weak_factory_.GetWeakPtr();
}
} // namespace content
// Copyright 2019 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 CONTENT_BROWSER_NATIVE_FILE_SYSTEM_NATIVE_FILE_SYSTEM_FILE_WRITER_IMPL_H_
#define CONTENT_BROWSER_NATIVE_FILE_SYSTEM_NATIVE_FILE_SYSTEM_FILE_WRITER_IMPL_H_
#include "base/memory/weak_ptr.h"
#include "components/services/filesystem/public/interfaces/types.mojom.h"
#include "content/browser/native_file_system/native_file_system_file_handle_impl.h"
#include "content/browser/native_file_system/native_file_system_handle_base.h"
#include "content/common/content_export.h"
#include "storage/browser/fileapi/file_system_url.h"
#include "third_party/blink/public/mojom/native_file_system/native_file_system_file_writer.mojom.h"
namespace content {
// This is the browser side implementation of the
// NativeFileSystemFileWriter mojom interface. Instances of this class are
// owned by the NativeFileSystemManagerImpl instance passed in to the
// constructor.
//
// This class is not thread safe, all methods should be called on the IO thread.
// The link to the IO thread is due to its dependencies on both the blob system
// (via storage::BlobStorageContext) and the file system backends (via
// storage::FileSystemContext and storage::FileSystemOperationRunner, which both
// expect some of their methods to always be called on the IO thread).
// See https://crbug.com/957249 for some thoughts about the blob system aspect
// of this.
class CONTENT_EXPORT NativeFileSystemFileWriterImpl
: public NativeFileSystemHandleBase,
public blink::mojom::NativeFileSystemFileWriter {
public:
NativeFileSystemFileWriterImpl(NativeFileSystemManagerImpl* manager,
const BindingContext& context,
const storage::FileSystemURL& url,
const SharedHandleState& handle_state);
~NativeFileSystemFileWriterImpl() override;
void Close(CloseCallback callback) override;
private:
void CloseImpl(CloseCallback callback);
base::WeakPtr<NativeFileSystemHandleBase> AsWeakPtr() override;
base::WeakPtrFactory<NativeFileSystemFileWriterImpl> weak_factory_{this};
DISALLOW_COPY_AND_ASSIGN(NativeFileSystemFileWriterImpl);
};
} // namespace content
#endif // CONTENT_BROWSER_NATIVE_FILE_SYSTEM_NATIVE_FILE_SYSTEM_FILE_WRITER_IMPL_H_
......@@ -10,6 +10,7 @@
#include "content/browser/native_file_system/fixed_native_file_system_permission_grant.h"
#include "content/browser/native_file_system/native_file_system_directory_handle_impl.h"
#include "content/browser/native_file_system/native_file_system_file_handle_impl.h"
#include "content/browser/native_file_system/native_file_system_file_writer_impl.h"
#include "content/browser/native_file_system/native_file_system_transfer_token_impl.h"
#include "content/public/browser/browser_task_traits.h"
#include "content/public/browser/browser_thread.h"
......@@ -139,6 +140,20 @@ NativeFileSystemManagerImpl::CreateDirectoryHandle(
return result;
}
blink::mojom::NativeFileSystemFileWriterPtr
NativeFileSystemManagerImpl::CreateFileWriter(
const BindingContext& binding_context,
const storage::FileSystemURL& url,
const SharedHandleState& handle_state) {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
blink::mojom::NativeFileSystemFileWriterPtr result;
writer_bindings_.AddBinding(std::make_unique<NativeFileSystemFileWriterImpl>(
this, binding_context, url, handle_state),
mojo::MakeRequest(&result));
return result;
}
blink::mojom::NativeFileSystemEntryPtr
NativeFileSystemManagerImpl::CreateFileEntryFromPath(
const BindingContext& binding_context,
......@@ -363,4 +378,4 @@ NativeFileSystemManagerImpl::CreateFileSystemURLFromPath(
return result;
}
} // namespace content
\ No newline at end of file
} // namespace content
......@@ -16,6 +16,7 @@
#include "mojo/public/cpp/bindings/strong_binding.h"
#include "mojo/public/cpp/bindings/strong_binding_set.h"
#include "storage/browser/fileapi/file_system_url.h"
#include "third_party/blink/public/mojom/native_file_system/native_file_system_file_writer.mojom.h"
#include "third_party/blink/public/mojom/native_file_system/native_file_system_manager.mojom.h"
#include "third_party/blink/public/mojom/permissions/permission_status.mojom.h"
......@@ -106,6 +107,13 @@ class CONTENT_EXPORT NativeFileSystemManagerImpl
const storage::FileSystemURL& url,
const SharedHandleState& handle_state);
// Creates a new NativeFileSystemFileWriterImpl for a given url. Assumes the
// passed in URL is valid and represents a file.
blink::mojom::NativeFileSystemFileWriterPtr CreateFileWriter(
const BindingContext& binding_context,
const storage::FileSystemURL& url,
const SharedHandleState& handle_state);
// Creates a new NativeFileSystemEntryPtr from the path to a file. Assumes the
// passed in path is valid and represents a file.
blink::mojom::NativeFileSystemEntryPtr CreateFileEntryFromPath(
......@@ -199,6 +207,8 @@ class CONTENT_EXPORT NativeFileSystemManagerImpl
file_bindings_;
mojo::StrongBindingSet<blink::mojom::NativeFileSystemDirectoryHandle>
directory_bindings_;
mojo::StrongBindingSet<blink::mojom::NativeFileSystemFileWriter>
writer_bindings_;
// Transfer token bindings are stored in what is effectively a
// StrongBindingMap. The Binding instances own the implementation, and tokens
......
......@@ -76,6 +76,7 @@ mojom("mojom_platform") {
"native_file_system/native_file_system_directory_handle.mojom",
"native_file_system/native_file_system_error.mojom",
"native_file_system/native_file_system_file_handle.mojom",
"native_file_system/native_file_system_file_writer.mojom",
"native_file_system/native_file_system_manager.mojom",
"native_file_system/native_file_system_transfer_token.mojom",
"net/ip_address_space.mojom",
......
......@@ -7,6 +7,7 @@ module blink.mojom;
import "third_party/blink/public/mojom/blob/blob.mojom";
import "third_party/blink/public/mojom/blob/serialized_blob.mojom";
import "third_party/blink/public/mojom/native_file_system/native_file_system_error.mojom";
import "third_party/blink/public/mojom/native_file_system/native_file_system_file_writer.mojom";
import "third_party/blink/public/mojom/native_file_system/native_file_system_transfer_token.mojom";
import "third_party/blink/public/mojom/permissions/permission_status.mojom";
......@@ -24,6 +25,7 @@ interface NativeFileSystemFileHandle {
AsBlob() => (NativeFileSystemError result, SerializedBlob? blob);
// Deletes this file.
// TODO(oyiptong): Remove this method. It isn't part of the spec anymore.
Remove() => (NativeFileSystemError result);
// Write data from |data| to the given |position| in the file being written
......@@ -31,6 +33,7 @@ interface NativeFileSystemFileHandle {
// written.
// TODO(mek): This might need some way of reporting progress events back to
// the renderer.
// TODO(oyiptong): Refactor into NFSFileWriter.
Write(uint64 offset, Blob data) => (NativeFileSystemError result,
uint64 bytes_written);
......@@ -39,14 +42,19 @@ interface NativeFileSystemFileHandle {
// written.
// TODO(mek): This might need some way of reporting progress events back to
// the renderer.
// TODO(oyiptong): Refactor into NFSFileWriter.
WriteStream(uint64 offset, handle<data_pipe_consumer> stream) =>
(NativeFileSystemError result, uint64 bytes_written);
// Changes the length of the file to be |length|. If |length| is larger than
// the current size of the file, the file will be extended, and the extended
// part is filled with null bytes.
// TODO(oyiptong): Refactor into NFSFileWriter.
Truncate(uint64 length) => (NativeFileSystemError result);
// Returns a FileWriter object. The FileWriter provides write operations on a file.
CreateFileWriter() => (NativeFileSystemError result, NativeFileSystemFileWriter? writer);
// Create a TransferToken for this directory. This token can be used to pass
// a reference to this directory to other methods, for example to copy or move
// the file, or when transferring the handle over postMessage.
......
// Copyright 2019 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.
module blink.mojom;
import "third_party/blink/public/mojom/native_file_system/native_file_system_error.mojom";
// Represents an object to modify a file.
interface NativeFileSystemFileWriter {
// Closes the file writer. This will materialize the writes operations on the
// intended file target in the case of atomic writes.
// Close() will close the mojo pipe after completion.
// If the mojo pipe closes before Close() is invoked, the write operation is deemed
// unsuccessful. Any temporary artifacts will be deleted.
// Returns whether the operation succeeded.
Close() => (NativeFileSystemError result);
};
\ No newline at end of file
......@@ -5,6 +5,7 @@
#include "third_party/blink/renderer/modules/native_file_system/native_file_system_file_handle.h"
#include "third_party/blink/public/mojom/native_file_system/native_file_system_error.mojom-blink.h"
#include "third_party/blink/public/mojom/native_file_system/native_file_system_file_writer.mojom-blink.h"
#include "third_party/blink/public/mojom/native_file_system/native_file_system_transfer_token.mojom-blink.h"
#include "third_party/blink/renderer/bindings/core/v8/script_promise_resolver.h"
#include "third_party/blink/renderer/core/dom/dom_exception.h"
......@@ -30,7 +31,19 @@ ScriptPromise NativeFileSystemFileHandle::createWriter(
auto* resolver = MakeGarbageCollected<ScriptPromiseResolver>(script_state);
ScriptPromise result = resolver->Promise();
resolver->Resolve(MakeGarbageCollected<NativeFileSystemWriter>(this));
mojo_ptr_->CreateFileWriter(WTF::Bind(
[](ScriptPromiseResolver* resolver, NativeFileSystemFileHandle* handle,
mojom::blink::NativeFileSystemErrorPtr result,
mojom::blink::NativeFileSystemFileWriterPtr writer) {
if (result->error_code == base::File::FILE_OK) {
resolver->Resolve(MakeGarbageCollected<NativeFileSystemWriter>(
handle, std::move(writer)));
} else {
resolver->Reject(file_error::CreateDOMException(result->error_code));
}
},
WrapPersistent(resolver), WrapPersistent(this)));
return result;
}
......
......@@ -24,9 +24,12 @@
namespace blink {
NativeFileSystemWriter::NativeFileSystemWriter(NativeFileSystemFileHandle* file)
: file_(file) {
NativeFileSystemWriter::NativeFileSystemWriter(
NativeFileSystemFileHandle* file,
mojom::blink::NativeFileSystemFileWriterPtr mojo_ptr)
: mojo_ptr_(std::move(mojo_ptr)), file_(file) {
DCHECK(file_);
DCHECK(mojo_ptr_);
}
ScriptPromise NativeFileSystemWriter::write(
......@@ -213,13 +216,18 @@ ScriptPromise NativeFileSystemWriter::truncate(ScriptState* script_state,
}
ScriptPromise NativeFileSystemWriter::close(ScriptState* script_state) {
if (!file_) {
if (!file_ || pending_operation_) {
return ScriptPromise::RejectWithDOMException(
script_state, MakeGarbageCollected<DOMException>(
DOMExceptionCode::kInvalidStateError));
}
file_ = nullptr;
return ScriptPromise::CastUndefined(script_state);
pending_operation_ =
MakeGarbageCollected<ScriptPromiseResolver>(script_state);
ScriptPromise result = pending_operation_->Promise();
mojo_ptr_->Close(
WTF::Bind(&NativeFileSystemWriter::CloseComplete, WrapPersistent(this)));
return result;
}
void NativeFileSystemWriter::Trace(Visitor* visitor) {
......@@ -254,4 +262,17 @@ void NativeFileSystemWriter::TruncateComplete(
pending_operation_ = nullptr;
}
void NativeFileSystemWriter::CloseComplete(
mojom::blink::NativeFileSystemErrorPtr result) {
DCHECK(pending_operation_);
if (result->error_code == base::File::FILE_OK) {
pending_operation_->Resolve();
} else {
pending_operation_->Reject(
file_error::CreateDOMException(result->error_code));
}
file_ = nullptr;
pending_operation_ = nullptr;
}
} // namespace blink
......@@ -6,6 +6,7 @@
#define THIRD_PARTY_BLINK_RENDERER_MODULES_NATIVE_FILE_SYSTEM_NATIVE_FILE_SYSTEM_WRITER_H_
#include "third_party/blink/public/mojom/native_file_system/native_file_system_error.mojom-blink.h"
#include "third_party/blink/public/mojom/native_file_system/native_file_system_file_writer.mojom-blink.h"
#include "third_party/blink/renderer/bindings/core/v8/array_buffer_or_array_buffer_view_or_blob_or_usv_string.h"
#include "third_party/blink/renderer/platform/bindings/script_wrappable.h"
......@@ -24,7 +25,8 @@ class NativeFileSystemWriter final : public ScriptWrappable {
DEFINE_WRAPPERTYPEINFO();
public:
explicit NativeFileSystemWriter(NativeFileSystemFileHandle*);
explicit NativeFileSystemWriter(NativeFileSystemFileHandle*,
mojom::blink::NativeFileSystemFileWriterPtr);
ScriptPromise write(ScriptState*,
uint64_t position,
......@@ -47,7 +49,9 @@ class NativeFileSystemWriter final : public ScriptWrappable {
void WriteComplete(mojom::blink::NativeFileSystemErrorPtr result,
uint64_t bytes_written);
void TruncateComplete(mojom::blink::NativeFileSystemErrorPtr result);
void CloseComplete(mojom::blink::NativeFileSystemErrorPtr result);
mojom::blink::NativeFileSystemFileWriterPtr mojo_ptr_;
Member<NativeFileSystemFileHandle> file_;
Member<ScriptPromiseResolver> pending_operation_;
......
......@@ -7,6 +7,7 @@ promise_test(async t => {
const writer = await handle.createWriter();
await writer.write(0, new Blob([]));
await writer.close();
assert_equals(await getFileContents(handle), '');
assert_equals(await getFileSize(handle), 0);
......@@ -17,6 +18,7 @@ promise_test(async t => {
const writer = await handle.createWriter();
await writer.write(0, new Blob(['1234567890']));
await writer.close();
assert_equals(await getFileContents(handle), '1234567890');
assert_equals(await getFileSize(handle), 10);
......@@ -28,6 +30,7 @@ promise_test(async t => {
await writer.write(0, new Blob(['1234567890']));
await writer.write(4, new Blob(['abc']));
await writer.close();
assert_equals(await getFileContents(handle), '1234abc890');
assert_equals(await getFileSize(handle), 10);
......@@ -38,6 +41,7 @@ promise_test(async t => {
const writer = await handle.createWriter();
await promise_rejects(t, 'InvalidStateError', writer.write(4, new Blob(['abc'])));
await writer.close();
assert_equals(await getFileContents(handle), '');
assert_equals(await getFileSize(handle), 0);
......@@ -48,6 +52,7 @@ promise_test(async t => {
const writer = await handle.createWriter();
await writer.write(0, '');
await writer.close();
assert_equals(await getFileContents(handle), '');
assert_equals(await getFileSize(handle), 0);
}, 'write() with an empty string to an empty file');
......@@ -57,6 +62,7 @@ promise_test(async t => {
const writer = await handle.createWriter();
await writer.write(0, 'foo🤘');
await writer.close();
assert_equals(await getFileContents(handle), 'foo🤘');
assert_equals(await getFileSize(handle), 7);
}, 'write() with a valid utf-8 string');
......@@ -66,6 +72,7 @@ promise_test(async t => {
const writer = await handle.createWriter();
await writer.write(0, 'foo\n');
await writer.close();
assert_equals(await getFileContents(handle), 'foo\n');
assert_equals(await getFileSize(handle), 4);
}, 'write() with a string with unix line ending preserved');
......@@ -75,6 +82,7 @@ promise_test(async t => {
const writer = await handle.createWriter();
await writer.write(0, 'foo\r\n');
await writer.close();
assert_equals(await getFileContents(handle), 'foo\r\n');
assert_equals(await getFileSize(handle), 5);
}, 'write() with a string with windows line ending preserved');
......@@ -85,6 +93,7 @@ promise_test(async t => {
let buf = new ArrayBuffer(0);
await writer.write(0, buf);
await writer.close();
assert_equals(await getFileContents(handle), '');
assert_equals(await getFileSize(handle), 0);
}, 'write() with an empty array buffer to an empty file');
......@@ -99,6 +108,7 @@ promise_test(async t => {
intView[1] = 0x6f;
intView[2] = 0x6f;
await writer.write(0, buf);
await writer.close();
assert_equals(await getFileContents(handle), 'foo');
assert_equals(await getFileSize(handle), 3);
}, 'write() with a valid typed array buffer');
......@@ -109,6 +119,7 @@ promise_test(async t => {
await writer.write(0, new Blob(['1234567890']));
await writer.truncate(5);
await writer.close();
assert_equals(await getFileContents(handle), '12345');
assert_equals(await getFileSize(handle), 5);
......@@ -120,6 +131,7 @@ promise_test(async t => {
await writer.write(0, new Blob(['abc']));
await writer.truncate(5);
await writer.close();
assert_equals(await getFileContents(handle), 'abc\0\0');
assert_equals(await getFileSize(handle), 5);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment