330981 - Crash [@ GetNearestContainingBlock] with evil testcase, using table-caption, position: absolute/fixed etc

Status: RESOLVED WORKSFORME

(Core :: Layout: Tables, defect)

Description

[Martijn Wargers (dead)]

See upcoming testcase, which crashes current Mozilla trunk build on load.
It also crashes Mozilla1.7.12, so no recent regression.

Talkback ID: TB16551849Z
GetNearestContainingBlock   nsHTMLReflowState::InitAbsoluteConstraints   nsHTMLReflowState::InitConstraints   nsHTMLReflowState::Init   nsHTMLReflowState::nsHTMLReflowState   nsAbsoluteContainingBlock::ReflowAbsoluteFrame

Comment 1

[Martijn Wargers (dead)]

Attachment: testcase

Comment 2

[Martijn Wargers (dead)]

Ok, this one also doesn't crash always. If no crash, then click on the 'Go' button a few times.

Comment 3

[Martijn Wargers (dead)]

Attachment: Backtrace from debug build

Prior to the crash I get an assertion (stack attached):
###!!! ASSERTION: unexpected child frame type: 'PR_FALSE', file c:/mozilla/mozil
la/layout/tables/nsTableOuterFrame.cpp, line 229

Comment 4

[Hermann Schwab]

Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.9a1) Gecko/20060317 SeaMonkey/1.5a
TB16565280G  (talkback of testcase)

Comment 5

[Bernd]

The logic at nsCSSFrameConstructor::IsValidSibling is completely pseudo agnostic.

What happens here on the restyle event: we enter this function with display unset and compute the display from a completely broke parent style context.

   if (UNSET_DISPLAY == aDisplay) {
      nsRefPtr<nsStyleContext> styleContext;
      styleContext = ResolveStyleContext(aSibling.GetParent(), &aContent);
      if (!styleContext) return PR_FALSE;
      const nsStyleDisplay* display = styleContext->GetStyleDisplay();
      aDisplay = display->mDisplay;
    }
aSibling is the captionFrame the parent is the outer table frame...
and then we walk further, at the end we try to insert the span below the outer table frame on the primary childlist.

IMHO we should crawl up if the sibling is not a valid sibling and the parent does not coincide with the insertion point that we computed above and is a pseudo.

Boris any comments on this?

Comment 6

[Boris Zbarsky [:bzbarsky]]

Which insertion point?  I don't see one in IsValidSibling...

Comment 7

[Bernd]

Attachment: code illustration

Boris, thats what I have in mind (roughly), will that fly or do you see major showstoppers for this?

Comment 8

[Boris Zbarsky [:bzbarsky]]

I don't know.  I'd have to figure out what the function you're changing is doing and why.... I'm not sure why walking up to the parent makes sense, though... why does it?  What happens if none of the frames involved are pseudos?

Comment 9

[Martijn Wargers (dead)]

Attachment: another (not completely minimised) testcase

Another testcase, that triggers this kind of crash, it looks a bit different.

Comment 10

[Martijn Wargers (dead)]

The first testcase doesn't crash anymore in the 2006-09-25 trunk build, but the second testcase still does.

Comment 11

[Bernd]

Attachment: testcase that triggers the first assert

the testcase triggers 
###!!! ASSERTION: DeletingFrameSubtree on a special frame.  Prepare to crash.: '
!IsFrameSpecial(aFrame)', file d:/moz_src/mozilla/layout/base/nsCSSFrameConstruc
tor.cpp, line 9667

Comment 12

[Boris Zbarsky [:bzbarsky]]

So... one of my builds doesn't trigger the assert or crash reported in comment 9 and comment 11.  The local change which seems to help with it is a combination of the patch for bug 336718 and the patch for bug 318592 (or maybe even just the latter; I know the two together help and the former on its own does not).

This same build _does_ crash on the original testcase in this bug if I reload a few times.

Comment 13

[Bernd]

Attachment: first testcase with location.reload

I don't get the crash that Boris described.

Comment 14

[Boris Zbarsky [:bzbarsky]]

