With the patches in bug 1698997 I was noticing crashes.

The browser would crash with the following STR:

1. Copy a string to the clipboard.
2. Open the web console with Cmd+Opt+K.
3. In the filter textbox, right click and choose Paste.

What happened was that, during the context menu's popuphidden event, the menu's nsMenuPopupFrame was destroyed. This destroyed the nsStandaloneNativeMenu, which destroyed the nsMenuX inside it. However, nsMenuX::MenuClosed() was still running further up the stack - it's the method that was firing the popuphidden event. And after it fired the popuphidden event, it accessed a member variable, which was now a use-after-free.

So we need to make sure that the nsMenuX object can stay alive while one of its methods is running. Since it's currently uniquely owned by its parent nsMenuX/nsMenuBarX/nsStandaloneNativeMenu, it can't really keep itself alive. So we'll have to make it reference-counted.

This makes it clear that it cannot be any other subclass of nsMenuObjectX.

The other reason I want to do this is because I'd like to change nsMenuX to be
reference-counted, so that it can keep itself alive when firing DOM events.
This means that UniquePtr<nsMenuX> will become RefPtr<nsMenuX>. With this patch,
we can make just nsMenuX reference-counted without making nsMenuObjectX
reference-counted.

Depends on D109016

nsMenuX is currently kept alive by a UniquePtr field of the parent nsMenuX,
nsMenuBarX, or nsStandaloneNativeMenu. However, sometimes there is a risk that
those owner classes could be destroyed while an nsMenuX method is running. Then
they would destroy their nsMenuX field while an nsMenuX method is on the stack,
which means that the rest of the nsMenuX method would now be running on a free'd
object. This is not good.

Example:
When the popupshowing/popupshown/popuphiding/popuphidden events are fired, the
JS code that runs during the event listener can remove the menupopup DOM element
from the DOM, which can in some cases synchronously destroy the nsMenuX object
that is firing the event. So for those cases, we want the caller of the nsMenuX
method to hold an owning reference (a RefPtr) to the nsMenuX object that it's
calling the method on. This only works if nsMenuX is reference-counted.

Depends on D109017

This will make it easier to merge nsMenuX and nsMenuItemX at some point.
It also means that MenuChild no longer contains a UniquePtr, so it is now copyable.
This makes it possible to return MenuChild from Get(Visible)ItemAt, see next patch.

Depends on D109018

This gives the caller more concrete types to work with. It also means the child
is kept alive in the caller, because MenuChild contains a RefPtr.
The current callers don't manipulate the menu children array while they have
the pointer to the child, but in theory they could, and having a RefPtr here is
safer.

Depends on D109019

This is the main fix for this bug.

Depends on D109020

This is somewhat annoying.

I think the alternative would be:

• Use WeakPtr<...> for m(Menu)Parent and mMenuGroupOwner, so that these pointers
  "null themselves out" when the pointed-to object goes away, and
• removing the requirement in the menu group owner to have all listeners
  unregistered when the menu group owner is destroyed.

I'm planning to look at this more in the future, but for now, I think this patch
is the right way to go.

Depends on D109021

https://hg.mozilla.org/mozilla-central/rev/af586a5a749e
https://hg.mozilla.org/mozilla-central/rev/72781d0c5a4e
https://hg.mozilla.org/mozilla-central/rev/f8d23415e2a1
https://hg.mozilla.org/mozilla-central/rev/1eea9cbbcf8f
https://hg.mozilla.org/mozilla-central/rev/b6a58b107b57
https://hg.mozilla.org/mozilla-central/rev/99678994163c
https://hg.mozilla.org/mozilla-central/rev/238e290a23ee