[webkit-reviews] review granted: [Bug 176091] Implement FileSystemDirectoryReader.readEntries() : [Attachment 319385] Patch

[bugzilla-daemon at webkit.org]

Andreas Kling <akling at apple.com> has granted Chris Dumez <cdumez at apple.com>'s
request for review:
Bug 176091: Implement FileSystemDirectoryReader.readEntries()
https://bugs.webkit.org/show_bug.cgi?id=176091

Attachment 319385: Patch

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




--- Comment #9 from Andreas Kling <akling at apple.com> ---
Comment on attachment 319385
  --> https://bugs.webkit.org/attachment.cgi?id=319385
Patch

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

r=me

> Source/WebCore/Modules/entriesapi/DOMFileSystem.cpp:53
> +    Vector<String> childPaths = listDirectory(fullPath, "*");

auto

> Source/WebCore/Modules/entriesapi/DOMFileSystem.cpp:119
> +    Vector<String> components;
> +    virtualPath.split('/', false /* allowEmpty */, components);

auto + overload of split() would look nice :)

> Source/WebCore/Modules/entriesapi/DOMFileSystem.cpp:157
> +	   callOnMainThread([this, completionHandler =
WTFMove(completionHandler), listedChildren =
CrossThreadCopier<ExceptionOr<Vector<ListedChild>>>::copy(listedChildren),
directoryPtr]() mutable {

It would be cool to have a makeCrossThreadCopy() helper.

> Source/WebCore/Modules/entriesapi/FileSystemDirectoryReader.cpp:80
> +    setPendingActivity(this);

I think it would be nice to use a RAII helper object for this so we don't have
to unset in every possible code path.

>
LayoutTests/editing/pasteboard/datatransfer-items-drop-directoryReader-root.htm
l:16
> +function moveMouseToCenterOfElement(element) {
> +    var centerX = element.offsetLeft + element.offsetWidth / 2;
> +    var centerY = element.offsetTop + element.offsetHeight / 2;
> +    eventSender.mouseMoveTo(centerX, centerY);
> +}

This seems like a very useful function, maybe we could put it somewhere shared
so other tests can use it too?

>
LayoutTests/editing/pasteboard/datatransfer-items-drop-directoryReader.html:16
> +function moveMouseToCenterOfElement(element) {
> +    var centerX = element.offsetLeft + element.offsetWidth / 2;
> +    var centerY = element.offsetTop + element.offsetHeight / 2;
> +    eventSender.mouseMoveTo(centerX, centerY);
> +}

This seems like a very useful function, maybe we could put it somewhere shared
so other tests can use it too?