during plugin instantiation, frame tree that is being reflowed can be destroyed

RESOLVED WORKSFORME

Status

()

--
critical
RESOLVED WORKSFORME
17 years ago
7 years ago

People

(Reporter: alexsavulov, Assigned: dbaron)

Tracking

(Blocks: 1 bug, {crash, testcase})

Trunk
x86
All
crash, testcase
Points:
---
Dependency tree / graph
Bug Flags:
wanted-next +
blocking1.9.1 -
wanted1.9.1 +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [PL:branch], URL)

Attachments

(6 attachments, 10 obsolete attachments)

583 bytes, text/html
Details
291 bytes, text/html
Details
7.05 KB, text/plain
Details
4.75 KB, patch
jst
: review+
Details | Diff | Splinter Review
28.96 KB, patch
Details | Diff | Splinter Review
5.10 KB, text/html
Details
(Reporter)

Description

17 years ago
David Baron:

Sir, if you want to work on this issue, here is this new bug report where i will
put your last comments from bug 107545. You cannot just reopen bug 107545 (I
expected you to do that) just be cause the real problem was not solved. There is
a fix on the branch and trunk and the reporter's confirmation that the crash was
fixed. That bug has keywords that are tracked by the management and reopening it
causes confusion. Beside that by reopening that bug you just cancel the hours
of work that me and T. Greer spent to get at least a crash prevention fix. We
have other things to do too within the company. You also cancel the work the
reviewers, adt and the driver that approved the fix! I agree with you that the
real problem is still there, but I totaly disagree with your way of handling
such things.


*** transfered comments from bug 107545 **************************************

------- Additional Comment #49 From David Baron 2002-04-10 14:46 -------

I wasn't able to reproduce a crash, but I did manage to see an assertion which
is probably related:

#2  0x40267955 in nsDebug::Assertion (aStr=0x42a01ed8 "running past end", 
    aExpr=0x42a01ec2 "mCurrent != mListLink", 
    aFile=0x42a01da0 "/builds/trunk/mozilla/layout/html/base/src/nsLineBox.h", 
    aLine=537) at /builds/trunk/mozilla/xpcom/glue/nsDebug.cpp:291
#3  0x4299c0df in nsLineList_iterator::operator-> (this=0xbfffa848)
    at /builds/trunk/mozilla/layout/html/base/src/nsLineBox.h:537
#4  0x4277f604 in nsBlockFrame::ReflowLine (this=0x41ecd04c, 
    aState=@0xbfffac00, aLine={mCurrent = 0x41ee0954, mListLink = 0x41ecd088}, 
    aKeepReflowGoing=0xbfffa9e8, aDamageDirtyArea=1)
    at /builds/trunk/mozilla/layout/html/base/src/nsBlockFrame.cpp:2553
#5  0x4277e7d4 in nsBlockFrame::ReflowDirtyLines (this=0x41ecd04c, 
    aState=@0xbfffac00)
    at /builds/trunk/mozilla/layout/html/base/src/nsBlockFrame.cpp:2250
#6  0x4277b5ea in nsBlockFrame::Reflow (this=0x41ecd04c, 
    aPresContext=0x42bb2c70, aMetrics=@0xbfffb0e0, aReflowState=@0xbfffb020, 
    aStatus=@0xbfffc354)
    at /builds/trunk/mozilla/layout/html/base/src/nsBlockFrame.cpp:844

The cause of the assertion was that ReflowDirtyLines had passed a line to
ReflowLine that was no longer in the line list, but had prev and next pointers
indicating that it thought it was the only line in the line list (same list
link, but that link didn't point back to it).  The line list thought it had 5
lines, and aState.mLineNumber was 3.



------- Additional Comment #50 From David Baron 2002-04-10 15:17 -------

Of those 5 lines (I actually missed the first), the first, third, and fifth were
blocks.  The second had a single child, and the fourth had 2, and all the
sibling linkage seemed to be correct.

The line being reflowed, which was not in the list, had a child count of 2 and
its first child had 5 siblings after (so there were four siblings following not
on the line), but none of these frames were the same.  In fact, their block
parent (which should have been the |this| block) was in fact a
(great-)*5-grandchild of the |this| block, and the chain went through the first
child of the |this| block (the child of the first line).



------- Additional Comment #51 From David Baron 2002-04-10 16:26 -------

Recording which lines have already been reflown makes it look like the mLines
list was somehow entirely replaced in the middle of reflowing the children.

------- Additional Comment #53 From David Baron 2002-04-10 19:07 -------

The real problem here is that frames are being destroyed while they're in the
middle of being reflowed.  While we may be lucky enough to be able to work
around it with a single null check in this case, that's unlikely to work in
general.  We shouldn't have to protect against this.  Here's the stack showing
the real problem:

#3  0x40ca7adb in nsObjectFrame::Destroy (this=0x435c984c, 
    aPresContext=0x4307b6f0)
    at /builds/trunk/mozilla/layout/html/base/src/nsObjectFrame.cpp:633
#4  0x40c9dd27 in nsLineBox::DeleteLineList (aPresContext=0x4307b6f0, 
    aLines=@0x435c9764)
    at /builds/trunk/mozilla/layout/html/base/src/nsLineBox.cpp:311
#5  0x40c53177 in nsBlockFrame::Destroy (this=0x435c9728, 
    aPresContext=0x4307b6f0)
    at /builds/trunk/mozilla/layout/html/base/src/nsBlockFrame.cpp:328
#6  0x40c9dd27 in nsLineBox::DeleteLineList (aPresContext=0x4307b6f0, 
    aLines=@0x435c95f8)
    at /builds/trunk/mozilla/layout/html/base/src/nsLineBox.cpp:311
#7  0x40c53177 in nsBlockFrame::Destroy (this=0x435c95bc, 
    aPresContext=0x4307b6f0)
    at /builds/trunk/mozilla/layout/html/base/src/nsBlockFrame.cpp:328
#8  0x40de72f3 in nsFrameList::DestroyFrames (this=0x435c9590, 
    aPresContext=0x4307b6f0)
    at /builds/trunk/mozilla/layout/base/src/nsFrameList.cpp:130
#9  0x40c69aa8 in nsContainerFrame::Destroy (this=0x435c955c, 
    aPresContext=0x4307b6f0)
    at /builds/trunk/mozilla/layout/html/base/src/nsContainerFrame.cpp:138
#10 0x40de72f3 in nsFrameList::DestroyFrames (this=0x435c9478, 
    aPresContext=0x4307b6f0)
    at /builds/trunk/mozilla/layout/base/src/nsFrameList.cpp:130
#11 0x40c69aa8 in nsContainerFrame::Destroy (this=0x435c9444, 
    aPresContext=0x4307b6f0)
    at /builds/trunk/mozilla/layout/html/base/src/nsContainerFrame.cpp:138
#12 0x40de72f3 in nsFrameList::DestroyFrames (this=0x435d49e4, 
    aPresContext=0x4307b6f0)
    at /builds/trunk/mozilla/layout/base/src/nsFrameList.cpp:130
#13 0x40c69aa8 in nsContainerFrame::Destroy (this=0x435d49b0, 
    aPresContext=0x4307b6f0)
    at /builds/trunk/mozilla/layout/html/base/src/nsContainerFrame.cpp:138
#14 0x40de72f3 in nsFrameList::DestroyFrames (this=0x435c9324, 
    aPresContext=0x4307b6f0)
    at /builds/trunk/mozilla/layout/base/src/nsFrameList.cpp:130
#15 0x40c69aa8 in nsContainerFrame::Destroy (this=0x435c92f0, 
    aPresContext=0x4307b6f0)
    at /builds/trunk/mozilla/layout/html/base/src/nsContainerFrame.cpp:138
#16 0x40d694f6 in nsTableFrame::Destroy (this=0x435c92f0, 
    aPresContext=0x4307b6f0)
    at /builds/trunk/mozilla/layout/html/table/src/nsTableFrame.cpp:318
#17 0x40de72f3 in nsFrameList::DestroyFrames (this=0x435c914c, 
    aPresContext=0x4307b6f0)
    at /builds/trunk/mozilla/layout/base/src/nsFrameList.cpp:130
#18 0x40c69aa8 in nsContainerFrame::Destroy (this=0x435c9118, 
    aPresContext=0x4307b6f0)
    at /builds/trunk/mozilla/layout/html/base/src/nsContainerFrame.cpp:138
#19 0x40d7e8bc in nsTableOuterFrame::Destroy (this=0x435c9118, 
    aPresContext=0x4307b6f0)
    at /builds/trunk/mozilla/layout/html/table/src/nsTableOuterFrame.cpp:83
#20 0x40c9dd27 in nsLineBox::DeleteLineList (aPresContext=0x4307b6f0, 
    aLines=@0x435d40b8)
    at /builds/trunk/mozilla/layout/html/base/src/nsLineBox.cpp:311
