Open Bug 1903338 Opened 7 months ago Updated 7 months ago

Consider calling closeSession when _cleanup is called while awaiting NativeMessagingPortal::Start (_doInitPortal)

Categories

(WebExtensions :: General, enhancement, P3)

enhancement

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:

  1. NativeMessagingPortal::CreateSession (see NativeMessagingPortal.cpp in patch) creates a session.
  2. NativeMessagingPortal::GetManifest (see NativeMessagingPortal.cpp in patch) to retrieve metadata (the native messaging manifest).
  3. NativeMessagingPortal::Start (see NativeMessagingPortal.cpp in patch) to try and launch the application.
  4. Eventually when the browser wants to terminate the application, it does so by calling closeSession from _cleanup in NativeMessaging.sys.mjs, which awaits startupPromise - 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).

Severity: -- → N/A
Priority: -- → P3
You need to log in before you can comment on or make changes to this bug.