OK, I can confirm that updating from MOZ_CO_DATE="Sat Oct 28 00:03:30 CDT 2006" to MOZ_CO_DATE="Fri Nov 3 20:40:49 CST 2006" made the assert on the first testcase go away for me...  Sounds like _something_ got checked in there.  ;)

Comment 15

[Bernd]

this bug is now completely WFM with a debug cvs from today. Please reopen if there is still an issue that I overlooked.

Comment 16

[Hermann Schwab]

(In reply to comment #13)
> Created an attachment (id=244651) [details]
> first testcase with location.reload
> 
> I don't get the crash that Boris described.

TB33570185E TB33570101X Firefox15 Build ID 2007062807

TB33570383G Firefox2 Build ID 2007062403

The Talkbacks don't have symbols, Seamonkey is also crashing. Seems reproducable, crash at third automatic reload.

Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8.1.4pre) Gecko/20070517 SeaMonkey/1.1.2

I'll keep on testing newer branch versions.

Comment 17

[Hermann Schwab]

Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8.1.4) Gecko/20070515 Firefox/2.0.0.4

This is a branch release version, and Talkback records are using symbols.

 first testcase with location.reload   (728 bytes, text/html)
was crashing two times while doing 2nd reload, and last time onload.

TB33573282H, TB33573379W, TB33584959H identical

GetNearestContainingBlock  [mozilla/layout/generic/nsHTMLReflowState.cpp, line 661]
nsHTMLReflowState::InitAbsoluteConstraints  [mozilla/layout/generic/nsHTMLReflowState.cpp, line 1045]
nsHTMLReflowState::InitConstraints  [mozilla/layout/generic/nsHTMLReflowState.cpp, line 1961]
nsHTMLReflowState::Init  [mozilla/layout/generic/nsHTMLReflowState.cpp, line 342]
nsHTMLReflowState::nsHTMLReflowState  [mozilla/layout/generic/nsHTMLReflowState.cpp, line 315]
nsAbsoluteContainingBlock::ReflowAbsoluteFrame  [mozilla/layout/generic/nsAbsoluteContainingBlock.cpp, line 521]
nsAbsoluteContainingBlock::IncrementalReflow  [mozilla/layout/generic/nsAbsoluteContainingBlock.cpp, line 387]
ViewportFrame::Reflow  [mozilla/layout/generic/nsViewportFrame.cpp, line 300]
IncrementalReflow::Dispatch  [mozilla/layout/base/nsPresShell.cpp, line 915]
PresShell::ProcessReflowCommands  [mozilla/layout/base/nsPresShell.cpp, line 6947]
PresShell::WillPaint  [mozilla/layout/base/nsPresShell.cpp, line 6572]
0x778b0c24

Comment 18

[Boris Zbarsky [:bzbarsky]]

Yeah, I bet we did all sorts of work on trunk here.. if someone tracks down the checkin that fixed this, we can look into putting it on branch.

Comment 19

[Hermann Schwab]

(In reply to comment #18)
> Yeah, I bet we did all sorts of work on trunk here.. if someone tracks down the
> checkin that fixed this, we can look into putting it on branch.

I checked testcases of  bug 336718 and bug 318592 mentioned in your comment 12, but couldn't crash using Firefox 2 or Seamonkey.

Would be nice if this could be fixed, as Firefox2 is the last one working on win98. My father and his way younger girlfriend (sweet 80) are both having win98 computers, and he is too old to buy green bananas, or a new computer.

Comment 20

[Boris Zbarsky [:bzbarsky]]

I was thinking more along the lines of narrowing down the comment 14 range...

Comment 21

[Martijn Wargers (dead)]

That seems to have been fixed between 2006-10-23 and 2006-10-29.
Ria, do you have builds inbetween to see when https://bugzilla.mozilla.org/attachment.cgi?id=244651 stopped crashing?

Comment 22

[Bernd]

bug 341858 would fail into this range

Comment 23

[Ria Klaassen (not reading all bugmail)]

It was fixed between 2006-10-27 21 and 2006-10-28 09.

Comment 24

[Boris Zbarsky [:bzbarsky]]

Yeah, I bet bug 341858 is it.