From bafc711011a58f9003f7cf8347af27b9979bbbdf Mon Sep 17 00:00:00 2001 From: Randell Jesup Date: Tue, 13 Sep 2022 17:43:44 +0000 Subject: [PATCH] Bug 1761089: Add support for getFile() to FileSystemHandles in OPFS r=dom-storage-reviewers,jari,janv Depends on D154482 Differential Revision: https://phabricator.services.mozilla.com/D146986 --- dom/fs/api/FileSystemFileHandle.cpp | 2 +- dom/fs/child/FileSystemManagerChild.cpp | 3 ++ dom/fs/child/FileSystemRequestHandler.cpp | 25 ++++--------- dom/fs/parent/FileSystemManagerParent.cpp | 37 ++++++++++++++++++- .../FileSystemDatabaseManagerVersion001.cpp | 2 +- .../child/TestFileSystemRequestHandler.cpp | 21 ++++++++++- 6 files changed, 67 insertions(+), 23 deletions(-) diff --git a/dom/fs/api/FileSystemFileHandle.cpp b/dom/fs/api/FileSystemFileHandle.cpp index cb47d915f739..c72bd761c9fc 100644 --- a/dom/fs/api/FileSystemFileHandle.cpp +++ b/dom/fs/api/FileSystemFileHandle.cpp @@ -53,7 +53,7 @@ already_AddRefed FileSystemFileHandle::GetFile(ErrorResult& aError) { return nullptr; } - promise->MaybeReject(NS_ERROR_NOT_IMPLEMENTED); + mRequestHandler->GetFile(mManager, mMetadata, promise); return promise.forget(); } diff --git a/dom/fs/child/FileSystemManagerChild.cpp b/dom/fs/child/FileSystemManagerChild.cpp index e59bf31f9da8..a6a4269ae6f0 100644 --- a/dom/fs/child/FileSystemManagerChild.cpp +++ b/dom/fs/child/FileSystemManagerChild.cpp @@ -18,6 +18,9 @@ FileSystemManagerChild::AllocPFileSystemAccessHandleChild( ::mozilla::ipc::IPCResult FileSystemManagerChild::RecvCloseAll( CloseAllResolver&& aResolver) { + // NOTE: getFile() creates blobs that read the data from the child; + // we'll need to abort any reads and resolve this call only when all + // blobs are closed. for (const auto& item : ManagedPFileSystemAccessHandleChild()) { auto* child = static_cast(item); child->Close(); diff --git a/dom/fs/child/FileSystemRequestHandler.cpp b/dom/fs/child/FileSystemRequestHandler.cpp index 373de3523f40..b40331e8c31b 100644 --- a/dom/fs/child/FileSystemRequestHandler.cpp +++ b/dom/fs/child/FileSystemRequestHandler.cpp @@ -8,6 +8,7 @@ #include "FileSystemEntryMetadataArray.h" #include "fs/FileSystemConstants.h" +#include "mozilla/dom/BlobImpl.h" #include "mozilla/dom/File.h" #include "mozilla/dom/FileSystemAccessHandleChild.h" #include "mozilla/dom/FileSystemDirectoryHandle.h" @@ -17,6 +18,7 @@ #include "mozilla/dom/FileSystemManager.h" #include "mozilla/dom/FileSystemManagerChild.h" #include "mozilla/dom/FileSystemSyncAccessHandle.h" +#include "mozilla/dom/IPCBlobUtils.h" #include "mozilla/dom/Promise.h" #include "mozilla/dom/quota/QuotaCommon.h" @@ -35,20 +37,6 @@ using mozilla::ipc::RejectCallback; namespace { -// TODO: This is just a dummy implementation -RefPtr MakeGetFileResult(nsIGlobalObject* aGlobal, const nsString& aName, - const nsString& aType, - int64_t aLastModifiedMilliSeconds, - nsTArray&& aPath, IPCBlob&& /* aFile */, - RefPtr& aManager) { - // TODO: Replace with a real implementation - RefPtr result = File::CreateMemoryFileWithCustomLastModified( - aGlobal, static_cast(new uint8_t[1]), sizeof(uint8_t), aName, - aType, aLastModifiedMilliSeconds); - - return result; -} - bool MakeResolution(nsIGlobalObject* aGlobal, FileSystemGetEntriesResponse&& aResponse, const bool& /* aResult */, @@ -123,10 +111,11 @@ RefPtr MakeResolution(nsIGlobalObject* aGlobal, const Name& aName, RefPtr& aManager) { auto& fileProperties = aResponse.get_FileSystemFileProperties(); - return MakeGetFileResult(aGlobal, aName, fileProperties.type(), - fileProperties.last_modified_ms(), - std::move(fileProperties.path()), - std::move(fileProperties.file()), aManager); + + RefPtr blobImpl = IPCBlobUtils::Deserialize(fileProperties.file()); + MOZ_ASSERT(blobImpl); + RefPtr result = File::Create(aGlobal, blobImpl); + return result; } template diff --git a/dom/fs/parent/FileSystemManagerParent.cpp b/dom/fs/parent/FileSystemManagerParent.cpp index dcf7df9718b0..4b95b9a0718e 100644 --- a/dom/fs/parent/FileSystemManagerParent.cpp +++ b/dom/fs/parent/FileSystemManagerParent.cpp @@ -8,9 +8,11 @@ #include "FileSystemDatabaseManager.h" #include "mozilla/Maybe.h" +#include "mozilla/dom/FileBlobImpl.h" #include "mozilla/dom/FileSystemAccessHandleParent.h" #include "mozilla/dom/FileSystemDataManager.h" #include "mozilla/dom/FileSystemTypes.h" +#include "mozilla/dom/IPCBlobUtils.h" #include "mozilla/dom/QMResult.h" #include "mozilla/dom/quota/ForwardDecls.h" #include "mozilla/dom/quota/QuotaCommon.h" @@ -173,9 +175,40 @@ IPCResult FileSystemManagerParent::RecvGetFile( FileSystemGetFileRequest&& aRequest, GetFileResolver&& aResolver) { AssertIsOnIOTarget(); - FileSystemGetFileResponse response(NS_ERROR_NOT_IMPLEMENTED); - aResolver(response); + // XXX Spec https://www.w3.org/TR/FileAPI/#dfn-file wants us to snapshot the + // state of the file at getFile() time + // You can create a File with getFile() even if the file is locked + // XXX factor out this part of the code for accesshandle/ and getfile + auto reportError = [aResolver](nsresult rv) { + LOG(("getFile() Failed!")); + aResolver(rv); + }; + + nsString type; + fs::TimeStamp lastModifiedMilliSeconds; + fs::Path path; + nsCOMPtr fileObject; + QM_TRY(MOZ_TO_RESULT(mDataManager->MutableDatabaseManagerPtr()->GetFile( + {aRequest.entryId(), aRequest.entryId()}, type, + lastModifiedMilliSeconds, path, fileObject)), + IPC_OK(), reportError); + + if (MOZ_LOG_TEST(gOPFSLog, mozilla::LogLevel::Debug)) { + nsAutoString path; + if (NS_SUCCEEDED(fileObject->GetPath(path))) { + LOG(("Opening %s", NS_ConvertUTF16toUTF8(path).get())); + } + } + + RefPtr blob = MakeRefPtr(fileObject); + + IPCBlob ipcBlob; + QM_TRY(MOZ_TO_RESULT(IPCBlobUtils::Serialize(blob, ipcBlob)), IPC_OK(), + reportError); + + aResolver( + FileSystemFileProperties(lastModifiedMilliSeconds, ipcBlob, type, path)); return IPC_OK(); } diff --git a/dom/fs/parent/datamodel/FileSystemDatabaseManagerVersion001.cpp b/dom/fs/parent/datamodel/FileSystemDatabaseManagerVersion001.cpp index 3a9f7c21e129..4beb802888c3 100644 --- a/dom/fs/parent/datamodel/FileSystemDatabaseManagerVersion001.cpp +++ b/dom/fs/parent/datamodel/FileSystemDatabaseManagerVersion001.cpp @@ -160,7 +160,7 @@ nsresult GetFileAttributes(const FileSystemConnection& aConnection, const nsLiteralCString getFileLocation = "SELECT type FROM Files INNER JOIN Entries USING(handle) WHERE handle = :entryId;"_ns; - // TODO: Request this from filemanager who makes the system call + // TODO: bug 1790391: Request this from filemanager who makes the system call aLastModifiedMilliSeconds = 0; QM_TRY_UNWRAP(ResultStatement stmt, diff --git a/dom/fs/test/gtest/child/TestFileSystemRequestHandler.cpp b/dom/fs/test/gtest/child/TestFileSystemRequestHandler.cpp index 39147ffe9386..b17a8e6260eb 100644 --- a/dom/fs/test/gtest/child/TestFileSystemRequestHandler.cpp +++ b/dom/fs/test/gtest/child/TestFileSystemRequestHandler.cpp @@ -11,13 +11,17 @@ #include "gtest/gtest.h" #include "mozilla/SpinEventLoopUntil.h" #include "mozilla/UniquePtr.h" +#include "mozilla/dom/FileBlobImpl.h" #include "mozilla/dom/FileSystemManager.h" #include "mozilla/dom/FileSystemManagerChild.h" #include "mozilla/dom/IPCBlob.h" +#include "mozilla/dom/IPCBlobUtils.h" #include "mozilla/dom/PFileSystemManager.h" #include "mozilla/dom/StorageManager.h" #include "mozilla/ipc/FileDescriptorUtils.h" #include "mozilla/ipc/IPCCore.h" +#include "nsDirectoryServiceDefs.h" +#include "nsIFile.h" using ::testing::_; using ::testing::ByRef; @@ -143,9 +147,24 @@ TEST_F(TestFileSystemRequestHandler, isGetFileHandleSuccessful) { TEST_F(TestFileSystemRequestHandler, isGetFileSuccessful) { auto fakeResponse = [](const auto& /* aRequest */, auto&& aResolve, auto&& /* aReject */) { + // We have to create a temporary file + nsCOMPtr tmpfile; + nsresult rv = + NS_GetSpecialDirectory(NS_OS_TEMP_DIR, getter_AddRefs(tmpfile)); + ASSERT_EQ(NS_SUCCEEDED(rv), true); + + rv = tmpfile->AppendNative("GetFileTestBlob"_ns); + ASSERT_EQ(NS_SUCCEEDED(rv), true); + + rv = tmpfile->CreateUnique(nsIFile::NORMAL_FILE_TYPE, 0666); + ASSERT_EQ(NS_SUCCEEDED(rv), true); + + auto blob = MakeRefPtr(tmpfile); + TimeStamp last_modified_ms = 0; - mozilla::dom::IPCBlob file; ContentType type = u"txt"_ns; + IPCBlob file; + IPCBlobUtils::Serialize(blob, file); nsTArray path; path.AppendElement(u"root"_ns);