Bug 225820 - Drop FileSystem::fileMetadata() / fileMetadataFollowingSymlinks()
Summary: Drop FileSystem::fileMetadata() / fileMetadataFollowingSymlinks()
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Template Framework (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-05-14 13:40 PDT by Chris Dumez
Modified: 2021-05-14 22:47 PDT (History)
25 users (show)

See Also:

Attachments
Patch (63.51 KB, patch)
2021-05-14 13:46 PDT, Chris Dumez 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (63.58 KB, patch)
2021-05-14 14:37 PDT, Chris Dumez 		no flags Details | Formatted Diff | Diff
Patch (63.48 KB, patch)
2021-05-14 16:55 PDT, Chris Dumez 		no flags Details | Formatted Diff | Diff
Patch (61.22 KB, patch)
2021-05-14 21:03 PDT, Chris Dumez 		no flags Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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>