Closed Bug 1299348 Opened 5 years ago Closed 5 years ago

stylo: Crash in layout/reftests/bugs/212563-2.html trying to eagerly style scroll frame anonymous content before styling the root element

Categories

(Core :: CSS Parsing and Computation, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla51
Tracking Status
firefox51 --- fixed

People

(Reporter: bholley, Assigned: bholley)

References

Details

Attachments

(1 file)

I think the solution here is to remove StylingStarted(), which is kind of a nebulous concept. I'll post a patch.
StylingStarted is a kind of nebulous and not-very-useful concept. The concept
that _is_ useful is whether the presshell has been initialized or not, but the
root element may not exist at that point.

So we need to make sure we that we can trigger the initial document style in both
presshell initialized _and_ ContentInserted, which has the nice effect of handling
root element reinsertions.

We also take the opportunity to make StyleDocument assert the existence of a root
element, and align the responsibility for clearing the dirty descendant bits between
document and non-document nodes.
Comment on attachment 8786559 [details] [diff] [review]
Remove StylingStarted(). v1

I think emilio can probably review this one, but flagging heycam for an optional async lookover if he wants to.
Attachment #8786559 - Flags: review?(ecoal95)
Attachment #8786559 - Flags: feedback?(cam)
Comment on attachment 8786559 [details] [diff] [review]
Remove StylingStarted(). v1

Review of attachment 8786559 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with that comment if it's clear why (I don't remember exactly why we had StylingStarted in the first place, apart from the fact that it was happening).

Nice cleanup :)

::: layout/base/ServoRestyleManager.cpp
@@ +282,2 @@
>  
> +  if (!PresContext()->PresShell()->DidInitialize()) {

Can you leave a comment here on when can this happen? I know this is mostly the same condition as before, but now it could maybe be better diagnosed?

Also... Maybe it's too early to make it MOZ_UNLIKELY, I guess.
Attachment #8786559 - Flags: review?(ecoal95) → review+
https://hg.mozilla.org/mozilla-central/rev/3f6b84d701ae
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Attachment #8786559 - Flags: feedback?(cam) → feedback+
You need to log in before you can comment on or make changes to this bug.