1785294 - Consider to defer terminating non-persistent background context on pending calls to async API event listeners

Status: RESOLVED FIXED

(WebExtensions :: General, enhancement, P2)

Description

[Luca Greco [:rpl] [:luca] [:lgreco]]

This bug has been split from Bug 1781265, with the following motivations:

We would like to avoid forcing the extensions to apply changes to their listeners just to explicitly call a waitUntil method for async work (and related promise instances) that the WebExtensions API internals are already implicitly aware of (and have already access to the related promise instances).

In particular when the WebExtensions API internals will be calling an async listener, they will have access to a promise returned by calling the async listener (or to a promise that a non-async listener may be returning as its return value).

And so we agreed to look into making sure that we implicitly track promise returned by API listeners (and based on those defer terminating the background context for a bit longer if the promises are still pending, and eventually forcefully terminating the event page if they are still pending after a certain extended timeout).

Comment 1

[Luca Greco [:rpl] [:luca] [:lgreco]]

Attachment: Bug 1785294 - Reset idle on API event listeners with a pending queryRunListener promise. r?mixedpuppy!

Comment 2

[Luca Greco [:rpl] [:luca] [:lgreco]]

Attachment: Bug 1785294 - Allow pending async API event listeners calls to reset event page idle timeout only once. r?mixedpuppy!

Depends on D153525

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4b7c6808489d
Reset idle on API event listeners with a pending queryRunListener promise. r=mixedpuppy
https://hg.mozilla.org/integration/autoland/rev/fea7efce878b
Allow pending async API event listeners calls to reset event page idle timeout only once. r=mixedpuppy

Comment 4

[Cosmin Sabou [:CosminS]]

Backed out for causing xpcshell failures on test_ext_eventpage_settings.js, they look to be Android only.

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=fea7efce878b8a06456a10f04e68a53c552e008f&selectedTaskRun=Fi1ZYPSTRbWt3S6ZowGjdg.0

Failure log: https://treeherder.mozilla.org/logviewer?job_id=388827465&repo=autoland

Backout link: https://hg.mozilla.org/integration/autoland/rev/fd5312d0bd6bd1361a1cff25042fee3c2842e992

Comment 5

[Luca Greco [:rpl] [:luca] [:lgreco]]

I've updated D153525 to handle the additional failure, not sure why I missed in my pushes to try, when I tried it locally it failed also on Linux Desktop builds.

I took also a look to the other failures from the links in comment 4 and they looked unrelated (besides the same test_ext_eventpage_settings.js which also seems to have failed for the same reasons it failed on android).

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b34a5a23c322
Reset idle on API event listeners with a pending queryRunListener promise. r=mixedpuppy
https://hg.mozilla.org/integration/autoland/rev/c08cf9edf5a5
Allow pending async API event listeners calls to reset event page idle timeout only once. r=mixedpuppy

Comment 7

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/b34a5a23c322
https://hg.mozilla.org/mozilla-central/rev/c08cf9edf5a5