Bug 197078 – WKContentRuleLists should have a maximum FileProtection of CompleteUnlessOpen

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Alex Christensen]

WKContentRuleLists should have a maximum FileProtection of CompleteUnlessOpen

[Alex Christensen]

Created attachment 367767 [details]
Patch

[Alex Christensen]

<rdar://problem/49564348>

[Alex Christensen]

Created attachment 367822 [details]
Patch

[Geoffrey Garen]

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.)

[Alex Christensen]

http://trac.webkit.org/r244597

[Antti Koivisto]

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?

[Alex Christensen]

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.

[Alex Christensen]

(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