#21 0x40c53177 in nsBlockFrame::Destroy (this=0x435d407c, 
    aPresContext=0x4307b6f0)
    at /builds/trunk/mozilla/layout/html/base/src/nsBlockFrame.cpp:328
#22 0x40de72f3 in nsFrameList::DestroyFrames (this=0x435d4050, 
    aPresContext=0x4307b6f0)
    at /builds/trunk/mozilla/layout/base/src/nsFrameList.cpp:130
#23 0x40c69aa8 in nsContainerFrame::Destroy (this=0x435d401c, 
    aPresContext=0x4307b6f0)
    at /builds/trunk/mozilla/layout/html/base/src/nsContainerFrame.cpp:138
#24 0x40de75e3 in nsFrameList::DestroyFrame (this=0x435c76a8, 
    aPresContext=0x4307b6f0, aFrame=0x435d401c)
    at /builds/trunk/mozilla/layout/base/src/nsFrameList.cpp:217
#25 0x40d83c18 in nsTableRowFrame::RemoveFrame (this=0x435c7674, 
    aPresContext=0x4307b6f0, aPresShell=@0x4357ff60, aListName=0x0, 
    aOldFrame=0x435d401c)
    at /builds/trunk/mozilla/layout/html/table/src/nsTableRowFrame.cpp:334
#26 0x40c7d669 in FrameManager::RemoveFrame (this=0x435b5658, 
    aPresContext=0x4307b6f0, aPresShell=@0x4357ff60, aParentFrame=0x435c7674, 
    aListName=0x0, aOldFrame=0x435d401c)
    at /builds/trunk/mozilla/layout/html/base/src/nsFrameManager.cpp:1014
#27 0x40d3dd0b in nsCSSFrameConstructor::ContentRemoved (this=0x43093910, 
    aPresContext=0x4307b6f0, aContainer=0x435ce310, aChild=0x435ce3e8, 
    aIndexInContainer=1, aInContentReplaced=1)
    at /builds/trunk/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp:9587
#28 0x40d3ba42 in nsCSSFrameConstructor::ContentReplaced (this=0x43093910, 
    aPresContext=0x4307b6f0, aContainer=0x435ce310, aOldChild=0x435ce3e8, 
    aNewChild=0x435ce3e8, aIndexInContainer=1)
    at /builds/trunk/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp:8917
#29 0x40d498d4 in nsCSSFrameConstructor::WipeContainingBlock (this=0x43093910, 
    aPresContext=0x4307b6f0, aState=@0xbfff5e34, aBlockContent=0x435ce3e8, 
    aFrame=0x4364fa8c, aFrameList=0x436500a4)
    at /builds/trunk/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp:13686
#30 0x40d395da in nsCSSFrameConstructor::ContentAppended (this=0x43093910, 
    aPresContext=0x4307b6f0, aContainer=0x435c7f38, aNewIndexInContainer=1)
    at /builds/trunk/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp:8266
#31 0x42567cf1 in StyleSetImpl::ContentAppended (this=0x430937d0, 
    aPresContext=0x4307b6f0, aContainer=0x435c7f38, aNewIndexInContainer=1)
    at /builds/trunk/mozilla/content/base/src/nsStyleSet.cpp:1520
#32 0x40cc6a3e in PresShell::ContentAppended (this=0x4357ff60, 
    aDocument=0x4307b960, aContainer=0x435c7f38, aNewIndexInContainer=1)
    at /builds/trunk/mozilla/layout/html/base/src/nsPresShell.cpp:5160
#33 0x424e4bda in nsDocument::ContentAppended (this=0x4307b960, 
    aContainer=0x435c7f38, aNewIndexInContainer=1)
    at /builds/trunk/mozilla/content/base/src/nsDocument.cpp:1893
#34 0x4238ab0d in nsHTMLDocument::ContentAppended (this=0x4307b960, 
    aContainer=0x435c7f38, aNewIndexInContainer=1)
    at /builds/trunk/mozilla/content/html/document/src/nsHTMLDocument.cpp:1335
#35 0x42381f6c in HTMLContentSink::NotifyAppend (this=0x430b2198, 
    aContainer=0x435c7f38, aStartIndex=1)
    at /builds/trunk/mozilla/content/html/document/src/nsHTMLContentSink.cpp:4819
#36 0x42378197 in SinkContext::FlushTags (this=0x4308add0, aNotify=1)
    at /builds/trunk/mozilla/content/html/document/src/nsHTMLContentSink.cpp:2184
#37 0x423834f1 in HTMLContentSink::FlushPendingNotifications (this=0x430b2198)
    at /builds/trunk/mozilla/content/html/document/src/nsHTMLContentSink.cpp:5268
#38 0x4238b26a in nsHTMLDocument::FlushPendingNotifications (this=0x4307b960, 
    aFlushReflows=0, aUpdateViews=0)
    at /builds/trunk/mozilla/content/html/document/src/nsHTMLDocument.cpp:1486
#39 0x424dc50b in nsContentList::GetLength (this=0x43625d48, 
    aLength=0xbfff64c0, aDoFlush=1)
    at /builds/trunk/mozilla/content/base/src/nsContentList.cpp:392
#40 0x424dc88f in nsContentList::GetLength (this=0x43625d48, 
    aLength=0xbfff64c0)
    at /builds/trunk/mozilla/content/base/src/nsContentList.cpp:475
#41 0x40cb100c in nsPluginInstanceOwner::EnsureCachedAttrParamArrays (
    this=0x43593cf0)
    at /builds/trunk/mozilla/layout/html/base/src/nsObjectFrame.cpp:2809
#42 0x40cada5a in nsPluginInstanceOwner::GetAttributes (this=0x43593cf0, 
    n=@0xbfff6626, names=@0xbfff6628, values=@0xbfff662c)
    at /builds/trunk/mozilla/layout/html/base/src/nsObjectFrame.cpp:2071
#43 0x427d9641 in nsPluginInstancePeerImpl::GetAttributes (this=0x43623480, 
    n=@0xbfff6626, names=@0xbfff6628, values=@0xbfff662c)
    at /builds/trunk/mozilla/modules/plugin/base/src/nsPluginInstancePeer.cpp:345
#44 0x428a30a2 in JavaPluginInstance5::Initialize ()
   from /usr/java/jdk1.3.1/jre/plugin/i386/ns600/libjavaplugin_oji.so
#45 0x427cf7b4 in nsPluginHostImpl::SetUpPluginInstance (this=0x840fff0, 
    aMimeType=0x40ee4090 "application/x-java-vm", aURL=0x435be698, 
    aOwner=0x43593cf0)
    at /builds/trunk/mozilla/modules/plugin/base/src/nsPluginHostImpl.cpp:3932
#46 0x427ce2f5 in nsPluginHostImpl::InstantiateEmbededPlugin (this=0x840fff0, 
    aMimeType=0x40ee4090 "application/x-java-vm", aURL=0x435be698, 
    aOwner=0x43593cf0)
    at /builds/trunk/mozilla/modules/plugin/base/src/nsPluginHostImpl.cpp:3447
#47 0x40caa3a9 in nsObjectFrame::InstantiatePlugin (this=0x435c984c, 
    aPresContext=0x4307b6f0, aMetrics=@0xbfff6ed0, aReflowState=@0xbfff6f20, 
    aPluginHost=0x840fff4, aMimetype=0x40ee4090 "application/x-java-vm", 
    aURI=0x435be698)
    at /builds/trunk/mozilla/layout/html/base/src/nsObjectFrame.cpp:1242
#48 0x40ca95f9 in nsObjectFrame::Reflow (this=0x435c984c, 
    aPresContext=0x4307b6f0, aMetrics=@0xbfff6ed0, aReflowState=@0xbfff6f20, 
    aStatus=@0xbfff70b0)
    at /builds/trunk/mozilla/layout/html/base/src/nsObjectFrame.cpp:1048
#49 0x40ca1cc8 in nsLineLayout::ReflowFrame (this=0xbfff7190, 
    aFrame=0x435c984c, aNextRCFrame=0xbfff7b1c, aReflowStatus=@0xbfff70b0, 
    aMetrics=0x0, aPushedFrame=@0xbfff70ac)
    at /builds/trunk/mozilla/layout/html/base/src/nsLineLayout.cpp:1088
#50 0x40c5b03c in nsBlockFrame::ReflowInlineFrame (this=0x435c9728, 
    aState=@0xbfff7aa0, aLineLayout=@0xbfff7190, aLine=
      {mCurrent = 0x435c9920, mListLink = 0x435c9764}, aFrame=0x435c984c, 
    aLineReflowStatus=0xbfff711f "")
    at /builds/trunk/mozilla/layout/html/base/src/nsBlockFrame.cpp:3700
