Bug 1798459 - Use a dedicated close message for closing writable file streams; r=dom-storage-reviewers,jesup,jari

Depends on D160915

Differential Revision: https://phabricator.services.mozilla.com/D161023
This commit is contained in:
Jan Varga 2022-11-09 17:15:32 +00:00
parent cd3ad114fc
commit 504b78d1c9
12 changed files with 148 additions and 116 deletions

View File

@ -16,6 +16,7 @@
#include "mozilla/dom/WritableStreamDefaultController.h"
#include "nsIInputStream.h"
#include "nsNetUtil.h"
#include "private/pprio.h"
namespace mozilla {
extern LazyLogModule gOPFSLog;
@ -68,9 +69,7 @@ class WritableFileStreamUnderlyingSinkAlgorithms final
}
MOZ_CAN_RUN_SCRIPT already_AddRefed<Promise> CloseCallback(
JSContext* aCx, ErrorResult& aRv) override {
return mStream->Close(aRv);
};
JSContext* aCx, ErrorResult& aRv) override;
private:
~WritableFileStreamUnderlyingSinkAlgorithms() = default;
@ -83,20 +82,24 @@ class WritableFileStreamUnderlyingSinkAlgorithms final
FileSystemWritableFileStream::FileSystemWritableFileStream(
nsIGlobalObject* aGlobal, RefPtr<FileSystemManager>& aManager,
RefPtr<FileSystemWritableFileStreamChild> aActor,
const ::mozilla::ipc::FileDescriptor& aFileDescriptor,
const fs::FileSystemEntryMetadata& aMetadata)
: WritableStream(aGlobal),
mManager(aManager),
mActor(std::move(aActor)),
mMetadata(aMetadata) {
LOG(("Created WritableFileStream %p for fd %p", this,
mActor->MutableFileDescPtr()));
mFileDesc(nullptr),
mMetadata(aMetadata),
mClosed(false) {
auto rawFD = aFileDescriptor.ClonePlatformHandle();
mFileDesc = PR_ImportFile(PROsfd(rawFD.release()));
LOG(("Created WritableFileStream %p for fd %p", this, mFileDesc));
mActor->SetStream(this);
}
FileSystemWritableFileStream::~FileSystemWritableFileStream() {
if (mActor) {
mActor->Close();
}
MOZ_ASSERT(!mActor);
MOZ_ASSERT(mClosed);
}
// https://streams.spec.whatwg.org/#writablestream-set-up
@ -112,6 +115,7 @@ MOZ_CAN_RUN_SCRIPT_BOUNDARY already_AddRefed<FileSystemWritableFileStream>
FileSystemWritableFileStream::Create(
nsIGlobalObject* aGlobal, RefPtr<FileSystemManager>& aManager,
RefPtr<FileSystemWritableFileStreamChild> aActor,
const ::mozilla::ipc::FileDescriptor& aFileDescriptor,
const fs::FileSystemEntryMetadata& aMetadata) {
AutoJSAPI jsapi;
if (!jsapi.Init(aGlobal)) {
@ -123,7 +127,7 @@ FileSystemWritableFileStream::Create(
// (Done by the constructor)
RefPtr<FileSystemWritableFileStream> stream =
new FileSystemWritableFileStream(aGlobal, aManager, std::move(aActor),
aMetadata);
aFileDescriptor, aMetadata);
// Step 1 - 3
auto algorithms =
@ -157,42 +161,41 @@ NS_IMPL_CYCLE_COLLECTION_CLASS(FileSystemWritableFileStream)
NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(FileSystemWritableFileStream,
WritableStream)
// Per the comment for the FileSystemManager class, don't unlink mManager!
tmp->Close();
NS_IMPL_CYCLE_COLLECTION_UNLINK_END
NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INHERITED(FileSystemWritableFileStream,
WritableStream)
NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mManager)
NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
void FileSystemWritableFileStream::LastRelease() {
Close();
if (mActor) {
PFileSystemWritableFileStreamChild::Send__delete__(mActor);
MOZ_ASSERT(!mActor);
}
}
void FileSystemWritableFileStream::ClearActor() {
MOZ_ASSERT(mActor);
mActor = nullptr;
}
bool FileSystemWritableFileStream::IsClosed() const {
return !mActor || !mActor->MutableFileDescPtr();
}
already_AddRefed<Promise> FileSystemWritableFileStream::Close(
ErrorResult& aRv) {
RefPtr<Promise> promise = Promise::Create(GetParentObject(), aRv);
if (aRv.Failed()) {
return nullptr;
}
if (IsClosed()) {
promise->MaybeRejectWithTypeError("WritableFileStream closed");
return promise.forget();
void FileSystemWritableFileStream::Close() {
if (mClosed) {
return;
}
if (mActor) {
// close file, tell parent file is closed
LOG(("WritableFileStream %p: Closing", mActor->MutableFileDescPtr()));
mActor->Close();
MOZ_ASSERT(!mActor);
}
LOG(("%p: Closing", mFileDesc));
promise->MaybeResolveWithUndefined();
return promise.forget();
mClosed = true;
PR_Close(mFileDesc);
mFileDesc = nullptr;
mActor->SendClose();
}
// WebIDL Boilerplate
@ -219,7 +222,7 @@ already_AddRefed<Promise> FileSystemWritableFileStream::Write(
nsresult result;
UniquePtr<uint8_t> temp;
if (IsClosed()) {
if (mClosed) {
promise->MaybeRejectWithTypeError("WritableFileStream closed");
return promise.forget();
}
@ -341,14 +344,13 @@ already_AddRefed<Promise> FileSystemWritableFileStream::Write(
#if 0
// Note that this is writing to a copy of the file, not the file itself
MOZ_ALWAYS_SUCCEEDS(mTaskQueue->Dispatch(NS_NewRunnableFunction("WritableFileStream::write",
[fd = mActor->MutableFileDescPtr(), buffer = std::move(buffer), length, promise]() {
[fd = mFileDesc, buffer = std::move(buffer), length, promise]() {
uint64_t written = 0;
written = PR_Write(fd, buffer.get(), length); // XXX errors?
promise->MaybeResolve(written);
})));
#else
written = PR_Write(mActor->MutableFileDescPtr(), buffer.get(),
length); // XXX errors?
written = PR_Write(mFileDesc, buffer.get(), length); // XXX errors?
promise->MaybeResolve(written);
#endif
return promise.forget();
@ -360,7 +362,7 @@ already_AddRefed<Promise> FileSystemWritableFileStream::Seek(
if (aError.Failed()) {
return nullptr;
}
if (IsClosed()) {
if (mClosed) {
promise->MaybeRejectWithTypeError("WritableFileStream closed");
return promise.forget();
}
@ -377,7 +379,7 @@ already_AddRefed<Promise> FileSystemWritableFileStream::Truncate(
if (aError.Failed()) {
return nullptr;
}
if (IsClosed()) {
if (mClosed) {
promise->MaybeRejectWithTypeError("WritableFileStream closed");
return promise.forget();
}
@ -390,28 +392,24 @@ already_AddRefed<Promise> FileSystemWritableFileStream::Truncate(
// Spec issues raised.
// truncate filehandle (can extend with 0's)
if (mActor->MutableFileDescPtr()) {
LOG(("%p: Truncate to %" PRIu64, mActor->MutableFileDescPtr(), aSize));
if (NS_WARN_IF(NS_FAILED(TruncFile(mActor->MutableFileDescPtr(), aSize)))) {
LOG(("%p: Truncate to %" PRIu64, mFileDesc, aSize));
if (NS_WARN_IF(NS_FAILED(TruncFile(mFileDesc, aSize)))) {
promise->MaybeReject(NS_ErrorAccordingToNSPR());
} else {
// We truncated; per non-normative text in the spec (2.5.3) we should
// adjust the cursor position to be within the new file size
int64_t where = PR_Seek(mFileDesc, 0, PR_SEEK_CUR);
if (where == -1) {
promise->MaybeReject(NS_ErrorAccordingToNSPR());
} else {
// We truncated; per non-normative text in the spec (2.5.3) we should
// adjust the cursor position to be within the new file size
int64_t where = PR_Seek(mActor->MutableFileDescPtr(), 0, PR_SEEK_CUR);
return promise.forget();
}
if (where > (int64_t)aSize) {
where = PR_Seek(mFileDesc, 0, PR_SEEK_END);
if (where == -1) {
promise->MaybeReject(NS_ErrorAccordingToNSPR());
return promise.forget();
}
if (where > (int64_t)aSize) {
where = PR_Seek(mActor->MutableFileDescPtr(), 0, PR_SEEK_END);
if (where == -1) {
promise->MaybeReject(NS_ErrorAccordingToNSPR());
}
}
promise->MaybeResolveWithUndefined();
}
} else {
promise->MaybeReject(NS_ERROR_DOM_INVALID_STATE_ERR);
promise->MaybeResolveWithUndefined();
}
return promise.forget();
@ -420,7 +418,7 @@ already_AddRefed<Promise> FileSystemWritableFileStream::Truncate(
nsresult FileSystemWritableFileStream::WriteBlob(Blob* aBlob,
uint64_t& aWritten) {
NS_ENSURE_ARG_POINTER(aBlob);
if (IsClosed()) {
if (mClosed) {
return NS_ERROR_DOM_NOT_FOUND_ERR; // XXX
}
@ -439,8 +437,7 @@ nsresult FileSystemWritableFileStream::WriteBlob(Blob* aBlob,
}
// XXX!!! this should be submitted to a TaskQueue instead of sync IO on
// mainthread!
aWritten +=
PR_Write(mActor->MutableFileDescPtr(), data, length); // XXX errors?
aWritten += PR_Write(mFileDesc, data, length); // XXX errors?
free(data);
return NS_OK;
}
@ -453,14 +450,12 @@ bool FileSystemWritableFileStream::DoSeek(RefPtr<Promise>& aPromise,
// XXX what happens if we read/write before seek finishes?
// Should we block read/write if an async operation is pending?
// Handle seek before write ('at')
LOG_VERBOSE(
("%p: Seeking to %" PRIu64, mActor->MutableFileDescPtr(), aPosition));
LOG_VERBOSE(("%p: Seeking to %" PRIu64, mFileDesc, aPosition));
const CheckedInt<PROffset32> checkedPosition(aPosition);
if (NS_WARN_IF(!checkedPosition.isValid())) {
return false;
}
int64_t where = PR_Seek(mActor->MutableFileDescPtr(), checkedPosition.value(),
PR_SEEK_SET);
int64_t where = PR_Seek(mFileDesc, checkedPosition.value(), PR_SEEK_SET);
if (where == -1) {
LOG(("Failed to seek to %" PRIu64 " (errno %d)", aPosition, errno));
aPromise->MaybeReject(NS_ERROR_FAILURE); // XXX
@ -499,4 +494,23 @@ WritableFileStreamUnderlyingSinkAlgorithms::WriteCallback(
return mStream->Write(chunkUnion, aRv);
}
already_AddRefed<Promise>
WritableFileStreamUnderlyingSinkAlgorithms::CloseCallback(JSContext* aCx,
ErrorResult& aRv) {
RefPtr<Promise> promise = Promise::Create(mStream->GetParentObject(), aRv);
if (aRv.Failed()) {
return nullptr;
}
if (mStream->IsClosed()) {
promise->MaybeRejectWithTypeError("WritableFileStream closed");
return promise.forget();
}
mStream->Close();
promise->MaybeResolveWithUndefined();
return promise.forget();
}
} // namespace mozilla::dom

View File

@ -31,17 +31,20 @@ class FileSystemWritableFileStream final : public WritableStream {
static already_AddRefed<FileSystemWritableFileStream> Create(
nsIGlobalObject* aGlobal, RefPtr<FileSystemManager>& aManager,
RefPtr<FileSystemWritableFileStreamChild> aActor,
const ::mozilla::ipc::FileDescriptor& aFileDescriptor,
const fs::FileSystemEntryMetadata& aMetadata);
NS_DECL_ISUPPORTS_INHERITED
NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(FileSystemWritableFileStream,
WritableStream)
void LastRelease() override;
void ClearActor();
bool IsClosed() const;
bool IsClosed() const { return mClosed; }
already_AddRefed<Promise> Close(ErrorResult& aRv);
void Close();
// WebIDL Boilerplate
JSObject* WrapObject(JSContext* aCx,
@ -57,10 +60,11 @@ class FileSystemWritableFileStream final : public WritableStream {
already_AddRefed<Promise> Truncate(uint64_t aSize, ErrorResult& aError);
private:
FileSystemWritableFileStream(nsIGlobalObject* aGlobal,
RefPtr<FileSystemManager>& aManager,
RefPtr<FileSystemWritableFileStreamChild> aActor,
const fs::FileSystemEntryMetadata& aMetadata);
FileSystemWritableFileStream(
nsIGlobalObject* aGlobal, RefPtr<FileSystemManager>& aManager,
RefPtr<FileSystemWritableFileStreamChild> aActor,
const ::mozilla::ipc::FileDescriptor& aFileDescriptor,
const fs::FileSystemEntryMetadata& aMetadata);
virtual ~FileSystemWritableFileStream();
@ -72,7 +76,11 @@ class FileSystemWritableFileStream final : public WritableStream {
RefPtr<FileSystemWritableFileStreamChild> mActor;
PRFileDesc* mFileDesc;
fs::FileSystemEntryMetadata mMetadata;
bool mClosed;
};
} // namespace dom

View File

@ -9,6 +9,7 @@
#include "FileSystemAccessHandleChild.h"
#include "FileSystemWritableFileStreamChild.h"
#include "mozilla/dom/FileSystemSyncAccessHandle.h"
#include "mozilla/dom/FileSystemWritableFileStream.h"
namespace mozilla::dom {
@ -26,7 +27,7 @@ void FileSystemManagerChild::CloseAll() {
for (const auto& item : ManagedPFileSystemWritableFileStreamChild()) {
auto* child = static_cast<FileSystemWritableFileStreamChild*>(item);
child->Close();
child->MutableWritableFileStreamPtr()->Close();
}
}
@ -44,9 +45,8 @@ FileSystemManagerChild::AllocPFileSystemAccessHandleChild() {
}
already_AddRefed<PFileSystemWritableFileStreamChild>
FileSystemManagerChild::AllocPFileSystemWritableFileStreamChild(
const FileDescriptor& aFileDescriptor) {
return MakeAndAddRef<FileSystemWritableFileStreamChild>(aFileDescriptor);
FileSystemManagerChild::AllocPFileSystemWritableFileStreamChild() {
return MakeAndAddRef<FileSystemWritableFileStreamChild>();
}
::mozilla::ipc::IPCResult FileSystemManagerChild::RecvCloseAll(

View File

@ -24,8 +24,7 @@ class FileSystemManagerChild : public PFileSystemManagerChild {
AllocPFileSystemAccessHandleChild();
already_AddRefed<PFileSystemWritableFileStreamChild>
AllocPFileSystemWritableFileStreamChild(
const FileDescriptor& aFileDescriptor);
AllocPFileSystemWritableFileStreamChild();
::mozilla::ipc::IPCResult RecvCloseAll(CloseAllResolver&& aResolver);

View File

@ -118,11 +118,15 @@ RefPtr<FileSystemWritableFileStream> MakeResolution(
const RefPtr<FileSystemWritableFileStream>& /* aReturns */,
const FileSystemEntryMetadata& aMetadata,
RefPtr<FileSystemManager>& aManager) {
const auto& properties =
aResponse.get_FileSystemWritableFileStreamProperties();
auto* const actor = static_cast<FileSystemWritableFileStreamChild*>(
aResponse.get_PFileSystemWritableFileStreamChild());
properties.writableFileStreamChild());
RefPtr<FileSystemWritableFileStream> result =
FileSystemWritableFileStream::Create(aGlobal, aManager, actor, aMetadata);
FileSystemWritableFileStream::Create(
aGlobal, aManager, actor, properties.fileDescriptor(), aMetadata);
return result;
}

View File

@ -7,7 +7,6 @@
#include "FileSystemWritableFileStreamChild.h"
#include "mozilla/dom/FileSystemWritableFileStream.h"
#include "private/pprio.h"
namespace mozilla {
extern LazyLogModule gOPFSLog;
@ -20,11 +19,8 @@ extern LazyLogModule gOPFSLog;
namespace mozilla::dom {
FileSystemWritableFileStreamChild::FileSystemWritableFileStreamChild(
const ::mozilla::ipc::FileDescriptor& aFileDescriptor)
FileSystemWritableFileStreamChild::FileSystemWritableFileStreamChild()
: mStream(nullptr) {
auto rawFD = aFileDescriptor.ClonePlatformHandle();
mFileDesc = PR_ImportFile(PROsfd(rawFD.release()));
LOG(("Created new WritableFileStreamChild %p", this));
}
@ -36,28 +32,8 @@ void FileSystemWritableFileStreamChild::SetStream(
mStream = aStream;
}
PRFileDesc* FileSystemWritableFileStreamChild::MutableFileDescPtr() const {
MOZ_ASSERT(mFileDesc);
return mFileDesc;
}
void FileSystemWritableFileStreamChild::Close() {
MOZ_ASSERT(mFileDesc);
LOG(("Closing WritableFileStreamChild %p", this));
PR_Close(mFileDesc);
mFileDesc = nullptr;
PFileSystemWritableFileStreamChild::Send__delete__(this);
}
void FileSystemWritableFileStreamChild::ActorDestroy(ActorDestroyReason aWhy) {
LOG(("Destroy WritableFileStreamChild %p", this));
if (mFileDesc) {
PR_Close(mFileDesc);
mFileDesc = nullptr;
}
if (mStream) {
mStream->ClearActor();

View File

@ -16,27 +16,25 @@ class FileSystemWritableFileStream;
class FileSystemWritableFileStreamChild
: public PFileSystemWritableFileStreamChild {
public:
explicit FileSystemWritableFileStreamChild(
const FileDescriptor& aFileDescriptor);
FileSystemWritableFileStreamChild();
NS_INLINE_DECL_REFCOUNTING(FileSystemWritableFileStreamChild, override)
FileSystemWritableFileStream* MutableWritableFileStreamPtr() const {
MOZ_ASSERT(mStream);
return mStream;
}
void SetStream(FileSystemWritableFileStream* aStream);
PRFileDesc* MutableFileDescPtr() const;
void Close();
void ActorDestroy(ActorDestroyReason aWhy) override;
private:
virtual ~FileSystemWritableFileStreamChild();
// Use a weak ref so actor does not hold DOM object alive past content use.
// The weak reference is cleared in FileSystemWritableFileStream destructor.
// The weak reference is cleared in FileSystemWritableFileStream::LastRelease.
FileSystemWritableFileStream* MOZ_NON_OWNING_REF mStream;
PRFileDesc* mFileDesc;
};
} // namespace mozilla::dom

View File

@ -216,13 +216,13 @@ mozilla::ipc::IPCResult FileSystemManagerParent::RecvGetWritable(
auto writableFileStreamParent =
MakeRefPtr<FileSystemWritableFileStreamParent>(this, aRequest.entryId());
if (!SendPFileSystemWritableFileStreamConstructor(writableFileStreamParent,
fileDescriptor)) {
if (!SendPFileSystemWritableFileStreamConstructor(writableFileStreamParent)) {
aResolver(NS_ERROR_FAILURE);
return IPC_OK();
}
aResolver(FileSystemGetWritableFileStreamResponse(writableFileStreamParent));
aResolver(FileSystemWritableFileStreamProperties(
fileDescriptor, writableFileStreamParent, nullptr));
return IPC_OK();
}

View File

@ -15,11 +15,27 @@ FileSystemWritableFileStreamParent::FileSystemWritableFileStreamParent(
RefPtr<FileSystemManagerParent> aManager, const fs::EntryId& aEntryId)
: mManager(std::move(aManager)), mEntryId(aEntryId) {}
FileSystemWritableFileStreamParent::~FileSystemWritableFileStreamParent() =
default;
FileSystemWritableFileStreamParent::~FileSystemWritableFileStreamParent() {
MOZ_ASSERT(mClosed);
}
mozilla::ipc::IPCResult FileSystemWritableFileStreamParent::RecvClose() {
Close();
return IPC_OK();
}
void FileSystemWritableFileStreamParent::ActorDestroy(ActorDestroyReason aWhy) {
if (!IsClosed()) {
Close();
}
}
void FileSystemWritableFileStreamParent::Close() {
LOG(("Closing WritableFileStream"));
mClosed.Flip();
mManager->DataManagerStrongRef()->UnlockShared(mEntryId);
}

View File

@ -8,6 +8,7 @@
#define DOM_FS_PARENT_FILESYSTEMWRITABLEFILESTREAM_H_
#include "mozilla/dom/FileSystemTypes.h"
#include "mozilla/dom/FlippedOnce.h"
#include "mozilla/dom/PFileSystemWritableFileStreamParent.h"
namespace mozilla::dom {
@ -23,14 +24,22 @@ class FileSystemWritableFileStreamParent
NS_INLINE_DECL_THREADSAFE_REFCOUNTING(FileSystemWritableFileStreamParent,
override)
mozilla::ipc::IPCResult RecvClose();
void ActorDestroy(ActorDestroyReason aWhy) override;
private:
virtual ~FileSystemWritableFileStreamParent();
bool IsClosed() const { return mClosed; }
void Close();
const RefPtr<FileSystemManagerParent> mManager;
const fs::EntryId mEntryId;
FlippedOnce<false> mClosed;
};
} // namespace mozilla::dom

View File

@ -143,10 +143,16 @@ union FileSystemGetAccessHandleResponse
FileSystemAccessHandleProperties;
};
struct FileSystemWritableFileStreamProperties
{
FileDescriptor fileDescriptor;
PFileSystemWritableFileStream writableFileStream;
};
union FileSystemGetWritableFileStreamResponse
{
nsresult;
PFileSystemWritableFileStream;
FileSystemWritableFileStreamProperties;
};
/**
@ -402,7 +408,7 @@ async protocol PFileSystemManager
child:
async PFileSystemAccessHandle();
async PFileSystemWritableFileStream(FileDescriptor fileDescriptor);
async PFileSystemWritableFileStream();
async CloseAll()
returns(nsresult rv);

View File

@ -12,6 +12,8 @@ async protocol PFileSystemWritableFileStream
manager PFileSystemManager;
parent:
async Close();
async __delete__();
};