Bug 176168 – Implement FileSystemDirectoryEntry.getDirectory()

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Chris Dumez]

Implement FileSystemDirectoryEntry.getDirectory():
- https://wicg.github.io/entries-api/#dom-filesystemdirectoryentry-getdirectory

[Radar WebKit Bug Importer]

<rdar://problem/34187787>

[Chris Dumez]

Created attachment 319690 [details]
Patch

[Build Bot]

Attachment 319690 [details] did not pass style-queue:


ERROR: Source/WebCore/Modules/entriesapi/FileSystemDirectoryEntry.cpp:73:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/Modules/entriesapi/FileSystemDirectoryEntry.cpp:78:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 2 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Darin Adler]

Comment on attachment 319690 [details]
Patch

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

> Source/WebCore/Modules/entriesapi/DOMFileSystem.cpp:288
> +        callOnMainThread([this, context = makeRef(context), completionCallback = WTFMove(completionCallback)]() mutable {

Why is it safe to call on main thread without a ref/deref to make sure DOMFileSystem ("this") is still alive?

> Source/WebCore/Modules/entriesapi/DOMFileSystem.cpp:297
> +        callOnMainThread([this, context = WTFMove(context), resolvedVirtualPath = crossThreadCopy(resolvedVirtualPath), entryType = metadata.type, completionCallback = WTFMove(completionCallback)]() mutable {

Ditto.

> Source/WebCore/Modules/entriesapi/FileSystemDirectoryEntry.cpp:54
> +    filesystem().getEntry(context, *this, path, flags, [this, pendingActivity = makePendingActivity(*this), matches = WTFMove(matches), successCallback = WTFMove(successCallback), errorCallback = WTFMove(errorCallback)](auto&& result) {

No need to ref/deref "this" here?

[Chris Dumez]

Comment on attachment 319690 [details]
Patch

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

>> Source/WebCore/Modules/entriesapi/DOMFileSystem.cpp:288
>> +        callOnMainThread([this, context = makeRef(context), completionCallback = WTFMove(completionCallback)]() mutable {
> 
> Why is it safe to call on main thread without a ref/deref to make sure DOMFileSystem ("this") is still alive?

The file and directory entries hold a strong reference to their DOMFileSystem. Those operations (such as getFile) are requested by those entries. The file and directory entries make sure they stay alive during async operations via ActiveDOMObject's PendingActivity and therefore keep their DOMFileSystem alive during the async operation.

>> Source/WebCore/Modules/entriesapi/DOMFileSystem.cpp:297
>> +        callOnMainThread([this, context = WTFMove(context), resolvedVirtualPath = crossThreadCopy(resolvedVirtualPath), entryType = metadata.type, completionCallback = WTFMove(completionCallback)]() mutable {
> 
> Ditto.

Same comment as above. until the completionCallback is called, |this| is kept alive by the client (directory or file entry).

>> Source/WebCore/Modules/entriesapi/FileSystemDirectoryEntry.cpp:54
>> +    filesystem().getEntry(context, *this, path, flags, [this, pendingActivity = makePendingActivity(*this), matches = WTFMove(matches), successCallback = WTFMove(successCallback), errorCallback = WTFMove(errorCallback)](auto&& result) {
> 
> No need to ref/deref "this" here?

pendingActivity = makePendingActivity(*this) keeps us and our wrapper alive until destroyed (i.e. when the async operation finishes).

[WebKit Commit Bot]

Comment on attachment 319690 [details]
Patch

Clearing flags on attachment: 319690

Committed r221540: <http://trac.webkit.org/changeset/221540>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.