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)
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)
(deleted),
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
dveditz
:
approval1.9.1.8+
|
Details | Diff | Splinter Review |
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?
Reporter | ||
Comment 1•15 years ago
|
||
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 :)
Reporter | ||
Comment 2•15 years ago
|
||
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>*) ]
Comment 3•15 years ago
|
||
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
Comment 4•15 years ago
|
||
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.
Comment 5•15 years ago
|
||
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
Comment 6•15 years ago
|
||
at any rate this one should really be blocking.
Comment 7•15 years ago
|
||
Tomcat has hardware dep enabled. Would that be responsible for the difference?
Assignee | ||
Comment 8•15 years ago
|
||
Zack, can you debug this?
Comment 9•15 years ago
|
||
--> Layout:Frames
Roc: make a call on blocking? Feels like it probably should.
Component: General → Layout: HTML Frames
QA Contact: general → layout.html-frames
Assignee | ||
Comment 10•15 years ago
|
||
Given we can reproduce, yes.
Flags: blocking1.9.2? → blocking1.9.2+
Comment 11•15 years ago
|
||
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.
Assignee | ||
Comment 12•15 years ago
|
||
I crash instantly on Mac trunk.
Reporter | ||
Comment 13•15 years ago
|
||
(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.
Updated•15 years ago
|
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 | ||
Updated•15 years ago
|
Assignee: nobody → roc
Assignee | ||
Comment 14•15 years ago
|
||
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!
Assignee | ||
Comment 15•15 years ago
|
||
This fixes the crash, and the site seems to work too.
I'm not sure about a testcase, though.
Attachment #412387 -
Flags: review?(bzbarsky)
Assignee | ||
Updated•15 years ago
|
Whiteboard: [sg:critical?] → [sg:critical?][needs review]
Assignee | ||
Comment 16•15 years ago
|
||
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.
Comment 17•15 years ago
|
||
> 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.
Comment 18•15 years ago
|
||
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).
Updated•15 years ago
|
Attachment #412387 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Updated•15 years ago
|
Keywords: testcase-wanted
Whiteboard: [sg:critical?][needs review] → [sg:critical?][needs landing]
Assignee | ||
Comment 19•15 years ago
|
||
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]
Assignee | ||
Comment 20•15 years ago
|
||
Backed out on suspicion of regressing content/html/document/test/test_bug369370.html
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 21•15 years ago
|
||
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)?
Assignee | ||
Comment 22•15 years ago
|
||
Yes, this patch definitely caused the regression.
Assignee | ||
Updated•15 years ago
|
Whiteboard: [sg:critical?][needs 192 landing] → [sg:critical?]
Assignee | ||
Comment 23•15 years ago
|
||
Should I implement the suggestion in comment #21?
Comment 24•15 years ago
|
||
If it fixes the regression, I think so.
Assignee | ||
Comment 25•15 years ago
|
||
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.
Assignee | ||
Comment 26•15 years ago
|
||
Looks like the test script childLoaded runs when the image frame is still set to the alt text.
Comment 27•15 years ago
|
||
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?
Assignee | ||
Comment 28•15 years ago
|
||
(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.
Assignee | ||
Comment 29•15 years ago
|
||
It looks like nsImageFrame::OnStartContainer never fires to switch us out of displaying the alt text.
Assignee | ||
Comment 30•15 years ago
|
||
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.
Assignee | ||
Comment 31•15 years ago
|
||
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
Assignee | ||
Comment 32•15 years ago
|
||
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.
Assignee | ||
Comment 33•15 years ago
|
||
Fix the regression by simply not even trying to launch MaybeLoadImage if image loading is disabled.
Attachment #413498 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 34•15 years ago
|
||
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.
Updated•15 years ago
|
Attachment #413498 -
Flags: review?(bzbarsky) → review+
Comment 35•15 years ago
|
||
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+
Assignee | ||
Updated•15 years ago
|
Whiteboard: [sg:critical?] → [sg:critical?][needs landing]
Comment 36•15 years ago
|
||
Pushed the regression fix as:
http://hg.mozilla.org/mozilla-central/rev/711a206bccb0
The CheckOverflow patch as:
http://hg.mozilla.org/mozilla-central/rev/7e93778b4b14
The fix for this bug as:
http://hg.mozilla.org/mozilla-central/rev/e09eac0b5c0a
Status: REOPENED → RESOLVED
Closed: 15 years ago → 15 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•15 years ago
|
Whiteboard: [sg:critical?][needs landing] → [sg:critical?][needs 192 landing]
Comment 37•15 years ago
|
||
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.
status1.9.2:
--- → final-fixed
Whiteboard: [sg:critical?][needs 192 landing] → [sg:critical?]
Updated•15 years ago
|
blocking1.9.1: --- → ?
Updated•15 years ago
|
blocking1.9.1: ? → .7+
Whiteboard: [sg:critical?] → [sg:critical?][needs 1.9.1 rollup patch]
Comment 38•15 years ago
|
||
Can we get the 1.9.1 rollup patch (fix+regression fix) here so we can resolve this for 1.9.1.8?
Assignee | ||
Comment 39•15 years ago
|
||
Attachment #423593 -
Flags: approval1.9.1.9?
Comment 40•15 years ago
|
||
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+
Updated•15 years ago
|
Attachment #423593 -
Flags: approval1.9.1.9+ → approval1.9.1.8+
Updated•15 years ago
|
Whiteboard: [sg:critical?][needs 1.9.1 rollup patch] → [sg:critical?][needs 1.9.1 landing]
Assignee | ||
Comment 41•15 years ago
|
||
Assignee | ||
Updated•15 years ago
|
Whiteboard: [sg:critical?][needs 1.9.1 landing] → [sg:critical?]
Comment 42•15 years ago
|
||
Did the testcase ever get created for this? It looks like it didn't when the regression took over priority.
Reporter | ||
Comment 43•15 years ago
|
||
(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 ?
Comment 44•15 years ago
|
||
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.
Comment 45•15 years ago
|
||
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]
Updated•14 years ago
|
Group: core-security
Updated•13 years ago
|
Crash Signature: [@ nsIFrame::GetView() ]
Updated•6 years ago
|
Product: Core → Core Graveyard
Updated•6 years ago
|
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.
Description
•