[Webkit-unassigned] [Bug 54774] [fileapi] Implement LocalFileSystem.resolveFileSystemURI

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=54774





--- Comment #15 from Adam Klein <adamk at chromium.org>  2011-02-25 11:09:55 PST ---
(From update of attachment 83740)
View in context: https://bugs.webkit.org/attachment.cgi?id=83740&action=review

>> LayoutTests/ChangeLog:34
>> +        (var):
> 
> I would just remove this boilerplate from the changelog.  It's not really helping anyone.

Removed.

>> Source/WebCore/fileapi/DOMFileSystemBase.cpp:68
>> +    if (path.startsWith(kTemporaryPathPrefix)) {
> 
> Can you give an offset to startsWith to avoid the extra malloc from creating the substring?

I don't see anything in WTF::String matching std::string::compare(), which is what I'd need. I could play some tricks with characters(), memcmp(), and min(), if you think it's worth the loss of clarity.

>> Source/WebCore/fileapi/FileSystemCallbacks.cpp:195
>> +// ResolveURICallbacks --------------------------------------------------------
> 
> We don't usually include these sorts of comments.

As you see below, these are just matching the rest of the file.  Let me know if you think they should just be removed.

>> Source/WebCore/fileapi/FileSystemCallbacks.cpp:223
>> +bool ErrorCallbackWrapper::handleEvent(FileError*)
> 
> The error doesn't matter?

Good point, I've changed this to only look retry with getFile if the error is TYPE_MISMATCH_ERR, which the spec says _must_ be returned in the case we're worried about here.

But it turns out Chromium does this wrong and returns INVALID_STATE_ERR.  I've added a FIXME here and will clean this up in a future change.

>> Source/WebCore/fileapi/FileSystemCallbacks.cpp:229
>> +} // namespace (anonymous)
> 
> Usually we would omit the (anonymous) part of this comment.

Removed.

>> Source/WebCore/page/DOMWindow.cpp:765
>> +    KURL parsedURL(ParsedURLString, uri);
> 
> This still seems strange to me.  Everywhere else we call competeURL.  If this is what the spec says, it's probably wrong.  :(

Eric, can you comment here?  Are these URIs allowed to be relative?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.