250276 - right-click, cut of BM inside folder inside PT crashes [@ nsMenuPopupFrame::SetCurrentMenuItem ] (aviary)

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[Mike Shaver (:shaver emeritus)]

It's poking at mCurrentMenu, which looks like it has been freed.  No crash on
0.9.1, or the trunk.

Attaching stack.

Comment 1

[Mike Shaver (:shaver emeritus)]

Attachment: stack trace (Linux, today's build, clean)

Comment 2

[Mike Shaver (:shaver emeritus)]

Pretty sure we want this for 1.0, not sure if it hold 1.0RC1.

Comment 3

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

I'm guessing this is a nsIFrame-in-nsCOMPtr issue.  I fixed a few of these in my
local tree, but there's at least two dozen nsIMenuFrame's that get stuck in
nsCOMPtr's in one place or another.  Most look innocent, but some don't.. i'll
try to find the culprit.  Also, biesinger has embarked on a
remove-nsISupports-inheritance-from-nsIFrame project, which will also help.

Comment 4

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: context-menu-crash-0.patch

Blah.  The chain of events looked like this:

- A destructive context menu option is selected (Cut, Delete)
- The menu's command handler executes the command
- The command modifies the bookmarks RDF data source, and removes some
assertions from the parent RDF container (to remove the items)
- The template builder (!@#$) notices this, and sees that it has to remove
content
- It removes the menu item entry from the menu that corresponds to the thing
that was just removed
- The command returns to the menu handler after execution, and the menu starts
to tear down the visible menus
- It goes boom while trying to unselect the active menu item, because that item
got destroyed

The patch makes sure that the menu frame's parent doesn't have it selected as
the current item, if it's being destroyed.

Comment 5

[Mike Shaver (:shaver emeritus)]

Comment on attachment 152886 [details] [diff] [review]
context-menu-crash-0.patch

Looks good to me -- Ben?

Comment 6

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

In on aviary; does trunk want this as well?

Comment 7

[Mike Shaver (:shaver emeritus)]

Yes, very.  Maybe even for 1.8a2 -- asking.  (If not, a3 for sure.)

Comment 8

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Reopening this as it's not on trunk; need a menu person to take a look, though
this has been in on aviary for a while now.

Comment 9

[basic]

nominating

Comment 10

[Asa Dotzler [:asa]]

Neil, can you have a look at this for the trunk?

Comment 11

[[e-mail]]

As far as I can tell the patch looks good for the trunk too.

Comment 12

[Asa Dotzler [:asa]]

Comment on attachment 152886 [details] [diff] [review]
context-menu-crash-0.patch

Who can land this? Time is short for 1.8a6.  a=asa.

Comment 13

[[e-mail]]

Fix checked in to the trunk.