Bug 176165

Summary: Implement FileSystemEntry.getParent()
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: WebCore Misc.Assignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, commit-queue, dbates, esprehn+autocc, ggaren, kangil.han, kling, kondapallykalyan, rniwa, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
URL: https://wicg.github.io/entries-api/#dom-filesystementry-getparent
Bug Depends on:    
Bug Blocks: 175976    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Chris Dumez 2017-08-31 09:44:59 PDT
Implement FileSystemEntry.getParent():
- https://wicg.github.io/entries-api/#dom-filesystementry-getparent
Comment 1 Radar WebKit Bug Importer 2017-08-31 09:45:30 PDT
<rdar://problem/34187743>
Comment 2 Chris Dumez 2017-08-31 19:11:59 PDT
Created attachment 319564 [details]
Patch
Comment 3 Andreas Kling 2017-09-01 09:54:51 PDT
Comment on attachment 319564 [details]
Patch

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

> Source/WebCore/Modules/entriesapi/DOMFileSystem.cpp:92
> +    return !virtualPath.isEmpty() && virtualPath[0] == '/';

The isEmpty() check here is redundant but I hate that operator[] API returns a null character for invalid indices so let's not even care about that.

> Source/WebCore/Modules/entriesapi/FileSystemEntry.cpp:67
> +void FileSystemEntry::getParent(ScriptExecutionContext& context, RefPtr<FileSystemEntryCallback>&& successCallback, RefPtr<ErrorCallback>&& errorCallback)

If we have neither successCallback or errorCallback, we could just early return here, no need to do work. Silly that the API allows this at all.

> Source/WebCore/Modules/entriesapi/FileSystemEntry.cpp:74
> +        if (result.hasException()) {
> +            if (errorCallback)
> +                errorCallback->handleEvent(DOMException::create(result.releaseException()));
> +            return;
> +        }

This is not covered by the layout tests.
Comment 4 Chris Dumez 2017-09-01 09:59:06 PDT
Created attachment 319607 [details]
Patch
Comment 5 WebKit Commit Bot 2017-09-01 10:01:56 PDT
Comment on attachment 319607 [details]
Patch

Rejecting attachment 319607 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'validate-changelog', '--check-oops', '--non-interactive', 319607, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Ansdreas Kling found in /Volumes/Data/EWS/WebKit/LayoutTests/ChangeLog does not appear to be a valid reviewer according to contributors.json.
/Volumes/Data/EWS/WebKit/LayoutTests/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: http://webkit-queues.webkit.org/results/4427131
Comment 6 Chris Dumez 2017-09-01 10:14:20 PDT
Created attachment 319609 [details]
Patch
Comment 7 Andreas Kling 2017-09-01 10:20:51 PDT
r=hans
Comment 8 WebKit Commit Bot 2017-09-01 10:37:04 PDT
Comment on attachment 319609 [details]
Patch

Clearing flags on attachment: 319609

Committed r221481: <http://trac.webkit.org/changeset/221481>
Comment 9 WebKit Commit Bot 2017-09-01 10:37:06 PDT
All reviewed patches have been landed.  Closing bug.