Open Bug 1896461 Opened 5 months ago Updated 4 months ago

netmonitor janks the entire browser UI on visiblitychange events

Categories

(DevTools :: Netmonitor, defect, P2)

Product:
DevTools
Component:
Netmonitor
Type:
defect

Tracking

(Not tracked)

People

(Reporter: florian, Unassigned)

References

(Depends on 1 open bug, Blocks 2 open bugs)

Details

(Keywords: perf, perf:responsiveness)

See this profile: https://share.firefox.dev/3WzOj7b

The profile shows that a "visibilitychange" DOM event handler in the chrome://devtools/content/netmonitor/index.html window blocked the parent process main thread for 46s.

My Firefox instance had a single window. The session has been running for multiple days. Devtools has never been used in the foreground tab, but it's likely that devtools has been open for the entire week-end in a background tab that updates periodically using XHR.

Given devtools wasn't visible, I don't think it should update anything on receiving a visibilitychange event (or maybe it shouldn't receive it at all).

In theory the visibility handler should not kick in when DevTools are used in a background tab, so we will check what could be wrong there. Then it's very likely that the netmonitor can be the culprit for the performance issue, because we have no cap for the number of requests in this panel. But in your case it shouldn't have been rendered at all.

Severity: -- → S3
Flags: needinfo?(nchevobbe)
Priority: -- → P2
See Also: → 1594378

Looking into it, I'm seeing a few things:

  • visibilitychange is fired when we change the selected panel (so if Netmonitor was selected and you go to the settings panel)
  • visibilitychange is not fired when another tab is selected
  • visibilitychange is fired when a window is minimized/restored or when exiting screen saver, even when DevTools were opened in another tab than the selected one
  • the custom connect function we wrote is used for almost all connected components in the Netmonitor, which means that when the visibilitychange event handler fires, we force a lot of components to re-render

All those, combined with the fact there are no limits / virtualization, explains the issue Florian experienced.
I think the original intent of the VisibilityHandler was to avoid hurting the other panels performance (Bug 1399242), but we may have missed the case where an "unselected" tab has DevTools opened.

Note that for the "regular" case of switching DevTools panels, we are manually dispatching the event: https://searchfox.org/mozilla-central/rev/7a8904165618818f73ab7fc692ace4a57ecd38c9/devtools/client/framework/toolbox.js#2794-2819 , we should check if this is still needed and/or if we should rely on other APIs instead

Flags: needinfo?(nchevobbe)

(In reply to Nicolas Chevobbe [:nchevobbe] from comment #2)

  • visibilitychange is fired […] when exiting screen saver

This is what I encountered. Is it a platform bug, or is a docshell used by devtools incorrectly not set to inactive?

(In reply to Florian Quèze [:florian] from comment #3)

(In reply to Nicolas Chevobbe [:nchevobbe] from comment #2)

  • visibilitychange is fired […] when exiting screen saver

This is what I encountered. Is it a platform bug, or is a docshell used by devtools incorrectly not set to inactive?

Not sure, I filed Bug 1897187 so we can get a DOM peer look into this

You need to log in before you can comment on or make changes to this bug.