[macOS] Remove unneeded sandbox extension member

• Component: WebKit Misc.
• Status: RESOLVED FIXED

[Per Arne Vollan]

The commit https://trac.webkit.org/changeset/272999/webkit disabled this, but after adopting API to switch from static font registry to user font registry, this can be re-enabled.

[Per Arne Vollan]

Created attachment 434521 [details]
Patch

[Sam Weinig]

Comment on attachment 434521 [details]
Patch

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

> Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm:1282
> +void WebProcess::switchFromStaticFontRegistryToUserFontRegistry(const WebKit::SandboxExtension::Handle& fontMachExtensionHandle)

What is the point of changing this to an l-value? In general, handles should always be passed by r-value.

[Per Arne Vollan]

Created attachment 434537 [details]
Patch

[Per Arne Vollan]

(In reply to Sam Weinig from comment #2)
> Comment on attachment 434521 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=434521&action=review
> 
> > Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm:1282
> > +void WebProcess::switchFromStaticFontRegistryToUserFontRegistry(const WebKit::SandboxExtension::Handle& fontMachExtensionHandle)
> 
> What is the point of changing this to an l-value? In general, handles should
> always be passed by r-value.

Ah, good point. I have reverted back to the previous signature.

Thanks for reviewing!

[Myles C. Maxfield]

Does this actually improve performance on OSes which have a static registry?

[Per Arne Vollan]

(In reply to Myles C. Maxfield from comment #5)
> Does this actually improve performance on OSes which have a static registry?

Good point, I will start testing this now.

Thanks for reviewing!

[Darin Adler]

My review+ assumes that this works as expected and improves performance.

[Per Arne Vollan]

(In reply to Darin Adler from comment #7)
> My review+ assumes that this works as expected and improves performance.

Will test performance before landing.

Thanks for reviewing!

[Alexey Proskuryakov]

*** Bug 228583 has been marked as a duplicate of this bug. ***

[Per Arne Vollan]

(In reply to Per Arne Vollan from comment #8)
> (In reply to Darin Adler from comment #7)
> > My review+ assumes that this works as expected and improves performance.
> 
> Will test performance before landing.
> 
> Thanks for reviewing!

Landing just the sandbox extension bits, since re-enabling the font pre-warming does not show a meaningful performance improvement.

[Per Arne Vollan]

Created attachment 434765 [details]
Patch

[EWS]

Committed r280542 (240169@main): <https://commits.webkit.org/240169@main>

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

[Radar WebKit Bug Importer]

<rdar://problem/81418101>