1980452 - ProcessHangMonitor should detect and show a notification to users on extension child process hangs

Status: NEW

(Core :: DOM: Content Processes, defect)

Description

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

As part of investigating Bug 1980009 we have noticed there the ProcessHangMonitor isn't detecting and showing a notification to users on extension child process hangs when the extension process is kept hanging by an extension page and the extension page isn't opened in a currently selected tab that the user is actively interacting with (e.g. through mouse click events).

While looking a bit more closely into why ProcessHangMonitor wasn't always detecting and showing a notification, I noticed the following details that feels relevant:

No hang report sent for extension pages on no explicit user interaction

When there is no extension pages currently selected, the slow script report is not sent at all, the reason behind the hanging process not being sent seems to be due to the logic from XPCJSContext::InterruptCallback that is bailing out when there is no events related to user interaction queued, see: https://searchfox.org/mozilla-central/rev/ab26427a8d31be475be11bbae0e04c84cc7f20ef/js/xpconnect/src/XPCJSContext.cpp#680-707 but the extensions background pages will never have any user interaction events pending because they are actually not visible to the user, and so in those cases no slow script report is actually sent (despite being technically detected on the child process side).

Missing report addonId in hang report triggered for extension pages

If an extension page is loaded in the currently selected tab and the user is interacting with the page (e.g. through mouse clicks or drag events), then ProcessHangMonitor gets actually notified about a slow script from the extension process hanging, but the addonId captured in the slow script data report is not set.

The reason behind the addonId being missing is due to the fact that we only retrieve and include it in the slow script report data if the slow script belongs to a content script.

In particular the logic from XPCJSContext::InterruptCallback here https://searchfox.org/mozilla-central/rev/ab26427a8d31be475be11bbae0e04c84cc7f20ef/js/xpconnect/src/XPCJSContext.cpp#647-648 where we retrieve the addonId for the slow script report to be sent to the parent process and that is only done when principal->ContentScriptAddonPolicy() returns an AddonPolicy (which would be only the case for content scripts but not for extension pages).

e.g. adding something like the following would be enough to ensure we also set an addonId for reports related to an extension page:

...
  } else if(auto policy = principal->AddonPolicy()) {
    policy->GetId(addonId);
    ...
  } else if (auto policy = principal->ContentScriptAddonPolicy()) {
    policy->GetId(addonId);
    prefName = PREF_MAX_SCRIPT_RUN_TIME_EXT_CONTENT;
    limit = StaticPrefs::dom_max_ext_content_script_run_time();
  }

Along with that it may be worth double-checking if the extension page case should also get separate PREF_MAX_SCRIPT_RUN_TIME_EXT_PAGE for the prefName and separate StaticPrefs::dom_max_ext_page_run_time() for the limit.

Comment 1

[Andrew McCreight [:mccr8]]

I'm not going to get to this in the next few weeks, but it sounds like a good idea given how we don't really have that much control over WebExtension code, like web code.