Bug 212177 - [WK2] WebKit abandons compiled sandbox profiles
Summary: [WK2] WebKit abandons compiled sandbox profiles
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-05-20 16:32 PDT by Chris Dumez
Modified: 2020-05-21 09:12 PDT (History)
6 users (show)

See Also:


Attachments
Patch (2.32 KB, patch)
2020-05-20 16:35 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (2.36 KB, patch)
2020-05-21 08:25 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2020-05-20 16:32:45 PDT
WebKit generates new compiled sandbox profiles whenever sandbox profiles are updated or whenever webkit directories change (which is super common for WebKitTestRunner). Previous compiled sandbox profiles do not get deleted and therefore may accumulate.
Comment 1 Chris Dumez 2020-05-20 16:32:55 PDT
<rdar://problem/54613619>
Comment 2 Chris Dumez 2020-05-20 16:35:35 PDT
Created attachment 399913 [details]
Patch
Comment 3 Saam Barati 2020-05-20 17:29:44 PDT
Comment on attachment 399913 [details]
Patch

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

> Source/WebKit/Shared/mac/AuxiliaryProcessMac.mm:259
> +    // We save the profiles in the user tempory directory so that they get cleaned after on reboot

"after on" => "on"
Comment 4 Saam Barati 2020-05-20 17:30:38 PDT
Comment on attachment 399913 [details]
Patch

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

> Source/WebKit/Shared/mac/AuxiliaryProcessMac.mm:260
> +    // or if they are not accessed in 3 days. This avoids accumulating profiles whenever we change

"in 3 days" => "for 3 days"
Comment 5 Saam Barati 2020-05-20 17:31:13 PDT
Comment on attachment 399913 [details]
Patch

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

>> Source/WebKit/Shared/mac/AuxiliaryProcessMac.mm:260
>> +    // or if they are not accessed in 3 days. This avoids accumulating profiles whenever we change
> 
> "in 3 days" => "for 3 days"

actually, I'm not sure which is more grammatically correct. I'll leave it up to you
Comment 6 Chris Dumez 2020-05-21 08:22:51 PDT
Comment on attachment 399913 [details]
Patch

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

>> Source/WebKit/Shared/mac/AuxiliaryProcessMac.mm:259
>> +    // We save the profiles in the user tempory directory so that they get cleaned after on reboot
> 
> "after on" => "on"

Will fix.

>>> Source/WebKit/Shared/mac/AuxiliaryProcessMac.mm:260
>>> +    // or if they are not accessed in 3 days. This avoids accumulating profiles whenever we change
>> 
>> "in 3 days" => "for 3 days"
> 
> actually, I'm not sure which is more grammatically correct. I'll leave it up to you

I copied from the official documentation which says "if they are not accessed in 3 days".
Comment 7 Chris Dumez 2020-05-21 08:25:32 PDT
Created attachment 399956 [details]
Patch
Comment 8 EWS 2020-05-21 09:08:54 PDT
Committed r262004: <https://trac.webkit.org/changeset/262004>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 399956 [details].
Comment 9 mitz 2020-05-21 09:10:31 PDT
Comment on attachment 399956 [details]
Patch

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

> Source/WebKit/Shared/mac/AuxiliaryProcessMac.mm:264
>          WTFLogAlways("%s: Could not retrieve user cache directory path: %s\n", getprogname(), strerror(errno));

Should probably change this from “cache” to “temporary”.
Comment 10 Chris Dumez 2020-05-21 09:12:54 PDT
(In reply to mitz from comment #9)
> Comment on attachment 399956 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=399956&action=review
> 
> > Source/WebKit/Shared/mac/AuxiliaryProcessMac.mm:264
> >          WTFLogAlways("%s: Could not retrieve user cache directory path: %s\n", getprogname(), strerror(errno));
> 
> Should probably change this from “cache” to “temporary”.

Fixed in <https://trac.webkit.org/changeset/262005>, thanks.