Closed Bug 528493 Opened 15 years ago Closed 15 years ago

Crash [@ nsIFrame::GetView() ] Data from Faulting Address may be used as a return value starting at gklayout!nsRuleNode::GetPresContext+0x000000000000000a (Hash=0x39561 c53.0x607d3622)

Categories

(Core :: Layout: Images, Video, and HTML Frames, defect)

1.9.2 Branch
x86
All
defect
Not set
critical

Tracking

()

RESOLVED FIXED
Tracking Status
status1.9.2 --- beta4-fixed
blocking1.9.1 --- .8+
status1.9.1 --- .8-fixed

People

(Reporter: cbook, Assigned: roc)

References

(Blocks 1 open bug, )

Details

(Keywords: crash, Whiteboard: [sg:critical?][need to know if this affects 1.9.0.x])

Crash Data

Attachments

(4 files)

Steps to reproduce - (reproduced with 1.9.2 debug build and 1.9.1.5 Opt build) -> Loading http://www.under-my-skin.fr/indexx.php ->> Crashes on this Site found during the testrun for Bug 526587 (51c.748): Access violation - code c0000005 (!!! second chance !!!) eax=0000001c ebx=7ffd6000 ecx=0000001c edx=0629a4b0 esi=0012f3e8 edi=0012f41c eip=02e0a6ba esp=0012f2dc ebp=0012f2e0 iopl=0 nv up ei pl nz na pe nc cs=001b ss=0023 ds=0023 es=0023 fs=003b gs=0000 efl=00000206 gklayout!nsRuleNode::GetPresContext+0xa: 02e0a6ba 8b00 mov eax,dword ptr [eax] ds:0023:0000001c=???????? 0:000> cdb: Reading initial command '!load winext\msec.dll;.logappend;!exploitab le;k;q' Opened log file 'dbgeng.log' Exploitability Classification: UNKNOWN Recommended Bug Title: Data from Faulting Address may be used as a return value starting at gklayout!nsRuleNode::GetPresContext+0x000000000000000a (Hash=0x39561 c53.0x607d3622) The data from the faulting address may later be used as a return value from this function. ChildEBP RetAddr 0012f2e0 02e0a69d gklayout!nsRuleNode::GetPresContext+0xa 0012f2ec 02eabe15 gklayout!nsIFrame::PresContext+0x1d 0012f308 02ea5be2 gklayout!nsIFrame::GetProperty+0x25 0012f32c 02e71864 gklayout!nsIFrame::GetView+0x32 0012f344 02ea5e63 gklayout!nsLayoutUtils::GetCrossDocParentFrame+0x24 0012f37c 02e6eb80 gklayout!nsIFrame::GetOffsetTo+0x53 0012f3c0 02e70ad5 gklayout!PluginBoundsEnumerator+0x30 0012f3d0 00286503 gklayout!nsTHashtable<nsPtrHashKey<nsObjectFrame> >::s_EnumStu b+0x15 0012f41c 02e6f5d4 xpcom_core!PL_DHashTableEnumerate+0x113 0012f43c 02e6e947 gklayout!nsTHashtable<nsPtrHashKey<nsObjectFrame> >::Enumerate Entries+0x54 0012f784 02e6edd1 gklayout!nsRootPresContext::GetPluginGeometryUpdates+0xe7 0012f7a4 02e1354f gklayout!nsRootPresContext::UpdatePluginGeometry+0x21 0012f940 02e13b14 gklayout!PresShell::DoReflow+0x68f 0012f97c 02e0b542 gklayout!PresShell::ProcessReflowCommands+0xf4 0012f9ac 02f0d1a7 gklayout!PresShell::FlushPendingNotifications+0x212 0012f9c4 0030c82a gklayout!nsGfxScrollFrameInner::AsyncScrollPortEvent::Run+0x37 0012fa00 00297b43 xpcom_core!nsThread::ProcessNextEvent+0x1fa 0012fa1c 01e4cead xpcom_core!NS_ProcessNextEvent_P+0x53 0012fa30 020a42db gkwidget!nsBaseAppShell::Run+0x5d 0012fa44 1000c302 tkitcmps!nsAppStartup::Run+0x6b quit:
Flags: blocking1.9.2?
also crash report id : http://crash-stats.mozilla.com/report/index/41c453a9-241e-4228-b8ab-540a12091113 Note: for the steps to reproduce it might be that the page does not crash in the first seconds. For the crash report above on mac i loaded http://www.under-my-skin.fr/indexx.php left the tab open and checked my mail for 30 seconds...and boom -> Crash :)
On Windows XP Firefox 3.6 Beta 2 crashed here on load http://www.under-my-skin.fr/indexx.php http://crash-stats.mozilla.com/report/index/bp-9e62d401-9d45-40d5-a0da-26b992091113 Firefox 3.6b2 Crash Report [@nsIFrame::GetView() ] for some reasons on a second try with the same build i got: http://crash-stats.mozilla.com/report/index/541e5f2e-ad45-43ae-95a1-ab4402091113 Firefox 3.6b2 Crash Report [@nsObjectFrame::ComputeWidgetGeometry(nsRegion const&, nsPoint const&, nsTArray<nsIWidget::Configuration>*) ]
so I guess one question is weather these two crashes we see on the test url by a random user are the same crashes that tomcat has reproduced? and the stacks don't look quite the same. 20091109-crashdata.csv:nsIFrame::GetView() http://www.under-my-skin.fr/indexx.php http://crash-stats.mozilla.com/report/index/a514eb04-8e43-42fc-b28d-102472091109 200911090246 200911090246 94694 Firefox 3.6b1 20091029171059 1.9.2 Windows NT 5.1.2600 Service Pack 3 x86 0xfffffffff0dea7ff \N 20091109-crashdata.csv:nsIFrame::GetView() http://www.under-my-skin.fr/indexx.php http://crash-stats.mozilla.com/report/index/af304ed7-6d74-4a94-a3f2-e18132091109 200911090248 200911090249 177 Firefox 3.6b1 20091029171059 1.9.2 Windows NT 5.1.2600 Service Pack 3 x86 0xfffffffff0dea7ff \N
the crashes time proximity of the two crash reports also makes it appear that it might be the same user, or content that two users hit at about the same time.
the first crash report in comment 2 matches random-user's crash, but the crash in comment 0 shows tomcat got a bit further past GetView() ChildEBP RetAddr 0012f2e0 02e0a69d gklayout!nsRuleNode::GetPresContext+0xa 0012f2ec 02eabe15 gklayout!nsIFrame::PresContext+0x1d 0012f308 02ea5be2 gklayout!nsIFrame::GetProperty+0x25 0012f32c 02e71864 gklayout!nsIFrame::GetView+0x32
at any rate this one should really be blocking.
Tomcat has hardware dep enabled. Would that be responsible for the difference?
Zack, can you debug this?
--> Layout:Frames Roc: make a call on blocking? Feels like it probably should.
Component: General → Layout: HTML Frames
QA Contact: general → layout.html-frames
Given we can reproduce, yes.
Flags: blocking1.9.2? → blocking1.9.2+
On a 3.5.6pre windows debug build I get a completely different stack (long chain of various frame ::Destroy()s) ultimately crashing in nsSupportsArray::Clear() trying to release mArray[0] which is 0xdddddddd We need to capture the page and try to create a reduced testcase before it changes.
Keywords: testcase-wanted
Whiteboard: [sg:critical?]
I crash instantly on Mac trunk.
(In reply to comment #11) > We need to capture the page and try to create a reduced testcase before it > changes. i have captured the page but not able to crash on the local copy so far (In reply to comment #12) > I crash instantly on Mac trunk. confirmed, it seems from the testing i did so far, as much as we go to a newer branch (like 1.9.2/1.9.3) we crash sooner ? like 3.5.5 took a while here for the crash. Also submitting the 2 crash reports i got during this test (http://crash-stats.mozilla.com/report/index/1d920d18-b771-4c67-9d77-51beb2091113 and http://crash-stats.mozilla.com/report/index/bp-5ae29339-b487-4faf-b0a0-8c2292091113) - breakpad display for the latter one bug 471493 which was fixed in April 2009.
Summary: Data from Faulting Address may be used as a return value starting at gklayout!nsRuleNode::GetPresContext+0x000000000000000a (Hash=0x39561 c53.0x607d3622) → Crash [@ nsIFrame::GetView() ] Data from Faulting Address may be used as a return value starting at gklayout!nsRuleNode::GetPresContext+0x000000000000000a (Hash=0x39561 c53.0x607d3622)
Assignee: nobody → roc
OK, here's the problem stack: #0 DocumentViewerImpl::InitPresentationStuff (this=0xe225a40, aDoInitialReflow=0, aReenableRefresh=0) at /Users/roc/mozilla-checkin/layout/base/nsDocumentViewer.cpp:713 #1 0x02c958f0 in DocumentViewerImpl::Show (this=0xe225a40) at /Users/roc/mozilla-checkin/layout/base/nsDocumentViewer.cpp:2002 #2 0x047f8ff6 in nsDocShell::SetVisibility (this=0xf71ea60, aVisibility=1) at /Users/roc/mozilla-checkin/docshell/base/nsDocShell.cpp:4552 #3 0x02fb096b in nsFrameLoader::Show (this=0xf71e650, marginWidth=1, marginHeight=1, scrollbarPrefX=2, scrollbarPrefY=2, frame=0x967e288) at /Users/roc/mozilla-checkin/content/base/src/nsFrameLoader.cpp:530 #4 0x02d30d41 in nsSubDocumentFrame::ShowViewer (this=0x967e258) at /Users/roc/mozilla-checkin/layout/generic/nsFrameFrame.cpp:324 #5 0x02d30feb in nsSubDocumentFrame::Init (this=0x967e258, aContent=0xf71e720, aParent=0x967e1d0, aPrevInFlow=0x0) at /Users/roc/mozilla-checkin/layout/generic/nsFrameFrame.cpp:289 #6 0x02c48ec3 in nsCSSFrameConstructor::InitAndRestoreFrame (this=0xf3d62e0, aState=@0xbfffba14, aContent=0xf71e720, aParentFrame=0x967e1d0, aPrevInFlow=0x0, aNewFrame=0x967e258, aAllowCounters=1) at /Users/roc/mozilla-checkin/layout/base/nsCSSFrameConstructor.cpp:4695 #7 0x02c511bd in nsCSSFrameConstructor::ConstructFrameFromItemInternal (this=0xf3d62e0, aItem=@0xf73d340, aState=@0xbfffba14, aParentFrame=0x967e1d0, aFrameItems=@0xbfff9d74) at /Users/roc/mozilla-checkin/layout/base/nsCSSFrameConstructor.cpp:3906 #8 0x02c51969 in nsCSSFrameConstructor::ConstructFramesFromItem (this=0xf3d62e0, aState=@0xbfffba14, aIter=@0xbfff9cb4, aParentFrame=0x967e1d0, aFrameItems=@0xbfff9d74) at /Users/roc/mozilla-checkin/layout/base/nsCSSFrameConstructor.cpp:5600 #9 0x02c51afb in nsCSSFrameConstructor::ConstructFramesFromItemList (this=0xf3d62e0, aState=@0xbfffba14, aItems=@0xf73d27c, aParentFrame=0x967e1d0, aFrameItems=@0xbfff9d74) at /Users/roc/mozilla-checkin/layout/base/nsCSSFrameConstructor.cpp:9575 ... #42 0x02c526c2 in nsCSSFrameConstructor::ProcessChildren (this=0xf3d62e0, aState=@0xbfffba14, aContent=0xf717330, aStyleContext=0x965dc88, aFrame=0x965dcf0, aCanHaveGeneratedContent=1, aFrameItems=@0xbfffb91c, aAllowBlockStyles=1, aPendingBinding=0x0) at /Users/roc/mozilla-checkin/layout/base/nsCSSFrameConstructor.cpp:9683 #43 0x02c53345 in nsCSSFrameConstructor::ConstructBlock (this=0xf3d62e0, aState=@0xbfffba14, aDisplay=0x160fca0, aContent=0xf717330, aParentFrame=0x160ffe0, aContentParentFrame=0x160ffe0, aStyleContext=0x965dc88, aNewFrame=0xbfffbabc, aFrameItems=@0xbfffba94, aAbsPosContainer=0, aPendingBinding=0x0) at /Users/roc/mozilla-checkin/layout/base/nsCSSFrameConstructor.cpp:10726 #44 0x02c57ed3 in nsCSSFrameConstructor::ConstructDocElementFrame (this=0xf3d62e0, aDocElement=0xf717330, aFrameState=0x0, aNewFrame=0xbfffbbe0) at /Users/roc/mozilla-checkin/layout/base/nsCSSFrameConstructor.cpp:2627 #45 0x02c58391 in nsCSSFrameConstructor::ContentInserted (this=0xf3d62e0, aContainer=0x0, aChild=0xf717330, aIndexInContainer=0, aFrameState=0x0) at /Users/roc/mozilla-checkin/layout/base/nsCSSFrameConstructor.cpp:6640 #46 0x02cd32d2 in PresShell::InitialReflow (this=0xf3d5f80, aWidth=63300, aHeight=31260) at /Users/roc/mozilla-checkin/layout/base/nsPresShell.cpp:2621 #47 0x02f414c9 in nsContentSink::StartLayout (this=0x212c1a00, aIgnorePendingSheets=1) at /Users/roc/mozilla-checkin/content/base/src/nsContentSink.cpp:1326 #48 0x0311f266 in HTMLContentSink::StartLayout (this=0x212c1a00, aIgnorePendingSheets=1) at /Users/roc/mozilla-checkin/content/html/document/src/nsHTMLContentSink.cpp:2621 #49 0x031200bb in HTMLContentSink::FlushPendingNotifications (this=0x212c1a00, aType=Flush_Layout) at /Users/roc/mozilla-checkin/content/html/document/src/nsHTMLContentSink.cpp:3029 #50 0x02f92303 in nsDocument::FlushPendingNotifications (this=0x215d6400, aType=Flush_Layout) at /Users/roc/mozilla-checkin/content/base/src/nsDocument.cpp:6327 #51 0x02f92387 in nsDocument::FlushPendingNotifications (this=0x133f400, aType=Flush_Style) at /Users/roc/mozilla-checkin/content/base/src/nsDocument.cpp:6350 #52 0x02fe32bd in nsObjectLoadingContent::NotifyStateChanged (this=0xf735f60, aOldType=nsObjectLoadingContent::eType_Loading, aOldState=2097152, aSync=1) at /Users/roc/mozilla-checkin/content/base/src/nsObjectLoadingContent.cpp:1528 #53 0x02fe853c in AutoNotifier::Notify (this=0xbfffc194) at /Users/roc/mozilla-checkin/content/base/src/nsObjectLoadingContent.cpp:234 #54 0x02fe4c96 in nsObjectLoadingContent::LoadObject (this=0xf735f60, aURI=0xf736230, aNotify=1, aTypeHint=@0xbfffc36c, aForceLoad=0) at /Users/roc/mozilla-checkin/content/base/src/nsObjectLoadingContent.cpp:1174 #55 0x02fe5a0e in nsObjectLoadingContent::LoadObject (this=0xf735f60, aURI=@0xbfffc2d8, aNotify=1, aTypeHint=@0xbfffc36c, aForceLoad=0) at /Users/roc/mozilla-checkin/content/base/src/nsObjectLoadingContent.cpp:991 #56 0x030e6afd in nsHTMLSharedObjectElement::StartObjectLoad (this=0xf735f40, aNotify=1) at /Users/roc/mozilla-checkin/content/html/content/src/nsHTMLSharedObjectElement.cpp:432 #57 0x030e860d in nsHTMLSharedObjectElement::StartObjectLoad (this=0xf735f40) at /Users/roc/mozilla-checkin/content/html/content/src/nsHTMLSharedObjectElement.cpp:133 #58 0x030e87be in nsRunnableMethod<nsHTMLSharedObjectElement, void>::Run (this=0xf736080) at nsThreadUtils.h:282 #59 0x02f53f4a in nsContentUtils::RemoveScriptBlocker () at /Users/roc/mozilla-checkin/content/base/src/nsContentUtils.cpp:4485 #60 0x02d3a6a1 in nsContentUtils::RemoveRemovableScriptBlocker () at nsContentUtils.h:1403 #61 0x02d3acdf in mozAutoDocUpdate::~mozAutoDocUpdate (this=0xbfffc58c) at mozAutoDocUpdate.h:69 #62 0x02d3ad05 in mozAutoDocUpdate::~mozAutoDocUpdate (this=0xbfffc58c) at mozAutoDocUpdate.h:74 #63 0x02fc7472 in nsGenericElement::doInsertChildAt (aKid=0xf735f40, aIndex=0, aNotify=0, aParent=0xf735e10, aDocument=0x133f400, aChildArray=@0xf735e2c) at /Users/roc/mozilla-checkin/content/base/src/nsGenericElement.cpp:3313 #64 0x02fc74ff in nsGenericElement::InsertChildAt (this=0xf735e10, aKid=0xf735f40, aIndex=0, aNotify=0) at /Users/roc/mozilla-checkin/content/base/src/nsGenericElement.cpp:3227 #65 0x02c40a51 in nsINode::AppendChildTo (this=0xf735e10, aKid=0xf735f40, aNotify=0) at nsINode.h:428 #66 0x031461f4 in nsPluginDocument::CreateSyntheticPluginDocument (this=0x133f400) at /Users/roc/mozilla-checkin/content/html/document/src/nsPluginDocument.cpp:320 #67 0x0314628d in nsPluginDocument::SetScriptGlobalObject (this=0x133f400, aScriptGlobalObject=0xf72fdd0) at /Users/roc/mozilla-checkin/content/html/document/src/nsPluginDocument.cpp:205 #68 0x03281d40 in nsGlobalWindow::SetNewDocument (this=0xf71ef30, aDocument=0x133f400, aState=0x0, aClearScopeHint=1, aIsInternalCall=0) at /Users/roc/mozilla-checkin/dom/base/nsGlobalWindow.cpp:1986 #69 0x03282486 in nsGlobalWindow::SetNewDocument (this=0xf71ef30, aDocument=0x133f400, aState=0x0, aClearScopeHint=1) at /Users/roc/mozilla-checkin/dom/base/nsGlobalWindow.cpp:1550 #70 0x02c965a0 in DocumentViewerImpl::InitInternal (this=0xe225a40, aParentWidget=0x0, aState=0x0, aBounds=@0xbfffcdbc, aDoCreation=1, aInPrintPreview=0, aNeedMakeCX=1) at /Users/roc/mozilla-checkin/layout/base/nsDocumentViewer.cpp:952 #71 0x02c96d4f in DocumentViewerImpl::Init (this=0xe225a40, aParentWidget=0x0, aBounds=@0xbfffcdbc) at /Users/roc/mozilla-checkin/layout/base/nsDocumentViewer.cpp:697 #72 0x0480d7cf in nsDocShell::SetupNewViewer (this=0xf71ea60, aNewViewer=0xe225a40) at /Users/roc/mozilla-checkin/docshell/base/nsDocShell.cpp:7289 #73 0x04801e42 in nsDocShell::Embed (this=0xf71ea60, aContentViewer=0xe225a40, aCommand=0x488c14c "", aExtraInfo=0x0) at /Users/roc/mozilla-checkin/docshell/base/nsDocShell.cpp:5478 #74 0x048158e5 in nsDocShell::CreateContentViewer (this=0xf71ea60, aContentType=0xf702cc8 "application/x-shockwave-flash", request=0xf7205f0, aContentHandler=0xf7204e0) at /Users/roc/mozilla-checkin/docshell/base/nsDocShell.cpp:7074 #75 0x0483dbce in nsDSURIContentListener::DoContent (this=0xf71e790, aContentType=0xf702cc8 "application/x-shockwave-flash", aIsContentPreferred=0, request=0xf7205f0, aContentHandler=0xf7204e0, aAbortProcess=0xbfffd074) at /Users/roc/mozilla-checkin/docshell/base/nsDSURIContentListener.cpp:138 #76 0x0484762e in nsDocumentOpenInfo::TryContentListener (this=0xf7204d0, aListener=0xf71e790, aChannel=0xf7205f0) at /Users/roc/mozilla-checkin/uriloader/base/nsURILoader.cpp:736 #77 0x04847e44 in nsDocumentOpenInfo::DispatchContent (this=0xf7204d0, request=0xf7205f0, aCtxt=0x0) at /Users/roc/mozilla-checkin/uriloader/base/nsURILoader.cpp:434 #78 0x04848b48 in nsDocumentOpenInfo::OnStartRequest (this=0xf7204d0, request=0xf7205f0, aCtxt=0x0) at /Users/roc/mozilla-checkin/uriloader/base/nsURILoader.cpp:280 #79 0x01d4e3fa in nsHttpChannel::CallOnStartRequest (this=0xf7205c0) at /Users/roc/mozilla-checkin/netwerk/protocol/http/src/nsHttpChannel.cpp:839 The basic problem with this stack is that we're reentering nsDocumentViewer::InitPresentationStuff on the same document viewer!
Attached patch fix (deleted) — Splinter Review
This fixes the crash, and the site seems to work too. I'm not sure about a testcase, though.
Attachment #412387 - Flags: review?(bzbarsky)
Whiteboard: [sg:critical?] → [sg:critical?][needs review]
Like Tomcat, saving the page and then loading it locally doesn't crash for me. I also tried creating a synthetic testcase that loads the Flash object into an <iframe>, can't get that to crash either.
> I'm not sure about a testcase, though. Looking at the stack, something like this might work: <head> <link rel="stylesheet" href="something that will load slowly"> </head> <body> <iframe src="something that loads faster and has a plug-in type; maybe just 'data:plugin/type,'"></iframe> </body> The key is to guarantee getting OnStartRequest for the iframe before the stylesheet load finishes and calls StartLayout. The other option that might trigger this sort of thing is trying to make sure a reframe style change is in flight for the <iframe> when the OnStartRequest happens, but that's hard.
And in fact I just confirmed that this testcase: <!DOCTYPE html> <html> <head> <link rel="stylesheet" type="text/css" href="http://www.hixie.ch/tests/adhoc/html/parsing/script-style-blocking/slow-style.css"> </head> <body> <iframe src="data:application/x-test," id="x"></iframe> </body> </html> ends up with exactly the reentry stack in this bug and various asserts in a debug build (but not a reliable crash for me).
Attachment #412387 - Flags: review?(bzbarsky) → review+
Keywords: testcase-wanted
Whiteboard: [sg:critical?][needs review] → [sg:critical?][needs landing]
http://hg.mozilla.org/mozilla-central/rev/93a0acf68dd6 Still need to write the test and land it.
Status: NEW → RESOLVED
Closed: 15 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Whiteboard: [sg:critical?][needs landing] → [sg:critical?][needs 192 landing]
Backed out on suspicion of regressing content/html/document/test/test_bug369370.html
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Hmm. Are we being hit by imagelib firing notifications sync again? :( Maybe we need to put the CheckOverflowing call in nsImageDocument::OnStartContainer off on a script runner (so it'll happen sync if we're not in a batch, but once the batch ends and it's ok to do the flushing CheckOverflowing wants to do otherwise)?
Yes, this patch definitely caused the regression.
Whiteboard: [sg:critical?][needs 192 landing] → [sg:critical?]
Should I implement the suggestion in comment #21?
If it fixes the regression, I think so.
It doesn't. I haven't yet figured out what caused this regression. It seems PresShell::FlushPendingNotifications doesn't get called when it's unsafe to run scripts.
Looks like the test script childLoaded runs when the image frame is still set to the alt text.
When it's unsafe to run scripts the IsSafeToFlush() call at the top of PresShell::FlushPendingNotifications returns false, so the function becomes a no-op. Comment 21 was just based on looking for flushes that might be thus suppressed by the script blocker. Maybe there are more?
(In reply to comment #27) > When it's unsafe to run scripts the IsSafeToFlush() call at the top of > PresShell::FlushPendingNotifications returns false, so the function becomes a > no-op. Right, I tried setting a conditional breakpoint to catch that, but it didn't seem to happen.
It looks like nsImageFrame::OnStartContainer never fires to switch us out of displaying the alt text.
OK, that's not a problem, it seems. The problem is that with this patch, nsImageLoadingContent::OnStartContainer fires *after* we've already finished the test. Seems like onload is firing in the popup window before nsImageLoadingContent::OnStartContainer. That is definitely wrong.
hum, this is suspicious --- here is what triggers the load event #17 0x04752650 in nsDocShell::EndPageLoad (this=0xb4b0c0, aProgress=0xb4b0d4, aChannel=0xf7684b0, aStatus=2153578529) at /Users/roc/mozilla-checkin/docshell/base/nsDocShell.cpp:5730 #18 0x04746ecb in nsDocShell::OnStateChange (this=0xb4b0c0, aProgress=0xb4b0d4, aRequest=0xf7684b0, aStateFlags=131088, aStatus=2153578529) at /Users/roc/mozilla-checkin/docshell/base/nsDocShell.cpp:5608 #19 0x0478c901 in nsDocLoader::FireOnStateChange (this=0xb4b0c0, aProgress=0xb4b0d4, aRequest=0xf7684b0, aStateFlags=131088, aStatus=2153578529) at /Users/roc/mozilla-checkin/uriloader/base/nsDocLoader.cpp:1314 #20 0x0478cb39 in nsDocLoader::doStopDocumentLoad (this=0xb4b0c0, request=0xf7684b0, aStatus=2153578529) at /Users/roc/mozilla-checkin/uriloader/base/nsDocLoader.cpp:926 #21 0x0478ce5a in nsDocLoader::DocLoaderIsEmpty (this=0xb4b0c0, aFlushLayout=1) at /Users/roc/mozilla-checkin/uriloader/base/nsDocLoader.cpp:802 #22 0x0478d9e2 in nsDocLoader::OnStopRequest (this=0xb4b0c0, aRequest=0x89d43b0, aCtxt=0x0, aStatus=0) at /Users/roc/mozilla-checkin/uriloader/base/nsDocLoader.cpp:697 #23 0x01e1dd93 in nsLoadGroup::RemoveRequest (this=0xb4b2b0, request=0x89d43b0, ctxt=0x0, aStatus=0) at /Users/roc/mozilla-checkin/netwerk/base/src/nsLoadGroup.cpp:680 #24 0x027f3ecc in imgRequestProxy::RemoveFromLoadGroup (this=0x89d43b0, releaseLoadGroup=1) at /Users/roc/mozilla-checkin/modules/libpr0n/src/imgRequestProxy.cpp:194 #25 0x027f4587 in imgRequestProxy::OnStopRequest (this=0x89d43b0, request=0x0, ctxt=0x0, statusCode=2152398850, lastPart=1) at /Users/roc/mozilla-checkin/modules/libpr0n/src/imgRequestProxy.cpp:645 #26 0x027ef9c7 in imgRequest::RemoveProxy (this=0x89d4080, proxy=0x89d43b0, aStatus=2153644036, aNotify=0) at /Users/roc/mozilla-checkin/modules/libpr0n/src/imgRequest.cpp:203 #27 0x027f418a in imgRequestProxy::DoCancel (this=0x89d43b0, status=2153644036) at /Users/roc/mozilla-checkin/modules/libpr0n/src/imgRequestProxy.cpp:250 #28 0x027f52ff in imgRequestProxy::imgCancelRunnable::Run (this=0x89d6800) at imgRequestProxy.h:100
OK, here's the problem. In nsImageDocument::CreateSyntheticDocument: // Make sure not to start the image load from here... imageLoader->SetLoadingEnabled(PR_FALSE); mImageContent->SetAttr(kNameSpaceID_None, nsGkAtoms::src, srcString, PR_FALSE); mImageContent->SetAttr(kNameSpaceID_None, nsGkAtoms::alt, srcString, PR_FALSE); body->AppendChildTo(mImageContent, PR_FALSE); imageLoader->SetLoadingEnabled(PR_TRUE); If there is a script blocker in effect here, in AppendChildTo nsHTMLImageElement::BindToTree will trigger a *delayed* nsHTMLImageElement::MaybeLoadImage which ends up running after SetLoadingEnabled(PR_TRUE), so "make sure not to start the image load from here" doesn't.
Attached patch fix (deleted) — Splinter Review
Fix the regression by simply not even trying to launch MaybeLoadImage if image loading is disabled.
Attachment #413498 - Flags: review?(bzbarsky)
Attached patch non-fix (deleted) — Splinter Review
Tbis makes CheckOverflowing run off a scriptrunner in OnStartContainer like Boris suggested. Since it's not needed here, I'm not going to push for it, but you think it's the right thing to do, we can review and land on trunk.
Attachment #413498 - Flags: review?(bzbarsky) → review+
Comment on attachment 413499 [details] [diff] [review] non-fix I think this is the right thing to do, with a comment about how we need to flush to CheckOverflowing correctly, hence need to make sure we don't try to do it in a scriptblocker.
Attachment #413499 - Flags: review+
Whiteboard: [sg:critical?] → [sg:critical?][needs landing]
Status: REOPENED → RESOLVED
Closed: 15 years ago15 years ago
Resolution: --- → FIXED
Whiteboard: [sg:critical?][needs landing] → [sg:critical?][needs 192 landing]
Pushed regression fix as: http://hg.mozilla.org/releases/mozilla-1.9.2/rev/a97581dbce97 The fix for this bug as: http://hg.mozilla.org/releases/mozilla-1.9.2/rev/0113c6c01241 Not sure about the checkoverflow thing on branch; I let it be for now.
Whiteboard: [sg:critical?][needs 192 landing] → [sg:critical?]
blocking1.9.1: --- → ?
blocking1.9.1: ? → .7+
Whiteboard: [sg:critical?] → [sg:critical?][needs 1.9.1 rollup patch]
Can we get the 1.9.1 rollup patch (fix+regression fix) here so we can resolve this for 1.9.1.8?
Attached patch 1.9.1 rollup (deleted) — Splinter Review
Attachment #423593 - Flags: approval1.9.1.9?
Blocks: 533516
Comment on attachment 423593 [details] [diff] [review] 1.9.1 rollup Approved for 1.9.1.8, a=dveditz for release-drivers
Attachment #423593 - Flags: approval1.9.1.9? → approval1.9.1.9+
Attachment #423593 - Flags: approval1.9.1.9+ → approval1.9.1.8+
Whiteboard: [sg:critical?][needs 1.9.1 rollup patch] → [sg:critical?][needs 1.9.1 landing]
Whiteboard: [sg:critical?][needs 1.9.1 landing] → [sg:critical?]
Did the testcase ever get created for this? It looks like it didn't when the regression took over priority.
(In reply to comment #42) > Did the testcase ever get created for this? It looks like it didn't when the > regression took over priority. maybe comment #18 count as testcase here ?
Yes, it may (except it doesn't crash) but a testcase was going to be *checked in* for this according to comment 19. Comment 18 is just a manual case, non-crashing case.
Does this need to be fixed on the 1.9.0 branch as well?
Flags: wanted1.9.0.x?
Whiteboard: [sg:critical?] → [sg:critical?][need to know if this affects 1.9.0.x]
Blocks: 523895
Depends on: 567738
Group: core-security
No longer depends on: 567738
Crash Signature: [@ nsIFrame::GetView() ]
Product: Core → Core Graveyard
Component: Layout: HTML Frames → Layout: Images
Product: Core Graveyard → Core
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: