Bug 176165 - Implement FileSystemEntry.getParent()
Summary: Implement FileSystemEntry.getParent()
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:44 PDT by Chris Dumez
Modified: 2017-09-01 10:37 PDT (History)
11 users (show)

See Also:


Attachments
Patch (36.88 KB, patch)
2017-08-31 19:11 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (36.94 KB, patch)
2017-09-01 09:59 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (36.94 KB, patch)
2017-09-01 10:14 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: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.