[webkit-reviews] review granted: [Bug 225289] Restore pre-r276879 behavior for FileSystem::deleteFile() and FileSystem::deleteEmptyDirectory() : [Attachment 427558] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Chris Dumez <cdumez at apple.com>'s
request for review:
Bug 225289: Restore pre-r276879 behavior for FileSystem::deleteFile() and
FileSystem::deleteEmptyDirectory()
https://bugs.webkit.org/show_bug.cgi?id=225289

Attachment 427558: Patch

https://bugs.webkit.org/attachment.cgi?id=427558&action=review




--- Comment #3 from Darin Adler <darin at apple.com> ---
Comment on attachment 427558
  --> https://bugs.webkit.org/attachment.cgi?id=427558
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=427558&action=review

> Source/WTF/wtf/FileSystem.cpp:493
>  bool deleteFile(const String& path)

Maybe later we want to add a new call that matches more directly what
std::filesystem::remove does, and remove these? I don’t think all callers
*need* to do the "only do it if it’s a file/directory" version; it’s just that
before we were starting by mirroring that feature of the POSIX API where unlink
is for files only and rmdir is for directories only. Now we are in a funny
place where we are more work for a semantic callers might not need.

> Source/WTF/wtf/FileSystem.cpp:506
>  bool deleteEmptyDirectory(const String& path)

Given the complexity of all the ".DS_Store" stuff on Mac, maybe we should
consider eventually getting rid of deleteEmptyDirectory to instead use a
function that can delete even non-empty ones. I suppose it’s a safety measure
to prevent deleting a lot of files that we use deleteEmptyDirectory.