Bug 239401

Summary: Use ProcessTerminationReason::Unresponsive for unresponsive network processes
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: WebKit2Assignee: Simon Fraser (smfr) <simon.fraser>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, kkinnunen, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
ews-feeder: commit-queue-
Patch none

Description Simon Fraser (smfr) 2022-04-15 13:47:01 PDT
Use ProcessTerminationReason::Unresponsive for unresponsive network processes
Comment 1 Simon Fraser (smfr) 2022-04-15 13:48:37 PDT
Created attachment 457719 [details]
Patch
Comment 2 Chris Dumez 2022-04-15 13:51:33 PDT
Comment on attachment 457719 [details]
Patch

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

> Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp:166
>                  weakThis->terminate();

Seems like it would be less error-prone to have an ProcessTerminationReason = ProcessTerminationReason::RequestedByClient optional parameter on terminate(), rather than relying on the call site to call networkProcessDidTerminate() by itself.
Comment 3 Simon Fraser (smfr) 2022-04-15 17:56:54 PDT
(In reply to Chris Dumez from comment #2)
> Comment on attachment 457719 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=457719&action=review
> 
> > Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp:166
> >                  weakThis->terminate();
> 
> Seems like it would be less error-prone to have an ProcessTerminationReason
> = ProcessTerminationReason::RequestedByClient optional parameter on
> terminate(), rather than relying on the call site to call
> networkProcessDidTerminate() by itself.

I considered that, but terminate() is virtual in AuxiliaryProcess so I'd have to change all of them, and the others use the pattern I followed here.
Comment 4 Simon Fraser (smfr) 2022-04-15 20:44:24 PDT
Comment on attachment 457719 [details]
Patch

Breaks some tests because terminate() needs to trigger logging.
Comment 5 Simon Fraser (smfr) 2022-04-18 12:27:19 PDT
Created attachment 457815 [details]
Patch
Comment 6 EWS 2022-04-19 08:59:21 PDT
Committed r293017 (249756@main): <https://commits.webkit.org/249756@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 457815 [details].
Comment 7 Radar WebKit Bug Importer 2022-04-19 09:00:17 PDT
<rdar://problem/91963662>