#51 0x40c5ac6c in nsBlockFrame::DoReflowInlineFrames (this=0x435c9728, 
    aState=@0xbfff7aa0, aLineLayout=@0xbfff7190, aLine=
      {mCurrent = 0x435c9920, mListLink = 0x435c9764}, 
    aKeepReflowGoing=0xbfff7888, aLineReflowStatus=0xbfff765f "\002", 
    aUpdateMaximumWidth=0, aDamageDirtyArea=0)
    at /builds/trunk/mozilla/layout/html/base/src/nsBlockFrame.cpp:3582
#52 0x40c5a9dc in nsBlockFrame::DoReflowInlineFramesAuto (this=0x435c9728, 
    aState=@0xbfff7aa0, aLine={mCurrent = 0x435c9920, mListLink = 0x435c9764}, 
    aKeepReflowGoing=0xbfff7888, aLineReflowStatus=0xbfff765f "\002", 
    aUpdateMaximumWidth=0, aDamageDirtyArea=0)
    at /builds/trunk/mozilla/layout/html/base/src/nsBlockFrame.cpp:3505
#53 0x40c5a7b8 in nsBlockFrame::ReflowInlineFrames (this=0x435c9728, 
    aState=@0xbfff7aa0, aLine={mCurrent = 0x435c9920, mListLink = 0x435c9764}, 
    aKeepReflowGoing=0xbfff7888, aDamageDirtyArea=0, aUpdateMaximumWidth=0)
    at /builds/trunk/mozilla/layout/html/base/src/nsBlockFrame.cpp:3451
#54 0x40c588a2 in nsBlockFrame::ReflowLine (this=0x435c9728, 
    aState=@0xbfff7aa0, aLine={mCurrent = 0x435c9920, mListLink = 0x435c9764}, 
    aKeepReflowGoing=0xbfff7888, aDamageDirtyArea=0)
    at /builds/trunk/mozilla/layout/html/base/src/nsBlockFrame.cpp:2614
#55 0x40c57804 in nsBlockFrame::ReflowDirtyLines (this=0x435c9728, 
    aState=@0xbfff7aa0)
    at /builds/trunk/mozilla/layout/html/base/src/nsBlockFrame.cpp:2253
#56 0x40c545ea in nsBlockFrame::Reflow (this=0x435c9728, 
    aPresContext=0x4307b6f0, aMetrics=@0xbfff8168, aReflowState=@0xbfff7eb0, 
    aStatus=@0xbfff8024)
    at /builds/trunk/mozilla/layout/html/base/src/nsBlockFrame.cpp:844

... (through tons of block, table, gfxscroll, etc. reflow)

#137 0x40ce9023 in ViewportFrame::Reflow (this=0x43646424, 
    aPresContext=0x4307b6f0, aDesiredSize=@0xbfffe750, 
    aReflowState=@0xbfffe580, aStatus=@0xbfffe648)
    at /builds/trunk/mozilla/layout/html/base/src/nsViewportFrame.cpp:587
#138 0x40c89806 in nsHTMLReflowCommand::Dispatch (this=0x435a9758, 
    aPresContext=0x4307b6f0, aDesiredSize=@0xbfffe750, aMaxSize=@0xbfffe730, 
    aRendContext=@0x435ce058)
    at /builds/trunk/mozilla/layout/html/base/src/nsHTMLReflowCommand.cpp:216
#139 0x40cc9814 in PresShell::ProcessReflowCommand (this=0x4357ff60, 
    aQueue=@0x4357ffb0, aAccumulateTime=1, aDesiredSize=@0xbfffe750, 
    aMaxSize=@0xbfffe730, aRenderingContext=@0x435ce058)
    at /builds/trunk/mozilla/layout/html/base/src/nsPresShell.cpp:6287
#140 0x40cc9a98 in PresShell::ProcessReflowCommands (this=0x4357ff60, 
    aInterruptible=1)
    at /builds/trunk/mozilla/layout/html/base/src/nsPresShell.cpp:6342
#141 0x40e99e80 in ReflowEvent::HandleEvent (this=0x435aa158)
    at /builds/trunk/mozilla/layout/html/base/src/nsPresShell.cpp:6196
#142 0x40cc9556 in HandlePLEvent (aEvent=0x435aa158)
    at /builds/trunk/mozilla/layout/html/base/src/nsPresShell.cpp:6212
#143 0x402222e0 in PL_HandleEvent (self=0x435aa158)
    at /builds/trunk/mozilla/xpcom/threads/plevent.c:596
#144 0x40222a51 in PL_ProcessEventsBeforeID (aSelf=0x811a1b8, aID=7389)
    at /builds/trunk/mozilla/xpcom/threads/plevent.c:1270

... (main event loop, main, etc.)

Reopening because the fact that this doesn't crash with your null check is
merely at the mercy of what the allocator decides to recycle, and thus this bug
could come back any time, or could easily still be present on other machines. 
With my patches in bug 114219 and bug 114221 to fill pres-shell-freed memory
with 0xdd, we crash quite hard in other places.  (This is quite easy to
reproduce with those patches in my tree, by loading the URL
http://www.realgm.com/src_roster.php?team=Philadelphia and clicking on one of
the players.)
The real problem here is in the plugins code.  It should be fixed by calling
InstantiatePlugin sometime other than during Reflow, unless we want to state in
the plugin API that content modification during plugin instantiation is forbidden.

I think this should be fixed for mozilla 1.0.
Status: NEW → ASSIGNED
Component: Layout → Plug-ins
Summary: Solve line child count inconsistency that caused crash reported in bug 107545 → plugins can cause frame tree that is being reflowed to be destroyed
Target Milestone: --- → mozilla1.0

Comment 2

17 years ago
We also do bad on a reframe, destroying and recreating the plugin, see bug 90268. :(

Would it be possible to throw |aPluginHost->InstantiateEmbededPlugin| off a
PLEvent? Would |CantRenderReplacedElement| still work?
Bug 90268 is a bit more complicated, since it requires associating some of the
plugin stuff with the content node rather than the frame (at least that's the
logical way to fix this).

But I agree that the "obvious" solution (having barely looked at the code at
all) here is to post a PLEvent to trigger the InstantiatePlugin.  It would work
a lot like the CantRenderReplacedElement events, since there'd have to be
similar removal, and perhaps also duplication checks that don't need to be there.

What exactly is your question about CantRenderReplacedElement?  I don't think
there should be problems calling that from a time other than reflow, since all
it does is post a PLEvent to handle the frame tree munging later, from the event
loop.

Comment 4

17 years ago
Yeah, then this is probably a good idea. Putting plugin unloading off a PLEvent
also helped some crashed.

Is there a testcase?
This URL WFM: http://www.realgm.com/src_roster.php?team=Philadelphia 

CantRenderReplacedElement won't be a problem, but now I'm worred about any
special stuff that happens at DidReflow time.
Posted patch work in progress (obsolete) — Splinter Review
This patch seems to work, mostly, although plugins sometimes appear at the
wrong position.  I think that's somehow related to the DidReflow stuff (which I
moved into PositionPlugin so it could be called from two places).  I suspected
it was because I needed to call SyncFrameViewAfterReflow, but that didn't help.
 I'll debug more tomorrow or the weekend.

Comment 6

17 years ago
Nice patch! I think we may have to protect GetURL in the same way.

Updated

17 years ago
Whiteboard: [PL2:P2]

Comment 7

17 years ago
*** Bug 165462 has been marked as a duplicate of this bug. ***

Comment 8

17 years ago
The following stack eventually causes the test case to crash. In the middle of
nsBlockFrame::Reflow, frames are getting destroyed/created.

NS_NewLineBox(nsIPresShell * 0x015b2cb8, nsIFrame * 0x01607f90, int 1, int 0)
line 91
nsBlockFrame::AddFrames(nsIPresContext * 0x015a6da0, nsIFrame * 0x01607f90,
nsIFrame * 0x00000000) line 4975 + 24 bytes
nsBlockFrame::SetInitialChildList(nsBlockFrame * const 0x01607ccc,
nsIPresContext * 0x015a6da0, nsIAtom * 0x00000000 {???}, nsIFrame * 0x01607f90)
line 6404 + 18 bytes
nsCSSFrameConstructor::ConstructBlock(nsIPresShell * 0x015b2cb8, nsIPresContext
* 0x015a6da0, nsFrameConstructorState & {...}, const nsStyleDisplay *
0x015b9ad0, nsIContent * 0x015b9770, nsIFrame * 0x01607aec, nsIStyleContext *
0x01607c58, nsIFrame * 0x01607ccc) line 13388
nsCSSFrameConstructor::ConstructFrameByDisplayType(nsIPresShell * 0x015b2cb8,
nsIPresContext * 0x015a6da0, nsFrameConstructorState & {...}, const
nsStyleDisplay * 0x015b9ad0, nsIContent * 0x015b9770, int 3, nsIAtom *
0x011ad7f8 {"body"}, nsIFrame * 0x01607aec, nsIStyleContext * 0x01607c58,
nsFrameItems & {...}) line 6519 + 43 bytes
nsCSSFrameConstructor::ConstructFrameInternal(nsIPresShell * 0x015b2cb8,
nsIPresContext * 0x015a6da0, nsFrameConstructorState & {...}, nsIContent *
0x015b9770, nsIFrame * 0x01607aec, nsIAtom * 0x011ad7f8 {"body"}, int 3,
nsIStyleContext * 0x01607c58, nsFrameItems & {...}, int 0) line 7407 + 53 bytes

