Crash bugs from http://talkback-public.mozilla.org/reports/firefox/FFTrunk/FFTrunk-topcrashers.html

Roc, in which cases frame->GetContent() returns null? non-primary textframes?

Bz says that the viewport's frame is always null. Other frames always have a valid content pointer until the frame is destroyed. So perhaps this isn't the right patch -- maybe the frame pointer for a given text frame we have is no longer valid. That could happen if a frame got destroyed and we didn't invalidate the accessibility subtree it's in. We only cache frames for nsHTMLTextAccessible and nsHTMLLinkAccessible. I noticed that we don't shutdown mFrame when we should, so there's a chance that could cause this bug.

Checked in the 2nd patch. Let's see whether we get more crashes.

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a1) Gecko/20060921 Minefield/3.0a1 ID:2006092104 [cairo] I can still crash in today's build. See TB23567744E and TB23567730Z which are crashes using a clean profile. My 100% steps to reproduce are: Press / Type a few letters Press Esc Press / Crash However, strangely, if I add the following 2 steps before those above, it does not crash. I also cannot crash for the rest of the session. Press / Wait until the quick find bar disappears Follow steps above Don't crash I assume the build I am using carries the latest patch, if not, sorry for the bugspam!

I haven't been able to reproduce the crash.

(In reply to comment #6) > Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a1) Gecko/20060921 > Minefield/3.0a1 ID:2006092104 [cairo] > > I can still crash in today's build. See TB23567744E and TB23567730Z which are > crashes using a clean profile. > > My 100% steps to reproduce are: > Any chance that you could run debugger to see why this is crashing. Especially whether it is because of frame->mContent being null?

Comment on attachment 239363 [details] [diff] [review] Possible fix #2. Either way we do want this fix. >+ NS_IMETHOD Shutdown() { mFrame = nsnull; return nsLinkableAccessible::Shutdown(); } > When is this method actually called?

(In reply to comment #9) > (From update of attachment 239363 [details] [diff] [review] [edit]) > > >+ NS_IMETHOD Shutdown() { mFrame = nsnull; return nsLinkableAccessible::Shutdown(); } > > > When is this method actually called? It's called when a frame or DOM node associated with the accessible is destroyed. The hook for DOM changes is nsDocument's impl for nsIDocumentObserver. The hook for frame changes is in nsCSSFrameConstructor, where we call NotifyAccessibleChange(). I'm seeing problems with the frame notifications which is causing crashes in bug 350381, which can cause Shutdown() not to be called, and an accessible to still be holding onto a nsIFrame* which is no longer valid. That could be causing this bug as well. In fact, the steps in this bug are very suspicious. In bug 350381, it's the 2nd time the link is made visible that it crashes. The link is made visible by setting the class of the parent node. It's also the 2nd time the find bar is made visible that it crashes. The root problem in the other bug is that we're not getting notified when frames are created because of a style change that occured via somewhat indirect means.

Btw, has anyone profiled whether caching those nsIFrames helps at all? Or could GetPrimaryFrameFor be used always.

(In reply to comment #11) > Btw, has anyone profiled whether caching those nsIFrames > helps at all? Or could GetPrimaryFrameFor be used always. No I actually haven't, but it's also an issue of bloat. I doubt it helps with speed all that much -- and we certainly don't need to cache the frames for links. However, I liked the idea of keeping the primary frame map size down. By not asking for the primary frame of every text node we keep the map from expanding. In any case even if we removed the caching it would fix this bug, but there's still an issue of not firing the accessibility EVENT_SHOW events for stuff that's made visible via an indirect style change.

I think this will be fixed by the fix for bug 354745. There's a patch in there now for testing.

I can't repro. I believe it was caused by bug 354745, please reopen if it's still a problem.

The second patch is needed to fix this on branch. The branch bug is bug 358722.