If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

mCBReflowState can be uninitialized in some cases (nsHTMLReflowState::InitCBReflowState never called)

NEW
Unassigned

Status

()

Core
Layout: Misc Code
15 years ago
8 years ago

People

(Reporter: Jochen Suckfuell, Unassigned)

Tracking

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

15 years ago
There is a case where nsHTMLReflowState::InitCBReflowState doesn't set the
mCBReflowState member variable. This leads to a segfault later on. What's more,
even the constructors don't set this member to anything useful (like nsnull).
This would be important because on of the constructors might not call
Init-->InitCBReflowState .
Status: UNCONFIRMED → NEW
Ever confirmed: true
Jochen, what's the case in which nsHTMLReflowState::InitCBReflowState does not
set mCBReflowState?  I just looked at it, and every single early return sets
mCBReflowState before returning; the last statement in the function also sets
mCBReflowState.
(Reporter)

Comment 2

15 years ago
Boris, you're right. The current revision of the file sets the member in the
last line while assuming that the parentReflowState is set.

Yet there's still a case where InitCBReflowState isn't called from the one
constructor that takes a PRBool aInit to decide if Init() should happen. In this
case mCBReflowState should at least be initialized to nsnull.

The way I ran into the segfault was from a module I wrote for html2pdf
conversion. It uses the layout engine, but cannot use WebShell nor
DocumentViewer. It may well be that I initialized some classes differently than
the original mozilla.

Updated

15 years ago
QA Contact: petersen → amar

Updated

15 years ago
Priority: -- → P3

Updated

15 years ago
Target Milestone: --- → Future

Comment 3

15 years ago
Created attachment 94346 [details]
stack trace

Comment 4

15 years ago
we've had 96 crashes on the windows platform yesterday alone. since twister had
problems with symbols last week, the talkback team apologizes for not catching
this sooner. attached is the stack trace gathered from talkback.
Severity: normal → blocker
Keywords: crash, zt4newcrash
Summary: nsHTMLReflowState::InitCBReflowState doesn't initialize mCBReflowState → crash [@ nsHTMLReflowState::InitCBReflowState] doesn't initialize mCBReflowState

Comment 5

15 years ago
from the users who commented, everyone is having launch/startup problems. looks
like it dies before the profile manager even executes (at least visibly).
comment 3, comment 4, and comment 5 are about bug 161359, not this bug.  Please
don't add any more comments about that crash to this bug.
Just to clarify

This bug is about a possible situation in which
nsHTMLReflowState::InitCBReflowState is _never_called_, which leads to a reflow
state with an uninitialized mCBReflowState. The only way that could crash is if
a child reflow state that was not a CBReflowState itself then got initialized
with the "broken" reflow state as a parent.

The crash you are seeing is happening when the reflow state is constructed in  
 PresShell::InitialReflow.  This reflow state has no parents.  If it crashes on
construction, then almost certainly the root frame getting passed in is null or
bogus.

I think the bug you wanted is bug 161359.  Which was due to a bogus build.

Removing all the bogus markings.
Severity: blocker → normal
Keywords: crash, zt4newcrash
Summary: crash [@ nsHTMLReflowState::InitCBReflowState] doesn't initialize mCBReflowState → mCBReflowState can be uninitialized in some cases (nsHTMLReflowState::InitCBReflowState never called)

Comment 8

15 years ago
thx boris for the clarification =) i really appreciate it.
Blocks: 179819
.
Assignee: attinasi → misc
Component: Layout → Layout: Misc Code
Priority: P3 → --
QA Contact: amar → nobody
Target Milestone: Future → ---
I'll look at this once I finish making the mCBReflowState changes in bug 135082
Depends on: 135082
OK, so it looks like the only case we have to worry about is when a reflow state
for a kid is constructed with aInit == PR_FALSE and then caller never calls Init().

In this case, not only mCBReflowState but also a whole slew of other pointer
members (eg all the style struct) members will be null.

At the moment, the "no init" constructor is called by at least nsBlockFrame and
the various table frames.  Tables just want to pass in border-collapsed border
and padding to Init(), which we could roll into the constructor args.  But
nsBlockFrame actually creates and inits the reflow state at rather distinct
points...  The block code has an XXX comment by dbaron that that computation
should just happen in the reflow state itself.  I think if we fixed that we
could just eliminate the no-init version of the constructor altogether
(replacing it with one that takes optional nsMargins for the border and
padding), and make Init() a protected method of nsHTMLReflowState, called from
every constructor.

Does that seem reasonable?
Assignee: layout.misc-code → nobody
QA Contact: nobody → layout.misc-code
You need to log in before you can comment on or make changes to this bug.