334080 - crashes due to null mBodyContent in nsImageDocument::CheckOverflowing [@ nsStyleContext::GetStyleData]

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P1)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

One of the top crashes for Firefox 1.5.0.2 (and 1.5.0.1, I think) is two related stacks in nsImageDocument.  On Windows, they show up with the top slightly garbled:

nsStyleContext::GetStyleData  [c:/builds/tinderbox/Fx-Mozilla1.8.0/WINNT_5.2_Depend/mozilla/layout/style/nsStyleContext.cpp, line 248]
nsImageDocument::HandleEvent  [c:/builds/tinderbox/Fx-Mozilla1.8.0/WINNT_5.2_Depend/mozilla/content/html/document/src/nsImageDocument.cpp, line 566]
nsEventListenerManager::HandleEventSubType  [c:/builds/tinderbox/Fx-Mozilla1.8.0/WINNT_5.2_Depend/mozilla/content/events/src/nsEventListenerManager.cpp, line 1684]
nsEventListenerManager::HandleEvent  [c:/builds/tinderbox/Fx-Mozilla1.8.0/WINNT_5.2_Depend/mozilla/content/events/src/nsEventListenerManager.cpp, line 1785]
nsGlobalWindow::HandleDOMEvent  [c:/builds/tinderbox/Fx-Mozilla1.8.0/WINNT_5.2_Depend/mozilla/dom/src/base/nsGlobalWindow.cpp, line 1601]
PresShell::FireResizeEvent  [c:/builds/tinderbox/Fx-Mozilla1.8.0/WINNT_5.2_Depend/mozilla/layout/base/nsPresShell.cpp, line 3111]
nsTimerImpl::Fire  [c:/builds/tinderbox/Fx-Mozilla1.8.0/WINNT_5.2_Depend/mozilla/xpcom/threads/nsTimerImpl.cpp, line 394]
nsAppStartup::Run  [c:/builds/tinderbox/Fx-Mozilla1.8.0/WINNT_5.2_Depend/mozilla/toolkit/components/startup/src/nsAppStartup.cpp, line 151]
main  [c:/builds/tinderbox/Fx-Mozilla1.8.0/WINNT_5.2_Depend/mozilla/browser/app/nsBrowserApp.cpp, line 61]

or 

nsStyleContext::GetStyleData  [c:/builds/tinderbox/Fx-Mozilla1.8.0/WINNT_5.2_Depend/mozilla/layout/style/nsStyleContext.cpp, line 248]
nsImageDocument::OnStartContainer  [c:/builds/tinderbox/Fx-Mozilla1.8.0/WINNT_5.2_Depend/mozilla/content/html/document/src/nsImageDocument.cpp, line 508]
imgRequest::OnStartContainer  [c:/builds/tinderbox/Fx-Mozilla1.8.0/WINNT_5.2_Depend/mozilla/modules/libpr0n/src/imgRequest.cpp, line 458]
imgRequest::OnDataAvailable  [c:/builds/tinderbox/Fx-Mozilla1.8.0/WINNT_5.2_Depend/mozilla/modules/libpr0n/src/imgRequest.cpp, line 886]
ProxyListener::OnDataAvailable  [c:/builds/tinderbox/Fx-Mozilla1.8.0/WINNT_5.2_Depend/mozilla/modules/libpr0n/src/imgLoader.cpp, line 893]
nsMediaDocumentStreamListener::OnDataAvailable  [c:/builds/tinderbox/Fx-Mozilla1.8.0/WINNT_5.2_Depend/mozilla/content/html/document/src/nsMediaDocument.cpp, line 115]
nsDocumentOpenInfo::OnDataAvailable  [c:/builds/tinderbox/Fx-Mozilla1.8.0/WINNT_5.2_Depend/mozilla/uriloader/base/nsURILoader.cpp, line 374]
nsStreamListenerTee::OnDataAvailable  [c:/builds/tinderbox/Fx-Mozilla1.8.0/WINNT_5.2_Depend/mozilla/netwerk/base/src/nsStreamListenerTee.cpp, line 97]
nsHttpChannel::OnDataAvailable  [c:/builds/tinderbox/Fx-Mozilla1.8.0/WINNT_5.2_Depend/mozilla/netwerk/protocol/http/src/nsHttpChannel.cpp, line 4195]
...

However, on Linux, I saw (from 1.5.0.1) a slightly better stack for the latter, which I'll attach with more detailed analysis.  I got enough out of the raw stack data and compared to the disassembly of the release build to get local variables, and from this determined that the crash is due to mBodyContent (or at least |content|) being null in CheckOverflowing, which is called from OnStartContainer (in this case) or HandleEvent at the lines indicated in the Windows stacks.

ResolveStyleFor handles a null input and returns null, but then CheckOverflowing dereferences that result.

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: detailed analysis of a Linux Fx 1.5.0.1 incident

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch

Null check the result of QIing mBodyContent just like the one other use in this file.

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Checked in to trunk.

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Fix checked in to MOZILLA_1_8_BRANCH.

Comment 5

[Daniel Veditz [:dveditz]]

Comment on attachment 218494 [details] [diff] [review]
patch

approved for 1.8.0 branch, a=dveditz for drivers

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Fix checked in to MOZILLA_1_8_0_BRANCH.

Comment 7

[Adam Guthrie]

*** Bug 337644 has been marked as a duplicate of this bug. ***

Comment 8

[Phil Ringnalda (:philor)]

*** Bug 336280 has been marked as a duplicate of this bug. ***

Comment 9

[Adam Kowalczyk]

*** Bug 337703 has been marked as a duplicate of this bug. ***

Comment 10

[Dave Liebreich [:davel]]

I've checked the 1504 candidate build against the urls listed in the bugs dup'd against this bug.  no crashes.