[webkit-reviews] review denied: [Bug 56704] [fileapi/chromium] Fetch platform path using GetMetadata before creating File from FileEntry* : [Attachment 86338] Fixed the last style failure.

[bugzilla-daemon at webkit.org]

David Levin <levin at chromium.org> has denied Eric U. <ericu at chromium.org>'s
request for review:
Bug 56704: [fileapi/chromium] Fetch platform path using GetMetadata before
creating File from FileEntry*
https://bugs.webkit.org/show_bug.cgi?id=56704

Attachment 86338: Fixed the last style failure.
https://bugs.webkit.org/attachment.cgi?id=86338&action=review

------- Additional Comments from David Levin <levin at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=86338&action=review

> WebCore/fileapi/DOMFileSystemSync.cpp:79
> +	   bool m_failed;

Usually member variables come after methods.

> WebCore/fileapi/DOMFileSystemSync.cpp:85
> +	     return adoptRef(new GetPathResult());

two spaces.

> WebCore/fileapi/DOMFileSystemSync.cpp:92
> +	   }

Usually a blank line here.

> WebCore/fileapi/DOMFileSystemSync.cpp:99
> +    static PassOwnPtr<GetPathHelper> create(GetPathResult* result)

Should be PassRefPtr<GetPathResult> result

> WebCore/fileapi/DOMFileSystemSync.cpp:129
> +    // Called when there was an error.

Does this comment add any information? (The method name is didFail.)

> WebCore/fileapi/DOMFileSystemSync.cpp:160
> +    m_asyncFileSystem->readMetadata(platformPath,
GetPathHelper::create(result.get()));

I doubt you need result.get().

> WebCore/fileapi/DOMFileSystem.cpp:119
> +    static PassOwnPtr<GetPathCallback> create(DOMFileSystem* filesystem,
const String& path, PassRefPtr<FileCallback> successCallback,
PassRefPtr<ErrorCallback> errorCallback)

Since you put filesystem in a RefPtr, it should be passed in via a PassRefPtr.

> WebCore/fileapi/DOMFileSystem.cpp:139
> +    }

Usually a blank line here.

> WebCore/fileapi/DOMFileSystem.cpp:142
> +    PassRefPtr<FileCallback> m_successCallback;

Member variables should be RefPtr not PassRefPtr.