Consider calling closeSession when _cleanup is called while awaiting NativeMessagingPortal::Start (_doInitPortal)
Categories
(WebExtensions :: General, enhancement, P3)
Tracking
(Not tracked)
People
(Reporter: robwu, Unassigned)
References
Details
Launching a native messaging host application through a portal consists of the following messages, called from _doInitPortal
in NativeMessaging.sys.mjs
:
NativeMessagingPortal::CreateSession
(see NativeMessagingPortal.cpp in patch) creates a session.NativeMessagingPortal::GetManifest
(see NativeMessagingPortal.cpp in patch) to retrieve metadata (the native messaging manifest).NativeMessagingPortal::Start
(see NativeMessagingPortal.cpp in patch) to try and launch the application.- Eventually when the browser wants to terminate the application, it does so by calling
closeSession
from_cleanup
in NativeMessaging.sys.mjs, which awaitsstartupPromise
- that is it currently awaits step 3.
The NativeMessagingPortal::Start
message may take a very long time to respond, because it opens a prompt that the user should confirm. From this code review thread on the handle_start_in_thread
method in the portal, it is apparent that the method can take a long time, AND that it should be possible to cancel the request (and thus close the prompt) before the start
message has sent a response.
To prevent the prompt from being stuck forever (even after extension uninstallation), we could ensure that closeSession
can be called past step 2, before step 3.
An example of an implementation is to modify _doInitPortal
to use Promise.race
between portal.start()
and a new Promise, and resolve that new Promise from _cleanup
(to be notified of the request to shut down, optionally with a delay). If start()
has not resolved yet (which could indicate that the portal is waiting for the user to dismiss the prompt), the implementation there could call closeSession
(and clear this.portalSessionHandle
). Doing so makes sure that the native messaging host application won't launch with an excessive delay (long after the extension context has closed or even the extension uninstalled).
Reporter | ||
Updated•7 months ago
|
Description
•