Bug 176168 - Implement FileSystemDirectoryEntry.getDirectory()
Summary: Implement FileSystemDirectoryEntry.getDirectory()
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL: https://wicg.github.io/entries-api/#d...
Keywords: InRadar
Depends on:
Blocks: 175976
  Show dependency treegraph
 
Reported: 2017-08-31 09:47 PDT by Chris Dumez
Modified: 2017-09-02 14:31 PDT (History)
8 users (show)

See Also:


Attachments
Patch (22.92 KB, patch)
2017-09-01 21:21 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2017-08-31 09:47:55 PDT
Implement FileSystemDirectoryEntry.getDirectory():
- https://wicg.github.io/entries-api/#dom-filesystemdirectoryentry-getdirectory
Comment 1 Radar WebKit Bug Importer 2017-08-31 09:48:07 PDT
<rdar://problem/34187787>
Comment 2 Chris Dumez 2017-09-01 21:21:03 PDT
Created attachment 319690 [details]
Patch
Comment 3 Build Bot 2017-09-01 21:22:52 PDT
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.
Comment 4 Darin Adler 2017-09-02 10:59:54 PDT
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?
Comment 5 Chris Dumez 2017-09-02 14:02:43 PDT
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).
Comment 6 WebKit Commit Bot 2017-09-02 14:31:53 PDT
Comment on attachment 319690 [details]
Patch

Clearing flags on attachment: 319690

Committed r221540: <http://trac.webkit.org/changeset/221540>
Comment 7 WebKit Commit Bot 2017-09-02 14:31:55 PDT
All reviewed patches have been landed.  Closing bug.