Open Bug 1945904 Opened 8 days ago Updated 7 days ago

GenAIChild spams the parent process with `HideShortcuts` messages and should manage event listeners more efficiently

Categories

(Core :: Machine Learning, defect)

defect

Tracking

()

People

(Reporter: Gijs, Unassigned)

Details

(Keywords: perf)

Attachments

(1 file)

Attached image image.png

In an unrelated profile in a security bug (from Firefox 134, as well, and the code has changed), I noticed that the profiler marker thread had lots of GenAI:HideShortcuts messages in a very short space of time. This was surprising because the screenshots did not show the sidebar or any interaction with the chatbot or the shortcuts. In just over 1 minute, 280 such messages were received. There were 0 ShowShortcuts markers.

This led me to look at the code, and I was surprised how many events it's listening to. In the actor registration, it listens for:

  • DOMContentLoaded
  • mousemove
  • resize
  • scroll

so any of these events will cause the actor to be created. Because it also has allFrames, this means the actor is eagerly created whenever any web frame loads content (including any/all subframes, ads, tracker frames, etc., regardless of whether the user interacts with them).

Then in the actor creation handling, it adds more event listeners, namely mousedown, mouseup and pagehide as well as selectionchange.

The event handling code in the actor ignores DOMContentLoaded and mousemove (so they, AFAICT, are just used to create the actor and will also activate the handleEvent method but will not actually do anything). mousedown and mouseup will be handled and, potentially, at mouseup, will cause us to start showing shortcuts. mousedown, scroll, resize, pagehide and selectionchange all trigger hiding the popup again.

My suggestion to make this perform better would be:

  • remove mousemove/DOMContentLoaded from the list as they seem unnecessary.
  • don't register any of these events as necessitating creating the actor except mouseup. This avoids creating the actor in ad subframes etc. etc., unless/until the user actually interacts with it.
  • use event listeners in the parent to trap mousedown (as we don't care where the mousedown happens so might as well stick the listener on the browser element).
  • only add the scroll/resize/selectionchange/pagehide listeners after sending the "show shortcuts" message, and remove the listeners as soon as we've told the parent process to hide the shortcut (as both scroll/resize can fire many times in a short span of time so sending a message each time is not cheap)

Also, as a minor thing, the code seems to use new Date() to do timestamp computation for when events happen - event.timeStamp would be better suited to this (avoids impact of other listeners etc.) and should be faster. :-)

(In reply to :Gijs (he/him) from comment #0)

  • use event listeners in the parent to trap mousedown (as we don't care where the mousedown happens so might as well stick the listener on the browser element).

I guess the child still needs this in order to check if the selection has changed since the mousedown when the mouseup fires. The reason I suggested the parent is that I thought it was only used for the timestamp and to provide the "the user clicked outside the popup so it should close" functionality - which we could do in the parent.

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

Attachment

General

Creator:
Created:
Updated:
Size: