426499 - [10.5] Crash opening Help menu [@ DrawTheMenu(MenuSelectData*, __CFArray**, unsigned char, unsigned char*)]

Status: VERIFIED FIXED

(Core :: Widget: Cocoa, defect)

Description

[Simon Howes]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9pre) Gecko/2008040104 Minefield/3.0pre
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9pre) Gecko/2008040104 Minefield/3.0pre

I have tested this on two Intel Macs and two PPC Macs and seems only to be a problem with Intel machines, cannot repro on PPC.  Clicking on Help freezers the browser, and once it has crashed.

Running OS X 10.5.2.

http://crash-stats.mozilla.com/report/index/fd80f82e-0099-11dd-8fe2-001a4bd43ef6



Reproducible: Always

Comment 2

[Christiaan Hofman]

Are you sure it's a duplicate of bug 400291? That one appears only on PPCX Macs, and not on Intel Macs. So it seems to be the opposite problem!

Comment 3

[Steven Michaud [:smichaud] (Retired)]

Either this isn't a dup of bug 400291, or bug 400291 is more complicated than we thought.  Only time will tell.

Simon, please do Bookmarks : Organize Bookmarks, then look at All Bookmarks : Smart Bookmarks : Recent Tags, and tell us what you see there.

Comment 4

[Simon Howes]

I see no smart bookmarks.  I have tried a clean profile as well.  The bookmarks bar is also empty and Places seems not to exist.

Comment 5

[Steven Michaud [:smichaud] (Retired)]

> I see no smart bookmarks

You mean you don't see a "Smart Bookmarks" item on the bookmarks toolbar or when you do "Organize Bookmarks"?

Or do you mean that you don't see any individual bookmarks under "Smart Bookmarks" (under "Most Visisted", "Recently Bookmarked" or "Recent Tags")?

If it's the former, there's definitely something wrong.

Comment 6

[Simon Howes]

Attachment: Screengrab 1

Comment 7

[Simon Howes]

Attachment: Screengrab 2

Comment 8

[Simon Howes]

Fresh profile by trashing everything.

Comment 9

[Christiaan Hofman]

You can make sure it's recreated by going to about:config, set browser.places.createdSmartBookmarks to false, and restart.

Comment 10

[Simon Howes]

Yes that worked.  They all now appear in the Library window.

Of note on new profile, Smart Bookmarks was set to false.  I changed it to true, restarted, then to false, restarted, then Smart Bookmarks appeared.  

Anyone repro?


Steven, how does this relate to the bug submitted?

Comment 11

[Steven Michaud [:smichaud] (Retired)]

> Steven, how does this relate to the bug submitted?

Can you still reproduce it? :-)

Comment 12

[Simon Howes]

Yes, every time.

Comment 13

[Steven Michaud [:smichaud] (Retired)]

Alright then, do you see circular (self-referencing) bookmarks like those reported at bug 424769?

Comment 14

[Simon Howes]

No.

Comment 15

[Christiaan Hofman]

BTW, I can confirm that the crasher is also still there,

Comment 16

[Steven Michaud [:smichaud] (Retired)]

I think we can be pretty confident this isn't a dup of bug 400291.

Comment 17

[Steven Michaud [:smichaud] (Retired)]

There've been about 300 similar crashes in the last week.

A quick glance through these shows:

1) All seem to be on OS X 10.5 (and all on x86, but I'm not sure that
   means a lot).

2) All (or most?) happen when browsing a menu.

3) All happen in imgRequestProxy::OnStopFrame().

Sam Sidler and Gavin Sharp, please try to find a home for this bug :-)

It's worth noting that all of this is still consistent with this bug
being a Places bug.

Comment 18

[Steven Michaud [:smichaud] (Retired)]

http://crash-stats.mozilla.com/?do_query=1&branch=1.9&query_search=stack&query_type=exact&query=imgRequestProxy%3A%3AOnStopFrame%28gfxIImageFrame*%29&date=&range_value=1&range_unit=weeks