nsCSSFrameConstructor::ConstructFrame(nsIPresShell * 0x015b2cb8, nsIPresContext
* 0x015a6da0, nsFrameConstructorState & {...}, nsIContent * 0x015b9770,
nsIFrame * 0x01607aec, nsFrameItems & {...}) line 7258 + 56 bytes
nsCSSFrameConstructor::ContentInserted(nsCSSFrameConstructor * const
0x015a89c8, nsIPresContext * 0x015a6da0, nsIContent * 0x01598d30, nsIContent *
0x015b9770, int 2, nsILayoutHistoryState * 0x00000000, int 1) line 9156
nsCSSFrameConstructor::ContentReplaced(nsCSSFrameConstructor * const
0x015a89c8, nsIPresContext * 0x015a6da0, nsIContent * 0x01598d30, nsIContent *
0x015b9770, nsIContent * 0x015b9770, int 2) line 9299 + 32 bytes
nsCSSFrameConstructor::WipeContainingBlock(nsIPresContext * 0x015a6da0,
nsFrameConstructorState & {...}, nsIContent * 0x015b9770, nsIFrame *
0x016080dc, nsIFrame * 0x016082ec) line 13897
nsCSSFrameConstructor::ContentAppended(nsCSSFrameConstructor * const
0x015a89c8, nsIPresContext * 0x015a6da0, nsIContent * 0x01621a00, int 1) line
8550 + 42 bytes
StyleSetImpl::ContentAppended(StyleSetImpl * const 0x015a85f8, nsIPresContext *
0x015a6da0, nsIContent * 0x01621a00, int 1) line 1527
PresShell::ContentAppended(PresShell * const 0x015b2cc0, nsIDocument *
0x015218e8, nsIContent * 0x01621a00, int 1) line 5222 + 49 bytes
nsDocument::ContentAppended(nsDocument * const 0x015218e8, nsIContent *
0x01621a00, int 1) line 2124
nsHTMLDocument::ContentAppended(nsHTMLDocument * const 0x015218e8, nsIContent *
0x01621a00, int 1) line 1403 + 17 bytes
HTMLContentSink::NotifyAppend(nsIContent * 0x01621a00, int 1) line 4651
SinkContext::FlushTags(int 1) line 1948
HTMLContentSink::FlushPendingNotifications(HTMLContentSink * const 0x0159dc90)
line 5112 + 16 bytes
nsHTMLDocument::FlushPendingNotifications(nsHTMLDocument * const 0x015218e8,
int 0, int 0) line 1556 + 23 bytes
nsContentList::BringSelfUpToDate(int 1) line 1030
nsContentList::GetLength(nsContentList * const 0x0162c22c, unsigned int *
0x0012c84c, int 1) line 503
nsContentList::GetLength(nsContentList * const 0x0162c1e8, unsigned int *
0x0012c84c) line 588
nsPluginInstanceOwner::EnsureCachedAttrParamArrays() line 2963
nsPluginInstanceOwner::GetAttributes(nsPluginInstanceOwner * const 0x0151d1cc,
unsigned short & 0, const char * const * & 0x00000000, const char * const * &
0x00000000) line 2218 + 11 bytes
nsPluginInstancePeerImpl::GetAttributes(nsPluginInstancePeerImpl * const
0x0162bfd0, unsigned short & 0, const char * const * & 0x00000000, const char *
const * & 0x00000000) line 291 + 24 bytes
ns4xPluginInstance::InitializePlugin(nsIPluginInstancePeer * 0x0162bfc8) line
744 + 44 bytes
ns4xPluginInstance::Initialize(ns4xPluginInstance * const 0x0162be70,
nsIPluginInstancePeer * 0x0162bfc8) line 635
nsPluginHostImpl::TrySetUpPluginInstance(nsPluginHostImpl * const 0x0117c658,
const char * 0x015215b8, nsIURI * 0x0162b3b8, nsIPluginInstanceOwner *
0x0151d1c8) line 4000 + 21 bytes
nsPluginHostImpl::SetUpPluginInstance(nsPluginHostImpl * const 0x0117c65c,
const char * 0x015215b8, nsIURI * 0x0162b3b8, nsIPluginInstanceOwner *
0x0151d1c8) line 3802 + 28 bytes
nsPluginHostImpl::InstantiateEmbededPlugin(nsPluginHostImpl * const 0x0117c65c,
const char * 0x015215b8, nsIURI * 0x0162b3b8, nsIPluginInstanceOwner *
0x0151d1c8) line 3483 + 24 bytes
nsObjectFrame::InstantiatePlugin(nsIPresContext * 0x015a6da0,
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, nsIPluginHost *
0x0117c65c, const char * 0x015215b8, nsIURI * 0x0162b3b8) line 1307
nsObjectFrame::Reflow(nsObjectFrame * const 0x01608028, nsIPresContext *
0x015a6da0, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...},
unsigned int & 0) line 1158 + 51 bytes
nsLineLayout::ReflowFrame(nsIFrame * 0x01608028, unsigned int & 0,
nsHTMLReflowMetrics * 0x00000000, int & 0) line 1051 + 43 bytes
nsBlockFrame::ReflowInlineFrame(nsBlockReflowState & {...}, nsLineLayout &
{...}, nsLineList_iterator {...}, nsIFrame * 0x01608028, unsigned char *
0x0012d81b) line 3854 + 22 bytes
nsBlockFrame::DoReflowInlineFrames(nsBlockReflowState & {...}, nsLineLayout &
{...}, nsLineList_iterator {...}, int * 0x0012df60, unsigned char * 0x0012dd23,
int 0, int 1) line 3721 + 32 bytes
nsBlockFrame::DoReflowInlineFramesAuto(nsBlockReflowState & {...},
nsLineList_iterator {...}, int * 0x0012df60, unsigned char * 0x0012dd23, int 0,
int 1) line 3625 + 46 bytes
nsBlockFrame::ReflowInlineFrames(nsBlockReflowState & {...},
nsLineList_iterator {...}, int * 0x0012df60, int 1, int 0) line 3569 + 36 bytes

nsBlockFrame::ReflowLine(nsBlockReflowState & {...}, nsLineList_iterator {...},
int * 0x0012df60, int 1) line 2645 + 33 bytes
nsBlockFrame::ReflowDirtyLines(nsBlockReflowState & {...}) line 2289 + 31 bytes

nsBlockFrame::Reflow(nsBlockFrame * const 0x01607ccc, nsIPresContext *
0x015a6da0, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...},
unsigned int & 0) line 963 + 15 bytes
nsBlockReflowContext::ReflowBlock(const nsRect & {x=0 y=0 width=9180
height=1073741824}, int 1, nsCollapsingMargin & {...}, int 1, nsMargin & {top=0
right=0 bottom=0 left=0}, nsHTMLReflowState & {...}, unsigned int & 0) line 536
+ 42 bytes
nsBlockFrame::ReflowBlockFrame(nsBlockReflowState & {...}, nsLineList_iterator
{...}, int * 0x0012eb84) line 3328 + 59 bytes
nsBlockFrame::ReflowLine(nsBlockReflowState & {...}, nsLineList_iterator {...},
int * 0x0012eb84, int 1) line 2507 + 27 bytes
nsBlockFrame::ReflowDirtyLines(nsBlockReflowState & {...}) line 2289 + 31 bytes

