Closed Bug 595740 Opened 9 years ago Closed 9 years ago

Crash [@ nsFloatManager::GetFlowArea ] or [@ nsStyleContext::DoGetStyleDisplay(int) ] when print-previewing paradiso-design.net/videostandards.html

Categories

(Core :: Layout, defect, P2, critical)

x86
All
defect

Tracking

()

RESOLVED FIXED
mozilla2.0b10
Tracking Status
blocking2.0 --- betaN+

People

(Reporter: dholbert, Assigned: dbaron)

References

(Blocks 1 open bug, )

Details

(4 keywords, Whiteboard: [softblocker])

Crash Data

Attachments

(10 files)

STEPS TO REPRODUCE:
 1. Load URL: http://www.paradiso-design.net/videostandards.html
 2. Print-preview before the background-image shows up. (Shift-reload & try again if you're too late.)

ACTUAL RESULTS:
 Crash [@ nsFloatManager::GetFlowArea ]

Crash reports:
 bp-28a6ac57-a8a4-4a8d-8515-3c6a42100912
 bp-8f29f59b-0787-49f5-b8c1-23eb62100912

The Crash Address is "0xf0dea823", which I think (?) is a frame-poisoned value.
This seems like something that could be a regression from bug 563584.
Blocks: 563584
blocking2.0: --- → ?
Assignee: nobody → dbaron
blocking2.0: ? → betaN+
Severity: normal → critical
Crash reports for Windows :
Signature    nsStyleContext::DoGetStyleDisplay(int)
bp-2ac521cb-36e1-4e83-acb9-118322100913
Status: NEW → ASSIGNED
OS: Linux → All
Summary: Crash [@ nsFloatManager::GetFlowArea ] when print-previewing paradiso-design.net/videostandards.html → Crash [@ nsFloatManager::GetFlowArea ] or [@ nsStyleContext::DoGetStyleDisplay(int) ] when print-previewing paradiso-design.net/videostandards.html
Duplicate of this bug: 595776
I am tagging this with the keyword topcrash. This falls inside our top30 criteria on 4.0b6 and it still appearing on the trunk so it hasn't been fixed yet.
Keywords: topcrash
(In reply to comment #4)
> I am tagging this with the keyword topcrash. This falls inside our top30
> criteria on 4.0b6 and it still appearing on the trunk so it hasn't been fixed
> yet.

you are sure this is a topcrash, shows 18 crashes for beta6 ?
I think the volume used to be higher. Chris indicated that any in the top 30 could be considered a topcrash. There are just a handful of crashes now. I am removing the topcrash keyword.
Keywords: topcrash
Before the crash, I hit two occurrences of:

###!!! ASSERTION: Two floats with same parent in same floats list, expect weird errors.: 'c == f || c->GetParent() != this || !mFloats.ContainsFrame(c)', file /home/dbaron/builds/mozilla-central/mozilla/layout/generic/nsBlockFrame.cpp, line 4633
So the reason we have the assertion mentioned in the previous comment is that we push a first-in-flow float to a block that already has that float's next-in-flow.  In other words, the code related to pushing first-in-flows isn't following the invariants about not pushing something to a block that already has a continuation of that float.

I should probably try to confirm that the assertion is related to the crash, though.
(In that case, what we probably need to do is destroy the next-in-flow.)
I'm reasonably confident it's related; we're crashing after reading the poison pattern out of a "frame" at the same address as the next-in-flow that we asserted about.  (We get two assertions because the loop in nsBlockFrame::DrainPushedFloats (unnecessarily) asserts for the pair both ways round (prev-next and then next-prev).)
Duplicate of this bug: 622965
(In reply to comment #11)
> *** Bug 622965 has been marked as a duplicate of this bug. ***

Bug 622965 sees a crash with the same signature when trying to save a page (http://www.hpcwire.com/offthewire/Alteras-Stratix-IV-GT-FPGA-Passes-100GbE-Interoperability-Test-112874204.html) as pdf
Attached file valgrind warnings
I read through code yesterday to see how we handle this sort of case in the non-float case, and as far as I can tell we allow it.  As far as I can tell, we allow it; it's ok to have two continuations within the same parent, and presumably by the end of reflow we'll end up with all but the first of them completely empty.

So then, I suppose, the next question is why we crash when we do the same thing with floats.  Here's what valgrind says.
...and I think the key there may be that the stacks in the first valgrind warning are mostly common:  all but the top 3 frames of the read and the top 7 frames of the free.  But I'm not really sure, since it's not clear which functions have different parameters.
I wonder if we're going through a multi-pass reflow without properly pushing and popping float manager state.
Attached file reflow log
And when I set up reflow logging, there was a little bit of reflowing between the two-floats assertion and the DestroyFrom call, but none between the DestroyFrom call and the crash.
Attachment #501753 - Attachment mime type: application/octet-stream → text/plain
Whiteboard: [softblocker]
http://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/524a87d58df3/handle-multiple-continuations
fixes the crash (although not the assertions) on http://www.paradiso-design.net/videostandards.html .

I still need to:
 * figure out whether I want to remove the assertions or replace them with something else
 * figure out if I can make a simplified testcase
(In reply to comment #18)
> http://www.paradiso-design.net/videostandards.html .

Unfortunately, save-page-as, complete, doesn't even give a reproducable testcase, so I'm stumped as far as getting a simpler testcase goes.
I can use <base href>, though.
Attached file and more simplified
The trick is that the "y" has to end up at the very bottom of the page, such that a letter following the <br> after it would be on the next page.
Whiteboard: [softblocker] → [softblocker][needs review]
Attachment #502379 - Attachment is patch: true
Attachment #502379 - Attachment mime type: application/octet-stream → text/plain
Blocks: 602426
No longer depends on: 602426
Whiteboard: [softblocker][needs review] → [softblocker][needs landing][waiting to land until after b9 freeze]
http://hg.mozilla.org/mozilla-central/rev/5eb94b2c7606
http://hg.mozilla.org/mozilla-central/rev/e7504a9cf0cb
http://hg.mozilla.org/mozilla-central/rev/37d585cbcb75
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Priority: -- → P2
Resolution: --- → FIXED
Whiteboard: [softblocker][needs landing][waiting to land until after b9 freeze] → [softblocker]
Target Milestone: --- → mozilla2.0b10
Depends on: 647792
Crash Signature: [@ nsFloatManager::GetFlowArea ] [@ nsStyleContext::DoGetStyleDisplay(int) ]
You need to log in before you can comment on or make changes to this bug.