Bug 207265

Summary:

[iOS] Assert that WebProcessProxy::didBecomeUnresponsive is called if the service worker process takes no process assertion

Product:

WebKit

Reporter:

youenn fablet <youennf>

Component:

Service Workers

Assignee:

youenn fablet <youennf>

Status:

RESOLVED CONFIGURATION CHANGED

Severity:

Normal

CC:

beidson, cdumez

Priority:

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none

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	Details Formatted Diff Diff
Patch (1.96 KB, patch) 2020-02-05 06:07 PST, youenn fablet	no flags	Details Formatted Diff Diff
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.