[webkit-reviews] review denied: [Bug 79193] [EFL] Implements FileSystem on EFL port : [Attachment 165121] First patch

[bugzilla-daemon at webkit.org]

Kenneth Rohde Christiansen <kenneth at webkit.org> has denied Dongwoo Joshua Im
(dwim) <dw.im at samsung.com>'s request for review:
Bug 79193: [EFL] Implements FileSystem on EFL port
https://bugs.webkit.org/show_bug.cgi?id=79193

Attachment 165121: First patch
https://bugs.webkit.org/attachment.cgi?id=165121&action=review

------- Additional Comments from Kenneth Rohde Christiansen
<kenneth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=165121&action=review


This is a huge patch backed by no tests whatsoever. Can it be split into minor
patches with actual tests?

> Source/WebCore/ChangeLog:17
> +	   (WebCore::AsyncFileSystem::deleteFileSystem):
> +	   (WebCore::LocalFileSystem::deleteFileSystem):

No comment why you changed the RefPtr to an OwnPtr

> Source/WebCore/platform/efl/AsyncFileSystemCallbacksEfl.cpp:138
> +void AsyncFileSystemCallbacksEfl::didReadDirectoryEntries(bool hasMore)
> +{
> +    if (hasMore)
> +	   ASSERT_NOT_REACHED();

Why does it have the argument then?

> Source/WebCore/platform/efl/AsyncFileSystemEfl.cpp:52
> +    storageIdentifier = storageIdentifier + ":";

+ ':'

> Source/WebCore/platform/efl/AsyncFileSystemEfl.cpp:56
> +    rootURL.append("filesystem:");

appendLiteral

> Source/WebCore/platform/efl/AsyncFileSystemEfl.cpp:60
> +    rootURL.append(typeString + "/" + identifier + "/");

'/' should be faster