Comment 19

[Simon Howes]

New one for me: http://crash-stats.mozilla.com/report/index/4aa7565e-00ff-11dd-a76b-001a4bd43e5c

Comment 20

[Steven Michaud [:smichaud] (Retired)]

Just so that people are aware:  On OS X 10.5.X, opening the Help menu causes the OS to re-index all the app's menus.  This isn't itself a problem, but can uncover other bugs (i.e. this one (bug 426499) and bug bug 400291).

Bug 426499 and bug 400291 are otherwise _not_ related.

Comment 21

[Steven Michaud [:smichaud] (Retired)]

> http://crash-stats.mozilla.com/report/index/4aa7565e-00ff-11dd-a76b-001a4bd43e5c

I get "error not found" trying to view this.

Comment 22

[Marcia Knous [:marcia]]

Steven: Lately Breakpad is taking a bit of time to process the reports, so if you check back later you will probably see the stack.

(In reply to comment #21)
> > http://crash-stats.mozilla.com/report/index/4aa7565e-00ff-11dd-a76b-001a4bd43e5c
> 
> I get "error not found" trying to view this.
> 

Comment 23

[Steven Michaud [:smichaud] (Retired)]

Thanks, Marcia.

I can see it now, and it also (like the one from comment #0 and the others from comment #18) happens in imgRequestProxy::OnStopFrame().

Comment 24

[Thomas K. (:tom)]

(In reply to comment #2)
> Are you sure it's a duplicate of bug 400291? That one appears only on PPCX
> Macs, and not on Intel Macs. So it seems to be the opposite problem!
> 

That's not what I read in bug 400291 comment 5

> Yes, this is a clean install of 10.5 on my Mac Book Pro, where I have a
> partition for Tiger and a partition for Leoopard. 

Powerbook, PPC; Macbook Pro, Intel.

Comment 25

[Simon Howes]

(In reply to comment #16)
> I think we can be pretty confident this isn't a dup of bug 400291.
> 

For completeness.

There's no hang, no 100% CPU spiking like 400291.  The browser just quits and breakpad loads.  It is not always 100% reproducible, it's hit and miss and sometimes it does crash. On load and clicking Help for the first time, there is a wait for 2-3 seconds until the Help menu appears, guessing this is normal as it indexes the menu items. I have been using a PPC Mac more than Intel, and have not come across a crash like this bug. 

Comment 26

[Steven Michaud [:smichaud] (Retired)]

mconnor, since you're taking an interest in the bugs uncovered by the Help menu re-indexing (on OS X 10.5)  ... :-)

Comment 27

[Steuard Jensen]

Let me add my experience to this bug.  I have MacOS 10.5.2 running on my (PowerPC) PowerBook.  I had been running Firefox 3 beta 4 without incident ever since it was released.  Yesterday, I ran software update to beta 5, and this bug appeared: every time I try to open the Help menu, Firefox immediately hangs.  This is especially frustrating when I have opened the adjacent Window menu and the mouse strays over the "Help" label (in that situation, the program freezes immediately, even before the Window menu is erased).  I first observed the bug while using my primary profile, but is equally reproducible with a totally clean one.  The Mac's "Force Quit" function does close Firefox successfully, but even a plain "kill <pid>" from the Terminal will do the job (it doesn't require "kill -9").


I noticed the recent Firefox Support blog entry at

http://blog.mozilla.com/sumo/2008/04/03/moving-firefox-3-product-help-to-the-web/

about "Moving Firefox 3 product help to the web".  They didn't give a bug reference for that (and I haven't tracked it down), but my initial suspicion is that Leopard's fancy Help menu search tool is deeply unhappy about the Firefox help files no longer being available.  (If that's the reason, then it's even possible that a fresh install of Firefox 3 wouldn't show the behavior: I know nothing about how this Leopard feature is implemented, but maybe it's the removal of previously indexed help files that causes the problem.)

Comment 28

[Steuard Jensen]

I've identified a regression window for this issue: the problem does not occur in nightly build 2008-03-10-15 or earlier, but does occur in 2008-03-11-04 and later.  Squinting at Bonsai between those times has given me absolutely no insight into which checkin might have caused the problem.  (The only one that sounds particularly Mac-related is the fix for bug 421689, but that just looks like a build issue.)  Hopefully someone who knows the code better will recognize something that I didn't...

I'm nominating for blocking by reason of insane frustration (and probably things like potential data loss if you stray onto the wrong menu at a bad moment, etc.)

Hmm.  One more caveat.  I have yet to get a single actual crash out of this: I always see hangs.  It's conceivable that the bug I'm seeing is not the same bug that others have seen here.  (But really, how many independent Leopard-only help-menu kill-the-browser bugs appeared in the beta 5 update?)

Comment 29

[Steven Michaud [:smichaud] (Retired)]

Stuart, you're at the wrong bug.  You should be looking at bug 400291 (and bug 424769 and bug 424884).

It would also help if you read the previous comments in a bug before you comment.

Comment 31

[Rob Campbell [:rc] (:robcee)]

I encountered this problem this morning. I was able to reproduce it on the current nightly - Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9pre) Gecko/2008040704 Minefield/3.0pre ID:2008040704.

I checked my recent tags as per comment #3 and saw a tag labeled "mozilla reference". I split the two tags by adding a comma between then and now I'm unable to reproduce the bug.

when available, my crash report should be at:

http://crash-stats.mozilla.com/report/pending/330525cf-04aa-11dd-af6c-001cc45a2c28

Comment 32

[Rob Campbell [:rc] (:robcee)]

I commented too soon. The bug's still there it was just resting. Was able to reproduce by opening the Help menu, hovering over a couple of items then clicking "Help" in the menu strip a few times in rapid succession. I think I missed the "hover" part of the step in my previous verification attempt.

Comment 33

[Simon Howes]

There is no need to hover.  Sometime it can happen as soon as you hit Help, sometimes a few seonds later.  It's hit and miss if you get the crash, I haven't had one in two days, however I bet I'll have it happen a couple more times in the next day or two.

Comment 35

[Thomas K. (:tom)]

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9pre) Gecko/2008040804 Minefield/3.0pre
http://crash-stats.mozilla.com/report/pending/5f91751c-065c-11dd-ae7f-001cc45a2ce4

