1350060 - Tune our tolerance for long running beforeunload / unload scripts

Status: NEW

(Core :: DOM: Content Processes, enhancement, P3)

Description

[Mike Conley (:mconley) (:⚙️)]

When a tab is on its way to being destroyed, if it has set any beforeunload or unload handlers, we run those. Currently, I believe we have a tolerance of several seconds for long running scripts in those handlers. This might affect the performance of other tabs that are running in that same content process.

We show the ProcessHangNotification after some number of seconds for long running JS, but the time to wait to show that thing might be too high. Since at this point in a tabs lifetime, the user has made a clear choice ("I want this to go away now"), we might want to be more aggressive about killing the scripts if they're running too long.

Comment 1

[Sylvestre Ledru [:Sylvestre]]

Moving to p3 because no activity for at least 1 year(s).
See https://github.com/mozilla/bug-handling/blob/master/policy/triage-bugzilla.md#how-do-you-triage for more information

Comment 2

[Jens Stutte [:jstutte]]

My understanding would be that the way we handle processes now with fission largely reduces any impact whatsoever on content processes from other domains and thus we do not really need this anymore?

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

beforeunload listener can basically block moving to the next page (that is the whole point of it), so we could do something here, in case it takes too much time. unload listener is less an issue these days.

Comment 4

[Jens Stutte [:jstutte]]

(In reply to Mike Conley (:mconley) (:⚙️) from comment #0)

This might affect the performance of other tabs that are running in that same content process.

I assume that after fission (on desktop) that the probability for this has been reduced quite a lot.

We show the ProcessHangNotification after some number of seconds for long running JS, but the time to wait to show that thing might be too high.

I think after 10s currently.

Since at this point in a tabs lifetime, the user has made a clear choice ("I want this to go away now"), we might want to be more aggressive about killing the scripts if they're running too long.

(In reply to Olli Pettay [:smaug][[e-mail]] from comment #3)

beforeunload listener can basically block moving to the next page (that is the whole point of it), so we could do something here, in case it takes too much time.

I'd assume then that this is about dynamically tuning the dom.max_script_run_time setting for the beforeunload case.