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

Chris Dumez
Reported 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).
Attachments
Patch (63.51 KB, patch)
2021-05-14 13:46 PDT, Chris Dumez
ews-feeder: commit-queue-
Patch (63.58 KB, patch)
2021-05-14 14:37 PDT, Chris Dumez
no flags
Patch (63.48 KB, patch)
2021-05-14 16:55 PDT, Chris Dumez
no flags
Patch (61.22 KB, patch)
2021-05-14 21:03 PDT, Chris Dumez
no flags
Chris Dumez
Comment 1 2021-05-14 13:46:39 PDT
Chris Dumez
Comment 2 2021-05-14 14:37:01 PDT
Chris Dumez
Comment 3 2021-05-14 16:55:26 PDT
Darin Adler
Comment 4 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;
Chris Dumez
Comment 5 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.
Chris Dumez
Comment 6 2021-05-14 21:03:52 PDT
EWS
Comment 7 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].
Radar WebKit Bug Importer
Comment 8 2021-05-14 22:47:19 PDT
Note You need to log in before you can comment on or make changes to this bug.