Bug 221088 – [macOS] Crash when updating color preferences

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Per Arne Vollan]

After <https://trac.webkit.org/changeset/271965/webkit>, the WebContent process is no longer allowed to connect to the Launch Services daemon. This introduced a crash in the WebContent process when changing color preferences, since AppKit will then attempt to set application information with Launch Services, which causes a crash when that fails.

[Radar WebKit Bug Importer]

<rdar://problem/73709142>

[Per Arne Vollan]

Created attachment 418644 [details]
Patch

[Brent Fulgham]

Comment on attachment 418644 [details]
Patch

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

r=me, though I think a comment or changed function name might be helpful.

> Source/WebKit/ChangeLog:12
> +        WebContent process.

Do you mean UIProcess here? I think the preference updates are expected in UIProcess, and we should never attempt to make such updates in the WCP.

> Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm:188
> +static void updateCanQuitQuietlyAndSafely(NSApplication*, SEL)

Perhaps this would be clearer with a better name, rather than a comment like I suggested below:

static void preventAppKitFromContactingLaunchServicesFromWebContentProcess(NSApplication*, SEL)

... or maybe you can think of a better name.

> Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm:189
> +{

Perhaps a comment here would be useful:

// WebKit prohibits communication with Launch Services after entering the sandbox. This method override prevents AppKit
// in the WebContent process from attempting to set color preferences. Color preferences are instead handled in the UIProcess.

[Per Arne Vollan]

Created attachment 418651 [details]
Patch

[Per Arne Vollan]

(In reply to Brent Fulgham from comment #3)
> Comment on attachment 418644 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=418644&action=review
> 
> r=me, though I think a comment or changed function name might be helpful.
> 
> > Source/WebKit/ChangeLog:12
> > +        WebContent process.
> 
> Do you mean UIProcess here? I think the preference updates are expected in
> UIProcess, and we should never attempt to make such updates in the WCP.
> 

This happened when updating color preferences from System Preferences. I updated the change log.

> > Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm:188
> > +static void updateCanQuitQuietlyAndSafely(NSApplication*, SEL)
> 
> Perhaps this would be clearer with a better name, rather than a comment like
> I suggested below:
> 
> static void
> preventAppKitFromContactingLaunchServicesFromWebContentProcess(NSApplication*
> , SEL)
> 
> ... or maybe you can think of a better name.
> 

Fixed!

> > Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm:189
> > +{
> 
> Perhaps a comment here would be useful:
> 
> // WebKit prohibits communication with Launch Services after entering the
> sandbox. This method override prevents AppKit
> // in the WebContent process from attempting to set color preferences. Color
> preferences are instead handled in the UIProcess.

Done, with a minor modification.

Thanks for reviewing!

[EWS]

Committed r272017: <https://trac.webkit.org/changeset/272017>

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