Bug 220952

Summary: [macOS] Close all XPC connections to Launch Services after checking in
Product: WebKit Reporter: Per Arne Vollan <pvollan>
Component: WebKit Misc.Assignee: Per Arne Vollan <pvollan>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, cdumez, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 221094    
Bug Blocks:    
Attachments:
Description Flags
Patch
bfulgham: review+
Patch none

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.