Bug 220952 - [macOS] Close all XPC connections to Launch Services after checking in
Summary: [macOS] Close all XPC connections to Launch Services after checking in
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Per Arne Vollan
URL:
Keywords: InRadar
Depends on: 221094
Blocks:
  Show dependency tree / graph
 
Reported: 2021-01-25 15:08 PST by Per Arne Vollan
Modified: 2021-01-28 10:45 PST (History)
3 users (show)

See Also:

Attachments
Patch (3.26 KB, patch)
2021-01-25 15:23 PST, Per Arne Vollan 		bfulgham: review+
Details | Formatted Diff | Diff
Patch (3.49 KB, patch)
2021-01-26 10:05 PST, Per Arne Vollan 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Per Arne Vollan 2021-01-25 15:08:43 PST 
After checking in with Launch Services on macOS, close all open XPC connections to the service.
Comment 1 Per Arne Vollan 2021-01-25 15:09:48 PST 
<rdar://13785139>
Comment 2 Per Arne Vollan 2021-01-25 15:23:11 PST 
Created attachment 418344 [details]
Patch
Comment 3 Brent Fulgham 2021-01-26 09:02:36 PST 
Comment on attachment 418344 [details]
Patch

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

> Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm:323
> +    // FIXME: Replace the constant 4 with kLSServerConnectionStatusReleaseNotificationsMask when available in the SDK.

Can you add a Bugzilla here so we don't forget?

> Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm:459
> +    if (!m_networkProcessConnection)

This feels very unusual. Should we log when this happens?

> Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm:746
> +    if (!m_networkProcessConnection)

Ditto. Updating activate page information when the network process is dead seems unusual. Might be worth logging in case it is relevant to user-visible bugs.
Comment 4 Per Arne Vollan 2021-01-26 10:05:24 PST 
Created attachment 418435 [details]
Patch
Comment 5 Per Arne Vollan 2021-01-26 10:06:59 PST 
(In reply to Brent Fulgham from comment #3)
> Comment on attachment 418344 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=418344&action=review
> 
> > Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm:323
> > +    // FIXME: Replace the constant 4 with kLSServerConnectionStatusReleaseNotificationsMask when available in the SDK.
> 
> Can you add a Bugzilla here so we don't forget?
> 

Done.

> > Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm:459
> > +    if (!m_networkProcessConnection)
> 
> This feels very unusual. Should we log when this happens?
> 
> > Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm:746
> > +    if (!m_networkProcessConnection)
> 
> Ditto. Updating activate page information when the network process is dead
> seems unusual. Might be worth logging in case it is relevant to user-visible
> bugs.

Fixed in latest patch.

Thanks for reviewing!
Comment 6 EWS 2021-01-26 13:03:48 PST 
Committed r271897: <https://trac.webkit.org/changeset/271897>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 418435 [details].
Comment 7 Chris Dumez 2021-01-28 10:44:55 PST 
I don't think this change was correct. Following up in Bug 221094.