Bug 225820

Summary: Drop FileSystem::fileMetadata() / fileMetadataFollowingSymlinks()
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: Web Template FrameworkAssignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, annulen, benjamin, cgarcia, changseok, cmarcelo, darin, eric.carlson, esprehn+autocc, ews-watchlist, ggaren, glenn, gyuyoung.kim, jer.noble, keith_miller, mark.lam, mifenton, msaboff, philipj, ryuan.choi, saam, sam, sergio, tzagallo, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
ews-feeder: commit-queue-
Patch
none
Patch
none
Patch none

Description Chris Dumez 2021-05-14 13:40:53 PDT
Drop FileSystem::fileMetadata() / fileMetadataFollowingSymlinks(). Those don't match very closely the std::filesystem API and they are not very efficient because we gather several attributes of a file but the callers are usually only interested in one thing (e.g. file type or file size).
Comment 1 Chris Dumez 2021-05-14 13:46:39 PDT
Created attachment 428652 [details]
Patch
Comment 2 Chris Dumez 2021-05-14 14:37:01 PDT
Created attachment 428660 [details]
Patch
Comment 3 Chris Dumez 2021-05-14 16:55:26 PDT
Created attachment 428685 [details]
Patch
Comment 4 Darin Adler 2021-05-14 20:20:06 PDT
Comment on attachment 428685 [details]
Patch

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

> Source/WebCore/editing/cocoa/WebContentReaderCocoa.mm:716
> +    auto fileType = FileSystem::fileTypeFollowingSymlinks(path);
> +    bool isDirectory = fileType && *fileType == FileSystem::FileType::Directory;

The == operator in Optional should make this easier to write. Like this:

    bool isDirectory = FileSystem::fileTypeFollowingSymlinks(path) == FileSystem::FileType::Directory;

> Source/WebCore/fileapi/File.cpp:151
> +        auto fileType = FileSystem::fileTypeFollowingSymlinks(m_path);
> +        m_isDirectory = fileType && *fileType == FileSystem::FileType::Directory;

m_isDirectory = FileSystem::fileTypeFollowingSymlinks(m_path) == FileSystem::FileType::Directory;

> Source/WebCore/html/DirectoryFileListCreator.cpp:79
> +        auto fileType = FileSystem::fileType(info.path);
> +        if (fileType && *fileType == FileSystem::FileType::Directory)

if (FileSystem::fileType(info.path) == FileSystem::FileType::Directory)

> Source/WebKit/NetworkProcess/cache/NetworkCacheFileSystem.cpp:62
> +        auto fileType = FileSystem::fileType(entryPath);
> +        auto type = (fileType && *fileType == FileSystem::FileType::Directory) ? DirectoryEntryType::Directory : DirectoryEntryType::File;

auto type = FileSystem::fileType(entryPath) == FileSystem::FileType::Directory ? DirectoryEntryType::Directory : DirectoryEntryType::File;

> Source/WebKit/Shared/mac/AuxiliaryProcessMac.mm:368
> +        auto directoryType = FileSystem::fileType(info.directoryPath);
> +        if (directoryType && *directoryType == FileSystem::FileType::Directory) {

if (FileSystem::fileType(info.directoryPath) == FileSystem::FileType::Directory) {

> Source/WebKit/Shared/mac/AuxiliaryProcessMac.mm:392
> +            auto fileType = FileSystem::fileTypeFollowingSymlinks(info.directoryPath);
> +            hasSandboxDirectory = fileType && *fileType == FileSystem::FileType::Directory;

hasSandboxDirectory = FileSystem::fileTypeFollowingSymlinks(info.directoryPath) == FileSystem::FileType::Directory;
Comment 5 Chris Dumez 2021-05-14 20:24:05 PDT
(In reply to Darin Adler from comment #4)
> Comment on attachment 428685 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=428685&action=review
> 
> > Source/WebCore/editing/cocoa/WebContentReaderCocoa.mm:716
> > +    auto fileType = FileSystem::fileTypeFollowingSymlinks(path);
> > +    bool isDirectory = fileType && *fileType == FileSystem::FileType::Directory;
> 
> The == operator in Optional should make this easier to write. Like this:
> 
>     bool isDirectory = FileSystem::fileTypeFollowingSymlinks(path) ==
> FileSystem::FileType::Directory;
> 
> > Source/WebCore/fileapi/File.cpp:151
> > +        auto fileType = FileSystem::fileTypeFollowingSymlinks(m_path);
> > +        m_isDirectory = fileType && *fileType == FileSystem::FileType::Directory;
> 
> m_isDirectory = FileSystem::fileTypeFollowingSymlinks(m_path) ==
> FileSystem::FileType::Directory;
> 
> > Source/WebCore/html/DirectoryFileListCreator.cpp:79
> > +        auto fileType = FileSystem::fileType(info.path);
> > +        if (fileType && *fileType == FileSystem::FileType::Directory)
> 
> if (FileSystem::fileType(info.path) == FileSystem::FileType::Directory)
> 
> > Source/WebKit/NetworkProcess/cache/NetworkCacheFileSystem.cpp:62
> > +        auto fileType = FileSystem::fileType(entryPath);
> > +        auto type = (fileType && *fileType == FileSystem::FileType::Directory) ? DirectoryEntryType::Directory : DirectoryEntryType::File;
> 
> auto type = FileSystem::fileType(entryPath) ==
> FileSystem::FileType::Directory ? DirectoryEntryType::Directory :
> DirectoryEntryType::File;
> 
> > Source/WebKit/Shared/mac/AuxiliaryProcessMac.mm:368
> > +        auto directoryType = FileSystem::fileType(info.directoryPath);
> > +        if (directoryType && *directoryType == FileSystem::FileType::Directory) {
> 
> if (FileSystem::fileType(info.directoryPath) ==
> FileSystem::FileType::Directory) {
> 
> > Source/WebKit/Shared/mac/AuxiliaryProcessMac.mm:392
> > +            auto fileType = FileSystem::fileTypeFollowingSymlinks(info.directoryPath);
> > +            hasSandboxDirectory = fileType && *fileType == FileSystem::FileType::Directory;
> 
> hasSandboxDirectory =
> FileSystem::fileTypeFollowingSymlinks(info.directoryPath) ==
> FileSystem::FileType::Directory;

Oh that’s great. I did not realize I could do that. This should make things a lot nicer.
Comment 6 Chris Dumez 2021-05-14 21:03:52 PDT
Created attachment 428711 [details]
Patch
Comment 7 EWS 2021-05-14 22:46:50 PDT
Committed r277535 (237763@main): <https://commits.webkit.org/237763@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 428711 [details].
Comment 8 Radar WebKit Bug Importer 2021-05-14 22:47:19 PDT
<rdar://problem/78047472>