nsBlockFrame::Reflow(nsBlockFrame * const 0x01607aec, nsIPresContext *
0x015a6da0, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...},
unsigned int & 0) line 963 + 15 bytes
nsContainerFrame::ReflowChild(nsIFrame * 0x01607aec, nsIPresContext *
0x015a6da0, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, int
0, int 0, unsigned int 0, unsigned int & 0) line 790 + 31 bytes
CanvasFrame::Reflow(CanvasFrame * const 0x015b9940, nsIPresContext *
0x015a6da0, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...},
unsigned int & 0) line 567
nsBoxToBlockAdaptor::Reflow(nsBoxLayoutState & {...}, nsIPresContext *
0x015a6da0, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...},
unsigned int & 0, int 0, int 0, int 9180, int 4470, int 1) line 882
nsBoxToBlockAdaptor::DoLayout(nsBoxToBlockAdaptor * const 0x01607a50,
nsBoxLayoutState & {...}) line 625 + 46 bytes
nsBox::Layout(nsBox * const 0x01607a50, nsBoxLayoutState & {...}) line 1062
nsScrollBoxFrame::DoLayout(nsScrollBoxFrame * const 0x015b9de4,
nsBoxLayoutState & {...}) line 395
nsBox::Layout(nsBox * const 0x015b9de4, nsBoxLayoutState & {...}) line 1062
nsContainerBox::LayoutChildAt(nsBoxLayoutState & {...}, nsIBox * 0x015b9de4,
const nsRect & {x=0 y=0 width=9180 height=4470}) line 645 + 16 bytes
nsGfxScrollFrameInner::LayoutBox(nsBoxLayoutState & {...}, nsIBox * 0x015b9de4,
const nsRect & {x=0 y=0 width=9180 height=4470}) line 1107 + 17 bytes
nsGfxScrollFrameInner::Layout(nsBoxLayoutState & {...}) line 1262
nsGfxScrollFrame::DoLayout(nsGfxScrollFrame * const 0x015b9bec,
nsBoxLayoutState & {...}) line 1115 + 15 bytes
nsBox::Layout(nsBox * const 0x015b9bec, nsBoxLayoutState & {...}) line 1062
nsBoxFrame::Reflow(nsBoxFrame * const 0x015b9bb4, nsIPresContext * 0x015a6da0,
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0)
line 1003
nsGfxScrollFrame::Reflow(nsGfxScrollFrame * const 0x015b9bb4, nsIPresContext *
0x015a6da0, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...},
unsigned int & 0) line 801 + 25 bytes
nsContainerFrame::ReflowChild(nsIFrame * 0x015b9bb4, nsIPresContext *
0x015a6da0, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, int
0, int 0, unsigned int 0, unsigned int & 0) line 790 + 31 bytes
ViewportFrame::Reflow(ViewportFrame * const 0x015b9904, nsIPresContext *
0x015a6da0, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...},
unsigned int & 0) line 577
IncrementalReflow::Dispatch(nsIPresContext * 0x015a6da0, nsHTMLReflowMetrics &
{...}, const nsSize & {width=9180 height=4470}, nsIRenderingContext & {...})
line 893
PresShell::ProcessReflowCommands(int 1) line 6374
ReflowEvent::HandleEvent() line 6219
HandlePLEvent(ReflowEvent * 0x016213c8) line 6233
PL_HandleEvent(PLEvent * 0x016213c8) line 643 + 10 bytes
PL_ProcessPendingEvents(PLEventQueue * 0x010d7818) line 573 + 9 bytes

Updated

17 years ago
Keywords: crash, testcase

Updated

17 years ago
Summary: plugins can cause frame tree that is being reflowed to be destroyed → plugins cause frame tree that is being reflowed to be destroyed

Comment 9

17 years ago
OS=>All from bug 165462
OS: Windows 2000 → All
If someone else wants to take over this bug, feel free.  I might be able to
produce a slightly less bitrotted version of my current patch, although probably
large segments of it would have to be rewritten since they were done by moving
code (that has changed substantially since) around.  I'm probably not going to
get to it.

Comment 11

17 years ago
someone from the plugins group should really own this....I'll take it --->peterl
Assignee: dbaron → peterl
Status: ASSIGNED → NEW
This was a slightly newer version of that work in progress (from a diff I had
lying around dated May 23, although probably against a rather older tree,
perhaps May 10, from the look of the diff).
Attachment #78833 - Attachment is obsolete: true

Comment 13

17 years ago
Posted patch update work in progress (obsolete) — Splinter Review
I've taken the last patch and added a few things like ensuring the first
windows message to the plugin goes through our exception handling. I found out
that's needed there because this patch crashes during Shockave registration. :(
I'll continue to try to find what's really causing the crash.
Attachment #97891 - Attachment is obsolete: true

Updated

17 years ago
Summary: plugins cause frame tree that is being reflowed to be destroyed → during plugin instantiation, frame tree that is being reflowed can be destroyed

Comment 14

17 years ago
Posted patch patch v.4 (obsolete) — Splinter Review
Here's a mega patch that fixes this bug and doesn't crash on Shockwave
registration. I also took on the task of moving nsPluginInstanceOwner into its
own file and into the plugins module. The full-page implemenation should
disappear with bug 90256.

However, I found a new problem with this approach: the onLoad handler for the
page fires before the plugin gets started which causes Netscape Radio (and
others) to fail. I think plugins need a dummy load group request for another
bug anyway, so I'll investigate that.
Attachment #101111 - Attachment is obsolete: true

Comment 15

17 years ago
Posted patch patch v.5 (obsolete) — Splinter Review
This patch takes care of the onLoad handler for the page and makes C|Net Radio
work but now I'm back with a Shockwave crash on NPP_Destroy during
registration. I'll try to find the root cause.
Attachment #101539 - Attachment is obsolete: true

Comment 16

17 years ago
on unix we include X.h which defines KeyPress to avoid such conflict I'm
proposing the  next:
--- nsPluginInstanceOwner_PL.h	Wed Oct  9 17:45:56 2002
+++ nsPluginInstanceOwner.h	Wed Oct  9 17:42:28 2002
@@ -52,6 +52,10 @@
 #include "nsGUIEvent.h"
 #include "nsIPluginInstanceOwner.h"
 #include "nsIPluginTagInfo2.h"
+#if defined(XP_UNIX) && !defined(NO_X11) && defined(KeyPress) // X.h defines
KeyPress, fix this conflict for now 
+#define _KeyPress_OLD_ KeyPress
+#undef KeyPress
+#endif
 #include "nsIDOMMouseListener.h"
