Bug 212177 – [WK2] WebKit abandons compiled sandbox profiles

• Component: WebKit2
• Status: RESOLVED FIXED

[Chris Dumez]

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.

[Chris Dumez]

<rdar://problem/54613619>

[Chris Dumez]

Created attachment 399913 [details]
Patch

[Saam Barati]

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"

[Saam Barati]

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"

[Saam Barati]

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

[Chris Dumez]

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

[Chris Dumez]

Created attachment 399956 [details]
Patch

[EWS]

Committed r262004: <https://trac.webkit.org/changeset/262004>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 399956 [details].

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

[Chris Dumez]

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

[Chris Dumez]

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>

[Chris Dumez]

Created attachment 405292 [details]
Patch

[Chris Dumez]

Pull request: https://github.com/WebKit/WebKit/pull/24861

[EWS]

Committed 275108@main (2dfb4e9da6fd): <https://commits.webkit.org/275108@main>

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