And I was trying to get to the update checker, I've never actually had a problem with the help menu on intel, only PPC.  So I was surprised.

Comment 36

[Simon Howes]

Tom, the crash report you posted if from an Intel machine.

Comment 37

[Thomas K. (:tom)]

(In reply to comment #36)
> Tom, the crash report you posted if from an Intel machine.
> 

Yes, I understand this is the Intel bug and bug 400291 is the PPC version.

Comment 38

[Mike Beltzner [:beltzner, not reading bugmail]]

Is this in the right component, or is it a Cocoa widget bug?

Comment 39

[Steven Michaud [:smichaud] (Retired)]

> is it a Cocoa widget bug?

Very unlikely, given where the crashes are happening.

The reason people only see this on OS X (10.5) is that it's a bug (presumably a cross-platform bug) that's uncovered by something that only happens on OS X 10.5 -- every time you open the Help menu, it re-indexes all the app's menu items.

Comment 40

[Steven Michaud [:smichaud] (Retired)]

I personally think this (like bug 400291) is likely to be a Places bug ... though this is _not_ a dup of bug 400291 and the two are probably unrelated (except for how they're triggered).

Comment 41

[Marcia Knous [:marcia]]

An update from crash central - I was able to get the browser in a state where I could reproduce this reliably 5-6 times in a row. In this instance here is how I could reproduce it:

1. Start the browser session and make it load a bunch of tabs (25-30). I had a master password set as well as an intranet login. Dismiss those password dialogs and then click rapidly on the Help menu **before** the tabs finish loading. I crashed.
2. With 25-30 tabs open, Go to View | Sidebar | History and change the view. I then set focus back to a tab and when up to the Help menu and click a few times. I crashed 4 times this way.
3. Clicked rapidly on the help menu with 25-30 tabs open. Crashed once. 

Comment 42

[philcolbourn]

I have only just noticed a bug similar to this since I don't use the Help menu very often.

When I click on Help Firefox is immediately unresponsive and CPU load goes to 100%.

I am on Mozilla/5.0 (Macintosh; U; PPC Mac OS X 10.5; en-US; rv:1.9b5) Gecko/2008032619 Firefox/3.0b5

OS X 10.5.2

It happens every time today (but I have never noticed before today).

I hope this helps.

Comment 43

[Steven Michaud [:smichaud] (Retired)]

(In reply to comment #41)

Marcia, I tried both your suggestions (with today's nightly, with and
without a fresh profile) ... and nothing (no crashes).  So we're still
missing some crucial piece(s) of information, and we don't have usable
STR for this bug.

By the way, do you still see crashes with recent nightlies
(yesterday's and today's).  Some comments at bug 400291 indicate that
bug has been fixed recently, and (a real long shot) I wonder if this
one (bug 426499) might have been, too.

Comment 44

[Marcia Knous [:marcia]]

Steven: Today, I am still able to reproduce this crash, but I get a different stack: http://crash-stats.mozilla.com/report/index/4340e042-0a57-11dd-ad59-001b78bc73ea

STR:
1. Open a number of tabs. **Make sure** to have tabs that contain authentication dialogs, such as the intranet or a Master Password dialog.
2. Close the browser and restart the tabs.
3. While the tabs are loading, click rapidly in the Help menu area. I did not crash at first, but when I looked at the Window drop down, the menu was corrupted and only the "M" was showing (normally Minimize and Zoom both show with the appropriate keyboard shortcuts). Shortly after that operation I crashed.

Would console output help at all?

Comment 45

[Steven Michaud [:smichaud] (Retired)]

> http://crash-stats.mozilla.com/report/index/4340e042-0a57-11dd-ad59-001b78bc73ea

Interesting.  In fact this stack makes a lot more sense for this
trigger (opening the Help menu) than the other one.  _Both_ are top
crashers (each in their own right).  So this bug is even worse than we
thought :-(

http://crash-stats.mozilla.com/report/list?range_unit=weeks&branch=1.9&range_value=2&signature=DrawTheMenu%28MenuSelectData%2A%2C+__CFArray%2A%2A%2C+unsigned+char%2C+unsigned+char%2A%29

> Would console output help at all?

It might.  Upload it as an attachment if there's a lot of data.

I'll try your new STR.

Comment 46

[Steven Michaud [:smichaud] (Retired)]

> I'll try your new STR.

I did, and got one crash! ... but then Breakpad failed to submit it
(presumably because of server problems).

I've tried to reproduce this while Minefield is running in gdb.  But
for complicated reasons I can't do what's needed while Minefield is
starting up.

There's a "reload all tabs" available when you right-click on a tab
(in the tab bar), and this suits my purposes exactly.  But when I try
it (and then rapidly open and close the Help menu while all the tabs
are reloading) I don't get any crashes.

Do you see crashes when you open and close the Help menu while
"reloading all tabs"?

Comment 47

[Marcia Knous [:marcia]]

Attachment: Output from console.app before crash

Steven: When I am successful at crashing (and I just did it again), I exit the app and then try to hit the Help menu with my mouse *before* the authentication dialogs load. When I wait until after the authentication dialogs come up, then I don't seem to be successful.

I haven't tried it with reload tabs, but I think once when I did I was not successful. Moments ago I again saw the corruption of the Window menu listing, so that is also something in common with this one crash.

I have some data in console.app, but I am not sure if will help, but attaching anyway.

Comment 48

[Mike Connor [:mconnor]]

THis looks like a core bug on running on leopard, maybe bad interaction between menus and sheets? (i.e. indexing menus when sheets are popping up, where we don't show menus)

Latest stacks are looking entirely on the core side, the signature that I'm replacing in the summary isn't in the top100 crashes that I can see, so moving to Core based on the current state of the bug.

Comment 49

[Steven Michaud [:smichaud] (Retired)]

> I have some data in console.app, but I am not sure if will help, but
> attaching anyway.

Thanks ... though it's probably irrelevant.  (The "no security
manager" errors show that you were trying to do JavaScript-to-Java
LiveConnect, which is broken as of bug 421855.)

Comment 50

[Steven Michaud [:smichaud] (Retired)]

(In reply to comment #48)

> the signature that I'm replacing in the summary isn't in the top100
> crashes that I can see

Yes it is.  Look at comment #18.  I just tried that comment's URL
(altered to search over two weeks instead of just one) and it returned
about 2100 bugs -- easily among the topcrashers.

It's a mistake to only identify (and count) crashes by what's at the
top of the crashing stack.

Comment 51

[Josh Aas]

Some notes...

One potential problem with our native menu system is that almost all of our menu population (adding, removing, updating items) happens in the carbon event handler for kEventMenuOpening. The Cocoa equivalent to this, which we'd rather use but isn't available on 10.4, is "menuWillOpen:", a delegate method for NSMenu. The "Special Considerations" section of the API documentation for "menuWillOpen:" says "Do not modify the structure of the menu or the menu items during this method." In pure Carbon, populating/updating the menu in the kEventMenuOpening handler is OK, it says so explicitly in the Carbon docs. It is probably better to do population on the kEventMenuPopulate Carbon event, but either way is technically OK.

What we're doing is registering for Carbon events on the Carbon menus that back the Cocoa implementation. I'm not sure the pure Carbon rules apply here, if it is not OK to populate during the Cocoa "menuWillOpen:" it might not be OK to populate during kEventMenuPopulate for the Carbon backing on the Cocoa menu either. This is speculation of course, but I think there is a fairly good chance I'm right and this kind of guesswork is what we're stuck with since we have to use undocumented APIs and we can't see Apple's code.

Not using Carbon, even if we had access to 10.5 Cocoa APIs, actually makes our situation worse. The Apple APIs become diverge further from the way XUL works. Consider the problem of needing to update native menu items when a menu opens, and potentially doing more updates as a result of executing OnWillOpen and OnDidOpen handlers in Gecko. We can't do any of that using NSMenu's "menuWillOpen:" - all we can do with that is reject the open if we have a context menu up by ending the menu tracking session. Our only option for updating the menu contents and executing handlers when the menu opens is "menuNeedsUpdate:", which seems fine until you realize there is no way to know if that is getting called in response to a keyboard command targeting an item in the menu or a mouse click opening the menu.

Comment 52

[Simon Howes]

Attachment: Shark build w/ Apple Crash Reporter

I hope this give some more data.

Comment 53

[Steven Michaud [:smichaud] (Retired)]

Attachment: Patch for testing -- disables menu icons (not a fix)

This patch is for testing -- not a fix.  I hope it will let us narrow
the field of possible causes for this bug.

The patch disables menu icons (only blank placeholder menu icons still
appear).  With it I'm no longer able to crash using Marcia's STR from
comment #44 (which has "worked" for me pretty consistently).

Marcia and Simon (and others who are able to reproduce either of this
bug's two crashes fairly easily):  Please try my tryserver build (made
with this patch) and let us know your results.

https://build.mozilla.org/tryserver-builds/2008-04-16_15:[e-mail]-bugzilla426499-test1/[e-mail]

I'm playing a hunch here ... but it's not just a wild guess :-)

Among the crashes returned by the URL in comment #18, I noticed one
whose top frame is in _CGSWindowBounds.  It occurs in Cocoa code in
nsMenuIconItemX::OnStopFrame() in widget/src/cocoa/nsMenuIconItemX.mm.

http://crash-stats.mozilla.com/report/list?range_unit=weeks&query_search=stack&query_type=exact&branch=1.9&signature=_CGSWindowBounds&query=imgRequestProxy%3A%3AOnStopFrame%28gfxIImageFrame%2A%29&range_value=1

Seeing this made me wonder what would happen if I disabled menu icon
support in Cocoa widgets.

(By the way, what I mean by "this bug's two crashes" are 1) the crash
in imgRequestProxy::OnStopFrame() and 2) the crash in DrawTheMenu().)

(Side note:  At first (as I said in comment #46) I couldn't reproduce
Marcia's STR while running in gdb.  The reason was that if you run
Minefield from Terminal (by itself or inside gdb), Terminal keeps the
focus while Minefield starts up, and I couldn't switch the focus to
Minefield quickly enough.  But Josh pointed out the solution -- use
"set args" in gdb to give Minefield a "-foreground" startup parameter,
which will make it grab the focus as it starts up.)

(Another side note:  Just turning off synchronous loading of menu
icons (in nsMenuItemIconX::LoadIcon()) doesn't (by itself) stop the
crashes.)

Comment 54

[Simon Howes]

trying your build now.

Comment 55

[Simon Howes]

Of note.  When app is loaded and Help is clicked, usually there is 3-4 second wait for the menu to open.  Your build the menu opens after a fraction of a second delay.

Comment 56

[Marcia Knous [:marcia]]

Attachment: Menu in a funky state

Steven: I have started testing your tryserver build on my Intel Leopard Machine (PPC is next). While I have not yet been able to crash, I am able to get the menu in a funky state when authentication dialogs are present. I know that it wants me to act on the authentication dialog, but when I hit cancel and then go back up to the menu quickly I get the attached screenshot.

Comment 57

[Marcia Knous [:marcia]]

You can't see it well in the screenshot in Comment 56, but when that shot was taken all the menu items are missing except the Minefield menu label. I guess I would expect that even though an auth dialog is present I could still see all the menu items and that they would be available. Getting out of this state is fairly easy - I just click the cancel button on the auth dialog.

Comment 58

[Steven Michaud [:smichaud] (Retired)]

(In reply to comment #55)

The shorter delay is very interesting!  I think we're on to something.

(In reply to comment #56 and comment #57)

If you don't see a crash, Marcia (and I bet you won't), what you
report just shows that the funky menu business is unrelated to the
crashes.

Comment 59

[Marcia Knous [:marcia]]

No crash using the same build on Leopard PPC - so despite trying very hard, I was unable to crash using the build. I did see the same menu issue on PPC mac. I also noticed the Help menu opens quickly on Leopard, but I noticed that it does not on PPC.

Comment 60

[Nihar Shah]

Crashes for me on PPC ... both my PowerBook G4s have this issue... 

Comment 61

[Steven Michaud [:smichaud] (Retired)]

> Crashes for me on PPC ... both my PowerBook G4s have this issue...

Using my tryserver build from comment #53?

Using today's Minefield nightly?

Comment 62

[J. Gatellier]

Works for me on ppc the Help menu is faster, no crash. And History menu is also much faster with no icons.  

Comment 63

[monchi]

has the tryserver-fix been added to the nightlies ?

Comment 64

[Steven Michaud [:smichaud] (Retired)]

> has the tryserver-fix been added to the nightlies ?

No, and it won't be.  (It was just for testing, not a fix.)

But I'm close to a real fix, and should have a patch (and tryserver
build) up later today.

Comment 65

[Steven Michaud [:smichaud] (Retired)]

Attachment: Fix

Here's a patch that (as far as I can tell) fixes all the crashes that
can happen as Apple is reindexing our menus (including some not yet
reported), at the expense (I think) of stopping our menus from being
searchable via the Help menu.

It appears that some operations (including OS operations) are unsafe
while the indexing is happening.  We don't yet know exactly which ones
(and Apple doesn't yet have any documentation on the subject), so I
decided to simply ignore all Carbon menu events (in nsMenuX.mm's
MyMenuEventHandler()) during indexing.

This patch should also either eliminate or greatly reduce the delays
opening the Help menu ... and I've claimed so in my comments :-)  But
I've never been able to reproduce long delays, so I need confirmation
from others on this score.  If I'm right, this patch will also resolve
bug 414699.

What I'm doing to detect Apple's reindexing is also undocumented.  But
it seems very reliable, and Apple doesn't yet have a documented way to
do this.

A tryserver build will follow shortly.

Comment 66

[Steven Michaud [:smichaud] (Retired)]

Here's a tryserver build made with my latest patch (attachment
316462 [review]).  Marcia, Simon and others, please bang away at it and let us
know your results.

https://build.mozilla.org/tryserver-builds/2008-04-18_09:[e-mail]-bugzilla426499/[e-mail]

Comment 67

[Louise]

(In reply to comment #66)
> Here's a tryserver build made with my latest patch (attachment
> 316462 [details]).  Marcia, Simon and others, please bang away at it and let us
> know your results.

Works great.  Help menu opens the moment I click on it and help menu search seems to work ok for menu items other than the bookmarks.

OS X 10.5.2
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9pre) Gecko/2008041809 Minefield/3.0pre ID:2008041809

Comment 68

[Simon Howes]

I'll second #67.

Comment 69

[Christiaan Hofman]

That's a nasty hack. But it doers make opening the Help menu a lot faster. However this does not fix the crasher. I got it almost right away, only by opening the Bookmarks menu.

Comment 70

[Steven Michaud [:smichaud] (Retired)]

> However this does not fix the crasher. I got it almost right away, only by
> opening the Bookmarks menu.

Which crasher?  Please post a Breakpad log.

Comment 71

[Christiaan Hofman]

(In reply to comment #70)
> > However this does not fix the crasher. I got it almost right away, only by
> > opening the Bookmarks menu.
> 
> Which crasher?  Please post a Breakpad log.
> 

I can't find it in about:crashes. But it was a @DrawTheMenu crasher. I think that should be expected, as your fix only prevents indexing to trigger the the crasher, it does not address the crasher itself at all. 

Comment 72

[Steven Michaud [:smichaud] (Retired)]

> as your fix only prevents indexing to trigger the the crasher, it
> does not address the crasher itself at all

I had thought (or at least hoped) that all the crashes were happening
because some operations are unsafe during indexing (i.e. during calls
to _SimulateMenuOpening() and perhaps also _SimulateMenuClosed()).
This may still be true of some of them.

So I'd really like to know about menu-related crashes that happen even
when menu indexing is disabled.  Until my patch lands, it's probably
best to report them here.

(I never see these crashes myself, so I rely on others to report
them.)

Comment 73

[Steven Michaud [:smichaud] (Retired)]

> I never see these crashes myself

I should have said I never see these crashes myself spontaneously (without a lot of effort).

Comment 74

[Josh Aas]

Comment on attachment 316462 [details] [diff] [review]
Fix

+- (void)nsMenuX_SCTGRLIndex_indexMenuBarDynamically

This should have macro exception handlers I think.

Comment 75

[Steven Michaud [:smichaud] (Retired)]

> This should have macro exception handlers I think.

I tend to think we shouldn't add them.

It's the OS that calls this method (because we've used method
swizzling to "hook" it), not us.  (It's called whenever the Help menu
is opened.)  So this method will (presumably) never run in the midst
of an XPCOM call.  And it doesn't call any of our code.

If we do add them, any Objective-C exceptions that occur during it
(even ones the OS handles) will trigger crashes.

But I'm not dead set against adding them.

Comment 76

[Steven Michaud [:smichaud] (Retired)]

> (even ones the OS handles)

Should have said "even non-fatal ones ignored by the OS".

Comment 77

[Josh Aas]

Makes sense, don't add the macros.

Comment 78

[Steven Michaud [:smichaud] (Retired)]

(Following up comment #72)

I've spent the last few days trying to resolve this bug's crashes
directly (as distinct from resolving them by disabling 10.5's menu
indexing) -- the two crashes reported here, plus another that I found
playing with the Help menu while quitting.

Nothing that I tried worked out, so I'm giving up (at least for the
time being).

These crashes are simply too hard for me to reproduce.  Until we get
better STR, I don't think it's practical to tackle them directly.

We'll have to see what happens after my current patch (attachment
316462 [review]) is landed.  But I'm quite confident that it will greatly
reduce the frequency of this bug's crashes, and perhaps get rid of
some entirely.

The patch will (by all reports) also completely eliminate the delay
opening the Help menu (which is quite severe for some users) -- and
resolve bug 414699.

Comment 79

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 316462 [details] [diff] [review]
Fix

eeesh

Comment 80

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 316462 [details] [diff] [review]
Fix

Low risk.  The patch is very isolated, and disabling Apple's Help menu
re-indexing isn't a great loss (compared to the trouble it causes us).

High gain.  Most of this bug's crashes should disappear.  And bug
414699 will get fixed (a serious problem for some users).

Comment 81

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 316462 [details] [diff] [review]
Fix

a=beltzner omgthx

Comment 82

[Martijn Kint]

I can reproduce this quite easily on my Imac G5:

Crash stat below:
http://crash-stats.mozilla.com/report/index/cccf0091-1113-11dd-9312-001321b13766

Please let me know if you need anything else.

Comment 83

[Steven Michaud [:smichaud] (Retired)]

Landed on trunk:

Checking in widget/src/cocoa/nsMenuX.h;
/cvsroot/mozilla/widget/src/cocoa/nsMenuX.h,v  <--  nsMenuX.h
new revision: 1.32; previous revision: 1.31
done
Checking in widget/src/cocoa/nsMenuX.mm;
/cvsroot/mozilla/widget/src/cocoa/nsMenuX.mm,v  <--  nsMenuX.mm
new revision: 1.69; previous revision: 1.68
done

These changes will appear in tomorrow's trunk nightlies.

Comment 84

[Marcia Knous [:marcia]]

verifying this fixed using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9pre) Gecko/2008042806 Minefield/3.0pre. I have tried to crash both on both PPC and Intel and have not been able to. Good work Steven.

