RESOLVED CONFIGURATION CHANGED 207265
[iOS] Assert that WebProcessProxy::didBecomeUnresponsive is called if the service worker process takes no process assertion 
https://bugs.webkit.org/show_bug.cgi?id=207265
Summary [iOS] Assert that WebProcessProxy::didBecomeUnresponsive is called if the ser...
youenn fablet
Reported 2020-02-05 05:47:52 PST
WebProcessProxy::didBecomeUnresponsive should only terminate a service worker process if the service worker process takes an activity
Attachments
Patch (1.91 KB, patch)
2020-02-05 05:57 PST, youenn fablet 		no flags
DetailsFormatted DiffDiff
Patch (1.96 KB, patch)
2020-02-05 06:07 PST, youenn fablet 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
youenn fablet
Comment 1 2020-02-05 05:57:07 PST
Created attachment 389799 [details] Patch
youenn fablet
Comment 2 2020-02-05 06:07:46 PST
Created attachment 389801 [details] Patch
Chris Dumez
Comment 3 2020-02-05 08:06:13 PST
Comment on attachment 389801 [details] Patch Looks like a racy assertion to me. A process could be unresponsive before we drop the assertions and then the responsiveness timer would fire. If you want to protect against this, maybe we should just stop the responsiveness timer when we get the response to PrepareToSuspend IPC.
youenn fablet
Comment 4 2020-02-05 23:30:31 PST
(In reply to Chris Dumez from comment #3) > Comment on attachment 389801 [details] > Patch > > Looks like a racy assertion to me. A process could be unresponsive before we > drop the assertions and then the responsiveness timer would fire. If you > want to protect against this, maybe we should just stop the responsiveness > timer when we get the response to PrepareToSuspend IPC. My guess was that it might help explaining flakiness issues. I was thinking we could enable/disable the responsiveness timer when we take assertions. I am not familiar with this responsiveness timer logic, maybe that would conflict with logic enabling/disabling the timer by pages. Disabling the timer when we know the process is about to suspend seems good too.
Chris Dumez
Comment 5 2020-02-06 08:11:37 PST
(In reply to youenn fablet from comment #4) > (In reply to Chris Dumez from comment #3) > > Comment on attachment 389801 [details] > > Patch > > > > Looks like a racy assertion to me. A process could be unresponsive before we > > drop the assertions and then the responsiveness timer would fire. If you > > want to protect against this, maybe we should just stop the responsiveness > > timer when we get the response to PrepareToSuspend IPC. > > My guess was that it might help explaining flakiness issues. > I was thinking we could enable/disable the responsiveness timer when we take > assertions. I am not familiar with this responsiveness timer logic, maybe > that would conflict with logic enabling/disabling the timer by pages. > Disabling the timer when we know the process is about to suspend seems good > too. We see flakiness on debug bots and the responsiveness timer is disabled in debug.
youenn fablet
Comment 6 2020-02-10 09:10:01 PST
Closing since timer is no longer enabled on debug bots
Note You need to log in before you can comment on or make changes to this bug.