Summary: | WKContentRuleLists should have a maximum FileProtection of CompleteUnlessOpen | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Alex Christensen <achristensen> | ||||||
Component: | New Bugs | Assignee: | Alex Christensen <achristensen> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | cdumez, cgarcia, ews-watchlist, ggaren, koivisto, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Alex Christensen
2019-04-18 15:45:46 PDT
Created attachment 367767 [details]
Patch
Created attachment 367822 [details]
Patch
Comment on attachment 367822 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=367822&action=review r=me > Source/WebKit/NetworkProcess/cache/NetworkCacheFileSystemCocoa.mm:39 > + // FIXME: For the network cache we should either use makeSafeToUseMemoryMapForPath instead of this > + // or we should listen to UIApplicationProtectedDataWillBecomeUnavailable and unmap files. Can you address this FIXME in a follow-up patch? Should be pretty straight-forward. (We probably can't close the Radar until we fix both cases.) Comment on attachment 367822 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=367822&action=review > Source/WebKit/NetworkProcess/cache/NetworkCacheFileSystemCocoa.mm:41 > + NSDictionary<NSFileAttributeKey, id> *attributes = [[NSFileManager defaultManager] attributesOfItemAtPath:path error:&error]; This constructs a dictionary with all kinds of attributes that we don't care about. Should we worry about performance here? How about using the C API like the existing code does? Comment on attachment 367822 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=367822&action=review >> Source/WebKit/NetworkProcess/cache/NetworkCacheFileSystemCocoa.mm:41 >> + NSDictionary<NSFileAttributeKey, id> *attributes = [[NSFileManager defaultManager] attributesOfItemAtPath:path error:&error]; > > This constructs a dictionary with all kinds of attributes that we don't care about. Should we worry about performance here? How about using the C API like the existing code does? I don't think this is a significant performance regression. We could switch back if we find it to be worth it, but the lack of magic numbers and mysterious structures here makes the code much more maintainable. (In reply to Geoffrey Garen from comment #4) > Comment on attachment 367822 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=367822&action=review > > r=me > > > Source/WebKit/NetworkProcess/cache/NetworkCacheFileSystemCocoa.mm:39 > > + // FIXME: For the network cache we should either use makeSafeToUseMemoryMapForPath instead of this > > + // or we should listen to UIApplicationProtectedDataWillBecomeUnavailable and unmap files. > > Can you address this FIXME in a follow-up patch? Should be pretty > straight-forward. (We probably can't close the Radar until we fix both > cases.) Done in https://bugs.webkit.org/show_bug.cgi?id=197264 |