Skip to content

Commit

Permalink
FileSystemSyncAccessHandle::{read,write}'s option should be optional
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=249631
rdar://103541719

Reviewed by Justin Michaud.

The current FileSystemSyncAccessHandle::{read,write} implementation has a bug that `options` parameter is specified as mandatory.
As a result, these APIs throw errors because it is using `read` / `write` functions without options.
This patch fixes the implementation to make them optional.

When optional `at` is not found, then the behavior is using the current file position's cursor. Thus, we just skip seekFile if
it is not specified.

* LayoutTests/storage/filesystemaccess/resources/sync-access-handle-read-write.js:
(write):
(read):
(async test):
* LayoutTests/storage/filesystemaccess/sync-access-handle-read-write-worker-expected.txt:
* Source/WebCore/Modules/filesystemaccess/FileSystemSyncAccessHandle.cpp:
(WebCore::FileSystemSyncAccessHandle::read):
(WebCore::FileSystemSyncAccessHandle::write):
* Source/WebCore/Modules/filesystemaccess/FileSystemSyncAccessHandle.h:
* Source/WebCore/Modules/filesystemaccess/FileSystemSyncAccessHandle.idl:

Canonical link: https://commits.webkit.org/258160@main
  • Loading branch information
Constellation committed Dec 20, 2022
1 parent 4d0e3a4 commit 0834303
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 13 deletions.
Expand Up @@ -24,15 +24,21 @@ function stringToArrayBuffer(string)
function write(accessHandle, offset, text)
{
writeBuffer = stringToArrayBuffer(text);
writeSize = accessHandle.write(writeBuffer, { "at" : offset });
if (offset == null)
writeSize = accessHandle.write(writeBuffer);
else
writeSize = accessHandle.write(writeBuffer, { "at" : offset });
shouldBe("writeSize", "writeBuffer.byteLength");
return writeSize;
}

function read(accessHandle, offset, size, expectedString)
{
readBuffer = new ArrayBuffer(size);
readSize = accessHandle.read(readBuffer, { "at": offset });
if (offset == null)
readSize = accessHandle.read(readBuffer);
else
readSize = accessHandle.read(readBuffer, { "at": offset });
shouldBe("readSize", "readBuffer.byteLength");
if (expectedString) {
readText = arrayBufferToString(readBuffer);
Expand Down Expand Up @@ -60,6 +66,11 @@ async function test()
read(accessHandle, 0, totalWriteSize, "This is first sentence.This is second sentence.");

// Wrong offset to read and write.
totalReadSize = totalWriteSize;
// File cursor should be updated by last read(), so the next write() should append to file.
totalWriteSize += write(accessHandle, null, "This is third sentence.");
read(accessHandle, 0, totalReadSize, "This is first sentence.This is second sentence.");
read(accessHandle, null, totalWriteSize - totalReadSize, "This is third sentence.");
shouldThrow("accessHandle.read(new ArrayBuffer(1), { \"at\" : Number.MAX_SAFE_INTEGER })");
shouldThrow("accessHandle.write(new ArrayBuffer(1), { \"at\" : Number.MAX_SAFE_INTEGER })");

Expand Down
Expand Up @@ -14,6 +14,11 @@ PASS [Worker] readSize is readBuffer.byteLength
PASS [Worker] readText is "This is second sentence."
PASS [Worker] readSize is readBuffer.byteLength
PASS [Worker] readText is "This is first sentence.This is second sentence."
PASS [Worker] writeSize is writeBuffer.byteLength
PASS [Worker] readSize is readBuffer.byteLength
PASS [Worker] readText is "This is first sentence.This is second sentence."
PASS [Worker] readSize is readBuffer.byteLength
PASS [Worker] readText is "This is third sentence."
PASS [Worker] accessHandle.read(new ArrayBuffer(1), { "at" : Number.MAX_SAFE_INTEGER }) threw exception InvalidStateError: Failed to read at offset.
PASS [Worker] accessHandle.write(new ArrayBuffer(1), { "at" : Number.MAX_SAFE_INTEGER }) threw exception InvalidStateError: Failed to write at offset.
[Worker] test flush():
Expand Down
Expand Up @@ -211,11 +211,13 @@ ExceptionOr<unsigned long long> FileSystemSyncAccessHandle::read(BufferSource&&
if (!m_pendingPromises.isEmpty())
return Exception { InvalidStateError, "Access handle has unfinished operation"_s };

int result = FileSystem::seekFile(m_file.handle(), options.at, FileSystem::FileSeekOrigin::Beginning);
if (result == -1)
return Exception { InvalidStateError, "Failed to read at offset"_s };
if (options.at) {
int result = FileSystem::seekFile(m_file.handle(), options.at.value(), FileSystem::FileSeekOrigin::Beginning);
if (result == -1)
return Exception { InvalidStateError, "Failed to read at offset"_s };
}

result = FileSystem::readFromFile(m_file.handle(), buffer.mutableData(), buffer.length());
int result = FileSystem::readFromFile(m_file.handle(), buffer.mutableData(), buffer.length());
if (result == -1)
return Exception { InvalidStateError, "Failed to read from file"_s };

Expand All @@ -232,11 +234,13 @@ ExceptionOr<unsigned long long> FileSystemSyncAccessHandle::write(BufferSource&&
if (!m_pendingPromises.isEmpty())
return Exception { InvalidStateError, "Access handle has unfinished operation"_s };

int result = FileSystem::seekFile(m_file.handle(), options.at, FileSystem::FileSeekOrigin::Beginning);
if (result == -1)
return Exception { InvalidStateError, "Failed to write at offset"_s };
if (options.at) {
int result = FileSystem::seekFile(m_file.handle(), options.at.value(), FileSystem::FileSeekOrigin::Beginning);
if (result == -1)
return Exception { InvalidStateError, "Failed to write at offset"_s };
}

result = FileSystem::writeToFile(m_file.handle(), buffer.data(), buffer.length());
int result = FileSystem::writeToFile(m_file.handle(), buffer.data(), buffer.length());
if (result == -1)
return Exception { InvalidStateError, "Failed to write to file"_s };

Expand Down
Expand Up @@ -43,7 +43,7 @@ template<typename> class DOMPromiseDeferred;
class FileSystemSyncAccessHandle : public ActiveDOMObject, public RefCounted<FileSystemSyncAccessHandle>, public CanMakeWeakPtr<FileSystemSyncAccessHandle> {
public:
struct FilesystemReadWriteOptions {
unsigned long long at;
std::optional<unsigned long long> at;
};

static Ref<FileSystemSyncAccessHandle> create(ScriptExecutionContext&, FileSystemFileHandle&, FileSystemSyncAccessHandleIdentifier, FileHandle&&);
Expand Down
Expand Up @@ -33,8 +33,8 @@
Promise<unsigned long long> getSize();
Promise<undefined> flush();
Promise<undefined> close();
unsigned long long read([AllowShared] (ArrayBufferView or ArrayBuffer) buffer, FilesystemReadWriteOptions options);
unsigned long long write([AllowShared] (ArrayBufferView or ArrayBuffer) buffer, FilesystemReadWriteOptions options);
unsigned long long read([AllowShared] (ArrayBufferView or ArrayBuffer) buffer, optional FilesystemReadWriteOptions options = {});
unsigned long long write([AllowShared] (ArrayBufferView or ArrayBuffer) buffer, optional FilesystemReadWriteOptions options = {});
};

dictionary FilesystemReadWriteOptions {
Expand Down

0 comments on commit 0834303

Please sign in to comment.