Comment 85

[Robin Whittleton]

Is this an easy backport to 1.8.1? I see this fairly regularly on Firefox2 (Intel 10.5).

Comment 86

[[e-mail]]

Comment on attachment 316462 [details] [diff] [review]
Fix

>+PRInt32 nsMenuX::sIndexingMenuLevel = nsnull;
Integers are supposed to be 0 (zero), not nsnull!

Comment 87

[Steven Michaud [:smichaud] (Retired)]

> Is this an easy backport to 1.8.1? I see this fairly regularly on
> Firefox2 (Intel 10.5).

Yes, I think it would be.

>> +PRInt32 nsMenuX::sIndexingMenuLevel = nsnull;
> Integers are supposed to be 0 (zero), not nsnull!

Oops, you're right.

How would you suggest correcting this?  I don't think it's worth a
whole r+sr+a cycle (since it makes no difference in the code).  But is
there a way to do it without so much fuss?

Comment 88

[Josh Aas]

Just check it in as a simple followup fix, r=josh

Comment 89

[Steven Michaud [:smichaud] (Retired)]

> Just check it in as a simple followup fix, r=josh

Done:

Checking in widget/src/cocoa/nsMenuX.mm;
/cvsroot/mozilla/widget/src/cocoa/nsMenuX.mm,v  <--  nsMenuX.mm
new revision: 1.70; previous revision: 1.69
done

Comment 90

[Steven Michaud [:smichaud] (Retired)]

I don't see any DrawTheMenu crashes in builds made after this patch
was landed -- the latest is in 2008042304.  So it's possible that the
patch for this bug has completely eliminated them.

(I'm not able to use the URL from comment #18 to check for OnStopFrame
crashes -- the servers appear to be bogged down.)