---
and one more, in 
layout/html/base/src/nsPresShell.cpp
+NS_IMPL_THREADSAFE_ISUPPORTS1(nsDummyLayoutRequest, nsIRequest, nsIChannel);
has 3 params, so it should be
NS_IMPL_THREADSAFE_ISUPPORTS2(...

Comment 17

17 years ago
Posted patch patch v.6 (obsolete) — Splinter Review
new patch: fixes all previously known issues. Please review.
Attachment #102009 - Attachment is obsolete: true

Comment 18

17 years ago
Comment on attachment 102475 [details] [diff] [review]
patch v.6

Peter, this patch is incomplete, it has only 2 *.xml files

Comment 19

17 years ago
Posted patch patch v.7 (obsolete) — Splinter Review
oops, sorry. Here it is.
Attachment #102475 - Attachment is obsolete: true

Comment 20

17 years ago
Comment on attachment 102538 [details] [diff] [review]
patch v.7

Very nice clean up, Peter! 

I have a couple of questions which we can discuss but may be those should not
affect the review process

1. You publish nsPluginInstanceOwner.h. The interface is not frozen, are there
any reasons you don't just add a nesessary method(s) to it?

2. If we have quite a few local methods in |nsPluginInstanceOwner| which are
not interface methods and those are not going to be reused in full-page mode
then it looks like it would also make sense to have two implementations:
|nsPluginInstanceOwnerEmbed| and |nsPluginInstanceOwnerFullpage
. I understand that the plans are to get rid of the full-page concept all
together but still we can consider to change the class name.

Nits:

o there is #if 0 code in nsObjectFrame.cpp, is it needed?

o nsObjectFrame::GetFullURL -- why do you assign it this way, why not to do
*aFullURL = mFullURL; ?

o nsIPluginInstanceOwner.idl -- is there any particular reason to use native
|class nsObjectFrame| rather than |interface nsIObject| and native
|GetObjectFrame| method?

o nsPIPluginHost -- the same question about added method

o in nsPluginInstanceOwner::nsPluginInstanceOwner -- initialize mOwner with
null

o in nsPluginInstanceOwner.cpp -- some getters don't check for null argument,
particularly |nsPluginInstanceOwner::GetObjectFrame| and
|nsPluginInstanceOwner::GetMode|
Attachment #102538 - Flags: needs-work+

Updated

17 years ago
Blocks: 109082

Comment 21

17 years ago
Posted patch patch v.8 (obsolete) — Splinter Review
new patch addressing Andrei's comments
Attachment #102538 - Attachment is obsolete: true

Comment 22

17 years ago
Posted patch patch v.8.1 (obsolete) — Splinter Review
oops, forgot one nit
Attachment #103580 - Attachment is obsolete: true

Comment 23

17 years ago
Comment on attachment 103584 [details] [diff] [review]
patch v.8.1

r=av
Attachment #103584 - Flags: review+

Comment 24

17 years ago
Posted patch patch v.8.2 (obsolete) — Splinter Review
one more nit fixed to make the Mac happy

Updated

17 years ago
Attachment #103584 - Attachment is obsolete: true

Comment 25

17 years ago
Comment on attachment 103635 [details] [diff] [review]
patch v.8.2

I only got up to nsObjectFrame.h today. I'll probably pick up reviewing where I
left off on Monday. In the meantime, here are the notes I have so far:


==== Why not move the "release" into the |if| block?

-  if (nsnull != mInstanceOwner) {
+  if (mInstanceOwner) {
     mInstanceOwner->CancelTimer();
     mInstanceOwner->Destroy();
   }

+  mInstanceOwner = nsnull;  // release
+


==== Does |mPluginHost| need to be released in the destructor too? I didn't see
any place where it gets released. What about |mPageLoadHolder| should we also
make sure it gets released here too, just in case?


==== Shouldn't this be an |tag.get() == nsHTMLAtoms::object|?


+  if (tag.get() != nsHTMLAtoms::object) {


==== If |mPluginHost| is ever null, the assert will fire, do we want to prevent
a crash in that case by checking for null before calling stop?


+      NS_ASSERTION(mPluginHost, "instance without host, how can this
happen?");
+      mPluginHost->StopPluginInstance(inst);


==== In |nsObjectFrame::CreateWidget()| why are we returning |NS_OK| rather
than |result| when the init'ing of the view fails? It's not an error if didn't
really create the widget?


-      if (NS_OK != result) {
-	 result = NS_OK;       //XXX why OK? MMP
-	 goto exit;	       //XXX sue me. MMP
-      }
+      if (NS_FAILED(result))
+	 return NS_OK;



==== I think this should be calling |mParent->ReflowDirtyChild(shell, this)|
since the parent's version of |ReflowDirtyChild()| could be different:


+  // the last thing we want to do is post a reflow to adjust our position
+  // because starting our plugin may have caused changes to layout and
+  // reflow will clear out any holds the pres shell has
+  nsCOMPtr<nsIPresShell> shell;
+  aPresContext->GetShell(getter_AddRefs(shell));
+  ReflowDirtyChild(shell, this);


==== Why the name change? I don't see any references to images or iframes in
that method?


-    return HandleChild(aPresContext, aMetrics, aReflowState, aStatus, child);
+    return ReflowImageOrIFrame(aPresContext, aMetrics, aReflowState, aStatus,
child);


==== Need to adjust indentation of |aPresContext| so it lines up with the rest
of the args:


+ nsresult nsObjectFrame::TryMakingNewPlugin(nsIPresContext *aPresContext, 
+					     nsHTMLReflowMetrics&     aMetrics,
+					     const nsHTMLReflowState&
aReflowState)


==== The old code would return an error in some instances. The new code will
probably return NS_OK, is that intentional? 


+  if (!mInstanceOwner || 
+	NS_FAILED(mInstanceOwner->GetWindow(win)))
+    return rv;
+
   nsCOMPtr<nsIPluginInstance> pi; 
-  if (!mInstanceOwner ||
-      NS_FAILED(rv = mInstanceOwner->GetWindow(win)) || 
-      NS_FAILED(rv = mInstanceOwner->GetInstance(*getter_AddRefs(pi))) ||
-      !pi ||
-      !win)
+  mInstanceOwner->GetInstance(*getter_AddRefs(pi));
+  

+  if (!pi || !win)
     return rv;


==== Accidental indent of the comment?


-// Screen painting code
+  // Screen painting code
 #if defined (XP_MAC)

Comment 26

17 years ago
==== Disregard my comment above about releasing |mPluginHost| and
|mPageLoadHolder| in the destructor, I just noticed that they along with
|mInstanceOwner| are comptrs so they will be release automatically. So there
shouldn't be need to manually null out |mInstanceOwner| either right?

Comment 27

17 years ago
I found another problem with using PLEvents:
In this testcase, document.write'ing the plugin and then trying to access it
from the DOM fails with the last patch. Since those scripts must run before
onLoad and even before the rest of the page is layed out, it'll have to happen
synchronously, just not in reflow.
Attachment #103635 - Attachment is obsolete: true

Comment 28

17 years ago
We can't use PLEvents because of syncronization issues and since we can't start
the plugin during reflow, this bug will be fixed on the plugin branch when bug
90268 is fixed.
Depends on: 90268
Whiteboard: [PL2:P2] → [PL:branch]
Target Milestone: mozilla1.2alpha → mozilla1.4beta

Comment 29

16 years ago
By the definitions on <http://bugzilla.mozilla.org/bug_status.html#severity> and
<http://bugzilla.mozilla.org/enter_bug.cgi?format=guided>, crashing and dataloss
bugs are of critical or possibly higher severity.  Only changing open bugs to
minimize unnecessary spam.  Keywords to trigger this would be crash, topcrash,
topcrash+, zt4newcrash, dataloss.
Severity: normal → critical

Comment 30

16 years ago
*** Bug 187016 has been marked as a duplicate of this bug. ***

Comment 32

16 years ago
PeterL, Kevin - The duplicate bug (bug 187016) was marked P1. 
Is this still P1 now that football season is over? (See testcase in comment #31
above.)
Summary: during plugin instantiation, frame tree that is being reflowed can be destroyed → during plugin instantiation, frame tree that is being reflowed can be destroyed [@ nsLineLayout::ReflowFrame]

Comment 33

16 years ago
Making this topcrash+ since we have a reproducible testcase.

Also, I noticed this stack signature is showing up a lot in Mozilla 1.3 Alpha
Talkback data...but all the comments mention printing something, so I doubt
those crashes are related to this bug.  But if anyone wants to take a look, I
just logged bug 191339 for the printing crash.

Keywords: topcrash+
Summary: during plugin instantiation, frame tree that is being reflowed can be destroyed [@ nsLineLayout::ReflowFrame] → during plugin instantiation, frame tree that is being reflowed can be destroyed [@ nsLineLayout::ReflowFrame]

Comment 34

16 years ago
Making topcrash-.  I don't see any crashes like this in current Talkback data. 
If no one is able to reproduce this with the testcase, we probably should mark
this worksforme.

Peter:  Did your patch ever get checked in?
Keywords: topcrash+ → topcrash-

Comment 35

16 years ago
attachment 97875 [details] still crashes but it looks like a different stack now which
could explain why it's not showing up in talkback

Comment 36

16 years ago
Returning to topcrash+.  The testcase crashes everytime.  Here is one of my
incidents:

Incident ID 18534635
Stack Signature 	nsBlockFrame::PullFrameFrom 4746a16f
Email Address 	jpatel@netscape.com
Product ID 	MozillaTrunk
Build ID 	2003032504
Trigger Time 	2003-03-27 13:56:28
Platform 	Win32
Operating System 	Windows NT 5.1 build 2600
Module 	gklayout.dll
URL visited 	http://bugzilla.mozilla.org/attachment.cgi?id=97875&action=view
User Comments 	Opening testcase for bug 136927
Trigger Reason 	Access violation
Source File Name 	c:/builds/seamonkey/mozilla/layout/html/base/src/nsBlockFrame.cpp
Trigger Line No. 	2811
Stack Trace 	
nsBlockFrame::PullFrameFrom
[c:/builds/seamonkey/mozilla/layout/html/base/src/nsBlockFrame.cpp, line 2811]
nsBlockFrame::PullFrameFrom
[c:/builds/seamonkey/mozilla/layout/html/base/src/nsBlockFrame.cpp, line 2795]
nsBlockFrame::DoReflowInlineFrames
[c:/builds/seamonkey/mozilla/layout/html/base/src/nsBlockFrame.cpp, line 3776]
nsBlockFrame::PushTruncatedPlaceholderLine
[c:/builds/seamonkey/mozilla/layout/html/base/src/nsBlockFrame.cpp, line 3648]
nsBlockFrame::DoReflowInlineFramesMalloc
[c:/builds/seamonkey/mozilla/layout/html/base/src/nsBlockFrame.cpp, line 3603]
nsBlockFrame::ReflowLine
[c:/builds/seamonkey/mozilla/layout/html/base/src/nsBlockFrame.cpp, line 2706]
nsBlockFrame::ReflowDirtyLines
[c:/builds/seamonkey/mozilla/layout/html/base/src/nsBlockFrame.cpp, line 2344]
nsBlockFrame::Reflow
[c:/builds/seamonkey/mozilla/layout/html/base/src/nsBlockFrame.cpp, line 956]
nsBlockReflowContext::ReflowBlock
[c:/builds/seamonkey/mozilla/layout/html/base/src/nsBlockReflowContext.cpp, line
614]
nsBlockFrame::ReflowBlockFrame
[c:/builds/seamonkey/mozilla/layout/html/base/src/nsBlockFrame.cpp, line 3348]
nsBlockFrame::ReflowLine
[c:/builds/seamonkey/mozilla/layout/html/base/src/nsBlockFrame.cpp, line 2565]
nsBlockFrame::ReflowDirtyLines
[c:/builds/seamonkey/mozilla/layout/html/base/src/nsBlockFrame.cpp, line 2344]
nsBlockFrame::Reflow
[c:/builds/seamonkey/mozilla/layout/html/base/src/nsBlockFrame.cpp, line 956]
nsContainerFrame::ReflowChild
[c:/builds/seamonkey/mozilla/layout/html/base/src/nsContainerFrame.cpp, line 943]
CanvasFrame::Reflow
[c:/builds/seamonkey/mozilla/layout/html/base/src/nsHTMLFrame.cpp, line 589]
nsBoxToBlockAdaptor::Reflow
[c:/builds/seamonkey/mozilla/layout/xul/base/src/nsBoxToBlockAdaptor.cpp, line 927]
nsBoxToBlockAdaptor::DoLayout
[c:/builds/seamonkey/mozilla/layout/xul/base/src/nsBoxToBlockAdaptor.cpp, line 670]
nsBox::GetDirection [c:/builds/seamonkey/mozilla/layout/xul/base/src/nsBox.cpp,
line 1093]
nsScrollBoxFrame::DoLayout
[c:/builds/seamonkey/mozilla/layout/xul/base/src/nsScrollBoxFrame.cpp, line 369]
nsBox::GetDirection [c:/builds/seamonkey/mozilla/layout/xul/base/src/nsBox.cpp,
line 1093]
nsContainerBox::RelayoutChildAtOrdinal
[c:/builds/seamonkey/mozilla/layout/xul/base/src/nsContainerBox.cpp, line 665]
nsGfxScrollFrame::DoLayout
[c:/builds/seamonkey/mozilla/layout/html/base/src/nsGfxScrollFrame.cpp, line 1185]
nsGfxScrollFrameInner::Layout
[c:/builds/seamonkey/mozilla/layout/html/base/src/nsGfxScrollFrame.cpp, line 1350]
nsGfxScrollFrameInner::AdjustReflowStateForPrintPreview
[c:/builds/seamonkey/mozilla/layout/html/base/src/nsGfxScrollFrame.cpp, line 1217]
nsBox::GetDirection [c:/builds/seamonkey/mozilla/layout/xul/base/src/nsBox.cpp,
line 1093]
nsBoxFrame::Reflow
[c:/builds/seamonkey/mozilla/layout/xul/base/src/nsBoxFrame.cpp, line 923]
nsGfxScrollFrame::Reflow
[c:/builds/seamonkey/mozilla/layout/html/base/src/nsGfxScrollFrame.cpp, line 861]
nsContainerFrame::ReflowChild
[c:/builds/seamonkey/mozilla/layout/html/base/src/nsContainerFrame.cpp, line 943]
ViewportFrame::Reflow
[c:/builds/seamonkey/mozilla/layout/html/base/src/nsViewportFrame.cpp, line 277]
IncrementalReflow::Dispatch
[c:/builds/seamonkey/mozilla/layout/html/base/src/nsPresShell.cpp, line 911]
PresShell::ProcessReflowCommands
[c:/builds/seamonkey/mozilla/layout/html/base/src/nsPresShell.cpp, line 6571]
ReflowEvent::HandleEvent
[c:/builds/seamonkey/mozilla/layout/html/base/src/nsPresShell.cpp, line 6416]
PL_DequeueEvent [c:/builds/seamonkey/mozilla/xpcom/threads/plevent.c, line 714]
PL_InitEvent [c:/builds/seamonkey/mozilla/xpcom/threads/plevent.c, line 634]
_md_CreateEventQueue [c:/builds/seamonkey/mozilla/xpcom/threads/plevent.c, line
1458]
USER32.dll + 0x3d91 (0x77d43d91)
USER32.dll + 0x3df7 (0x77d43df7)
nsAppShellService::Run
[c:/builds/seamonkey/mozilla/xpfe/appshell/src/nsAppShellService.cpp, line 480]
main1 [c:/builds/seamonkey/mozilla/xpfe/bootstrap/nsAppRunner.cpp, line 1287]
main [c:/builds/seamonkey/mozilla/xpfe/bootstrap/nsAppRunner.cpp, line 1645]
WinMain [c:/builds/seamonkey/mozilla/xpfe/bootstrap/nsAppRunner.cpp, line 1666]
WinMainCRTStartup()
kernel32.dll + 0x214c7 (0x77e814c7) 

Adding [@ nsBlockFrame::PullFrameFrom] to summary for tracking.  Also updating
with Trunk and M130 since that stack sig is showing up in Talkback data for the
Trunk and Mozilla 1.3.
Keywords: topcrash- → topcrash+
Summary: during plugin instantiation, frame tree that is being reflowed can be destroyed [@ nsLineLayout::ReflowFrame] → during plugin instantiation, frame tree that is being reflowed can be destroyed - Trunk M130 [@ nsBlockFrame::PullFrameFrom] [@ nsLineLayout::ReflowFrame]

Comment 38

16 years ago
TB18696992Z Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.4a) Gecko/20030401
Java Plug-in 1.4.1_02 for Netscape Navigator (DLL Helper)

I couldn´t crash with testcase of bug 200104 but instantly crashed with testcase
from Comment #36 = attachment 97875 [details] = testcase from bug 165462

Updated

16 years ago
Blocks: 203401

Comment 39

16 years ago
TB20447745Z Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.4b) Gecko/20030525
    Java Plug-in 1.4.2 for Netscape Navigator (DLL Helper)

same test as above, comment #38

Updated

16 years ago
Target Milestone: mozilla1.4beta → ---

Comment 40

15 years ago
Mozilla trunk build 2004051108 WinNT4, Java Plugin 1.4.1
I cannot reproduce it with the two testcases attachment 97875 [details] and attachment
105218 [details]. No crash. Is it still an issue? Therefor see the related bug 240105.

Comment 41

15 years ago
The testcase in this bug no longer crashes...so marking WFM to get it off the
radar.  Most recent crashes with this stack trace can be found in bug 240105.
Status: NEW → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → WORKSFORME
The underlying bug reported in comment 0 is still present.  I didn't add all the
talkback notations.
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
Summary: during plugin instantiation, frame tree that is being reflowed can be destroyed - Trunk M130 [@ nsBlockFrame::PullFrameFrom] [@ nsLineLayout::ReflowFrame] → during plugin instantiation, frame tree that is being reflowed can be destroyed
I'm tired of rewriting this patch on whatever branch when I need to figure out
whether a crash is due to this bug.
Attachment #174162 - Flags: superreview?(jst)
Attachment #174162 - Flags: review?(jst)
Comment on attachment 174162 [details] [diff] [review]
assertion patch (checked in 2005-04-14 14:50 -0700)

r+sr=jst
Attachment #174162 - Flags: superreview?(jst)
Attachment #174162 - Flags: superreview+
Attachment #174162 - Flags: review?(jst)
Attachment #174162 - Flags: review+
Comment on attachment 174162 [details] [diff] [review]
assertion patch (checked in 2005-04-14 14:50 -0700)

DEBUG-only code I've been meaning to check in for ages
Attachment #174162 - Flags: approval1.8b2?
Comment on attachment 174162 [details] [diff] [review]
assertion patch (checked in 2005-04-14 14:50 -0700)

a=asa
Attachment #174162 - Flags: approval1.8b2? → approval1.8b2+
Attachment #174162 - Attachment description: assertion patch → assertion patch (checked in 2005-04-14 14:50 -0700)
this ought to be fixed now as best as I can tell now that bug 1156 moved plugin
instantiation outside of reflow, but I'll let someone who understands this bug
mark it...

Comment 48

14 years ago
http://toadstool.se/software/iexploder/demo/iexploder.cgi?test=5668879&random=1 :0.130

seems to trigger the assert and the crash after it

Updated

14 years ago
Blocks: 316894

Updated

14 years ago
Assignee: peterl-bugs → dbaron
Status: REOPENED → NEW
This is an attempt to resurrect the old patch for the 1.8 branch.  So far I've tested a page with flash and a page with Java, and they were fine, but a PDF didn't work (full-page plugin for Acrobat).

A few things of note:
 * a bunch of the calls to GetDesiredSize had an old comment that it needed to be called again because mInstanceOwner affected its result; it did at the time but it doesn't look like it does anymore.  That's good, since there's no way to call it appropriately now.
 * I changed the stuff at the very end of nsObjectFrame::Reflow (CantRenderReplacedElement, which couldn't happen in the Reinstantiate case anyway, and NotifyContentObjectWrapper, which could) so it only happens in HandleInstantiateEvent, i.e., not in the reinstantiate case.  But I also changed HandleInstantiateEvent (the bit moved out of Reflow) so that it doesn't have an early return in a major case, so that it now will call NotifyContentObjectWrapper the first time through.  This doesn't seem too dangerous given that it already calls it in the Reinstantiate case, and it allows it to no longer be called repeatedly.

Updated

13 years ago
Blocks: 312062

Comment 50

13 years ago
Comment on attachment 206646 [details] [diff] [review]
patch against 1.8 branch

I've been testing this patch for bug 312062 in camino: I can still crash with the patch.
*** Bug 346688 has been marked as a duplicate of this bug. ***
WFM will all of the testcases posted here. Can anybody reproduce?

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a2pre) Gecko/20070118 Firefox/3.0a2pre

Comment 54

11 years ago
if I can trust my console output, I am seeing this with the trunk () on windows when I use http://www.apartments.com.

WARNING: NS_ENSURE_TRUE(mSaveLayoutState || !aState) failed: file c:/builds/trun
k/mozilla/docshell/shistory/src/nsSHEntry.cpp, line 349
For application/x-shockwave-flash found plugin C:\WINDOWS\system32\Macromed\Flas
h\NPSWF32.dll
++DOMWINDOW == 62
###!!! ASSERTION: about to crash due to bug 136927: '!mInstantiating', file c:/b
uilds/trunk/mozilla/layout/generic/nsObjectFrame.cpp, line 524
###!!! ASSERTION: Adding child where we already have a child?  This will likely
misbehave: 'Error', file c:/builds/trunk/mozilla/docshell/shistory/src/nsSHEntry
.cpp, line 595
JavaScript error: , line 0: uncaught exception: Permission denied to call method
 Location.toString

stack from the debugger:

 	gklayout.dll!nsObjectFrame::Instantiate(const char * aMimeType=0x0822db78, nsIURI * aURI=0x084e11e0)  Line 1455 + 0x8 bytes	C++
>	gklayout.dll!nsObjectLoadingContent::Instantiate(nsIObjectFrame * aFrame=0x07142ab4, const nsACString_internal & aMIMEType={...}, nsIURI * aURI=0x084e11e0)  Line 1593 + 0x1a bytes	C++
 	gklayout.dll!nsAsyncInstantiateEvent::Run()  Line 146 + 0x22 bytes	C++
 	xpcom_core.dll!nsThread::ProcessNextEvent(int mayWait=1, int * result=0x0012f95c)  Line 511	C++
 	xpcom_core.dll!NS_ProcessNextEvent_P(nsIThread * thread=0x00c05ef0, int mayWait=1)  Line 227 + 0x16 bytes	C++
 	gkwidget.dll!nsBaseAppShell::Run()  Line 154 + 0xc bytes	C++
 	tkitcmps.dll!nsAppStartup::Run()  Line 181 + 0x1c bytes	C++
 	xul.dll!XRE_main(int argc=1, char * * argv=0x00bde898, const nsXREAppData * aAppData=0x00bded10)  Line 3207 + 0x25 bytes	C++
 	firefox.exe!NS_internal_main(int argc=1, char * * argv=0x00bde898)  Line 158 + 0x12 bytes	C++
 	firefox.exe!wmain(int argc=1, unsigned short * * argv=0x00bdd908)  Line 55 + 0xd bytes	C++
 	firefox.exe!__tmainCRTStartup()  Line 583 + 0x19 bytes	C
 	firefox.exe!wmainCRTStartup()  Line 403	C
 	kernel32.dll!7c816fd7() 	
 	[Frames below may be incorrect and/or missing, no symbols loaded for kernel32.dll]	
 	xpcom_core.dll!nsComponentManagerImpl::Init(const nsStaticModuleInfo * aStaticModules=0x00690057, unsigned int aStaticModuleCount=6553710)  Line 669 + 0x8 bytes	C++
 	xpcom_core.dll!nsComponentManagerImpl::Init(const nsStaticModuleInfo * aStaticModules=0x00690057, unsigned int aStaticModuleCount=6553710)  Line 669 + 0x8 bytes	C++
 	xpcom_core.dll!nsComponentManagerImpl::Init(const nsStaticModuleInfo * aStaticModules=0x0078005f, unsigned int aStaticModuleCount=7798829)  Line 669 + 0x8 bytes	C++
 	xpcom_core.dll!nsExceptionService::GetCurrentExceptionManager(nsIExceptionManager * * aCurrentScriptManager=0x0064006e)  Line 246 + 0x7 bytes	C++
 	xpcom_core.dll!nsComponentManagerImpl::Init(const nsStaticModuleInfo * aStaticModules=0x00690057, unsigned int aStaticModuleCount=6553710)  Line 669 + 0x8 bytes	C++
 	xpcom_core.dll!nsComponentManagerImpl::Init(const nsStaticModuleInfo * aStaticModules=0x00690057, unsigned int aStaticModuleCount=6553710)  Line 669 + 0x8 bytes	C++
 	xpcom_core.dll!nsComponentManagerImpl::Init(const nsStaticModuleInfo * aStaticModules=0x00690057, unsigned int aStaticModuleCount=6553710)  Line 669 + 0x8 bytes	C++
 	xpcom_core.dll!nsComponentManagerImpl::Init(const nsStaticModuleInfo * aStaticModules=0x00690057, unsigned int aStaticModuleCount=6553710)  Line 669 + 0x8 bytes	C++
 	xpcom_core.dll!nsComponentManagerImpl::Init(const nsStaticModuleInfo * aStaticModules=0x00690057, unsigned int aStaticModuleCount=6553710)  Line 669 + 0x8 bytes	C++
 	xpcom_core.dll!nsComponentManagerImpl::Init(const nsStaticModuleInfo * aStaticModules=0x00620034, unsigned int aStaticModuleCount=3145827)  Line 669 + 0x8 bytes	C++
 	xpcom_core.dll!nsCString::ReplaceSubstring(const char * aTarget=0x00650000, const char * aNewValue=0x0000006e)  Line 349 + 0x18 bytes	C++

in nsObjectLoadingContent::Instantiate(), aFrame is:


-		aFrame	0x07142ab4	nsIObjectFrame *
-		nsISupports	{...}	nsISupports
+		__vfptr	0xdddddddd	*

I am seeing this with my debug build and the official nightlies.

Comment 56

11 years ago
I'm seeing this crash on Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b3pre) Gecko/2008012204 Minefield/3.0b3pre

Comment 57

11 years ago
this isn't 100% reproducable for me, but I've crashed about 10 times today on www.apartments.com.

Comment 58

11 years ago
seth: please see http://developer.mozilla.org/en/docs/Using_the_Mozilla_symbol_server

if you setup the symbol server (specifically the *microsoft* symbol server), then this:
        kernel32.dll!7c816fd7()         
        [Frames below may be incorrect and/or missing, no symbols loaded for
kernel32.dll]      

should go away. (and as an added bonus, you'll actually have useful stack traces for windows system libraries.)

Comment 59

11 years ago
Some customers reported Firefox 3 crashes with some pages on video.baidu.com.
(baidu is the top 1 search engine in China.)

I modified the page to minimize the testcase.

I can 100% reproduce the crash with Firefox 3 and trunk on Windows.
RealPlayer(tm) G2 LiveConnect-Enabled Plug-In (32-bit) was installed.
The assert leads me to this bug.

If you've vistited a site with RealPlayer plugin, you need to quit Firefox completely, then open Firefox with the testcase.
Gin, what version of RealPlayer are you using? What do you get in about:plugins?
I don't crash with Firefox 3 on your testcase, and I have:
RealPlayer Version Plugin
    File name: nprpjplug.dll
    6.0.12.46

Comment 61

11 years ago
Same here.
RealPlayer Version Plugin
    File name: nprpjplug.dll
    6.0.12.46

I'm on Vista SP1.
Oh yeah, thanks for the info. I do indeed crash when using Vista.
Asking for blocking1.9.1? , but perhaps this should be filed as a separate bug? (since it could perhaps be fixed without fixing this bug?)
Flags: wanted1.9.1?
Flags: blocking1.9.1?
Not going to block this release on this old bug :(
Flags: wanted1.9.1?
Flags: wanted1.9.1+
Flags: wanted-next+
Flags: blocking1.9.1?
Flags: blocking1.9.1-
QA Contact: chrispetersen → plugins

Updated

8 years ago
Blocks: 616748
Blocks: 622310

Comment 64

8 years ago
1. http://www.kvfan.net/kurtlar-vadisi-pusu-73-bolum/ winxp (not 100% reliable)
   or
   http://www.diziizleyelim.com/kurtlar-vadisi-pusu-101-bolum.html

2. ###!!! ASSERTION: about to crash due to bug 136927: '!mPreventInstantiation || (mContent && mContent->GetCurrentDoc()->GetDisplayDocument())', file c:/work/mozilla/builds/2.0.0/mozilla/layout/gener/nsObjectFrame.cpp, line 747
Blocks: 532972

Comment 66

7 years ago
Seems quite likely that bug 90268 fixed this.

Comment 67

7 years ago
Then, let's make it WFM based on comment #65 testing and comment #66 info.
Status: NEW → RESOLVED
Last Resolved: 15 years ago7 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.