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

[bugzilla-daemon at webkit.org]

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





--- Comment #12 from Adam Klein <adamk at chromium.org>  2011-02-24 16:06:10 PST ---
(From update of attachment 83404)
View in context: https://bugs.webkit.org/attachment.cgi?id=83404&action=review

>> Source/WebCore/fileapi/DOMFileSystemBase.cpp:57
>> +bool DOMFileSystemBase::crackFileSystemURL(const KURL& url, AsyncFileSystem::Type& type, String& filePath)
> 
> We don't tend to use non-const reference parameters in the filesystem code.  It's not explicitly banned by the webkit style guide, but we prefer to use pointers for output parameters.

I was doing this to match what I perceived as WebKit style: there's no reference counting going on here, and these should never be null. So references seem appropriate.  Per a conversation just now on #webkit:

<aklein> what's the preferred style for "out" parameters in WebKit? it seems references are pretty common, are they preferred over pointers?
<abarth> aklein: yes

>> Source/WebCore/fileapi/FileSystemCallbacks.cpp:248
>> +    if (m_successCallback) {
> 
> I think we do not need m_successCallback check here (and line 225 above).  If the user only provides errorCallback (it doesn't make sense but technically possible) the current code doesn't work as expected.
> (Or we can skip the entire operation if neither callbacks are given, as it's read-only operation.)
> 
> 
> As for readFileSystem, no I don't think there's any other call site. We used to have some code for devtools but the code was reverted due to devtool code refactoring.
> If you're going to upload another patch please feel free to drop the size parameter.

Removed checks for m_successCallback (and calls to clear()).

And removed "size" argument to readFileSystem(), hiding the special-case of 0 inside LocalFileSystemChromium

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