Bug 212177

Summary: [WK2] WebKit abandons compiled sandbox profiles
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: WebKit2Assignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, benjamin, cmarcelo, ews-watchlist, ggaren, jbedard, mitz, saam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

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.
Comment 11 Chris Dumez 2020-06-29 15:56:11 PDT
Reverted r262004 for reason:

Revert r262004 as it is not OK to have a data vault in /var/folders/zy/g91x07sn08bgjkm8d_pg0vfc0000gn/T/ <rdar://problem/64540215>

Committed r263705: <https://trac.webkit.org/changeset/263705>
Comment 12 Chris Dumez 2020-07-27 10:47:06 PDT
Created attachment 405292 [details]
Patch
Comment 13 Chris Dumez 2024-02-20 20:31:56 PST
Pull request: https://github.com/WebKit/WebKit/pull/24861
Comment 14 EWS 2024-02-21 10:21:22 PST
Committed 275108@main (2dfb4e9da6fd): <https://commits.webkit.org/275108@main>

Reviewed commits have been landed. Closing PR #24861 and removing active labels.