[webkit-reviews] review granted: [Bug 176166] Implement FileSystemFileEntry.file() : [Attachment 319752] Patch

[bugzilla-daemon at webkit.org]

Sam Weinig <sam at webkit.org> has granted Chris Dumez <cdumez at apple.com>'s
request for review:
Bug 176166: Implement FileSystemFileEntry.file()
https://bugs.webkit.org/show_bug.cgi?id=176166

Attachment 319752: Patch

https://bugs.webkit.org/attachment.cgi?id=319752&action=review




--- Comment #3 from Sam Weinig <sam at webkit.org> ---
Comment on attachment 319752
  --> https://bugs.webkit.org/attachment.cgi?id=319752
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=319752&action=review

> Source/WebCore/Modules/entriesapi/DOMFileSystem.cpp:168
> +    if (!getFileMetadata(fullPath, metadata, ShouldFollowSymbolicLinks::No))
> +	   return Exception { NotFoundError, ASCIILiteral("Path does not
exist") };

Is it time we had a getFileMetadata that returned an
std::optional<FileMetadata> or Expected<FileMetadata>?

> Source/WebCore/Modules/entriesapi/DOMFileSystem.cpp:321
> +    String virtualPath = fileEntry.virtualPath();
> +    String fullPath = evaluatePath(virtualPath);

could use auto here.

> Source/WebCore/Modules/entriesapi/FileSystemFileEntry.cpp:53
> +	   if (successCallback)
> +	       successCallback->handleEvent(result.releaseReturnValue());

I'm possibly looking at the wrong version of the spec,
https://wicg.github.io/entries-api/#api-fileentry, but it looks to me like
success callback should not be nullable, so I think you can update the IDL and
remove this null check.