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

RESOLVED FIXED in Firefox 51

Status

()

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: bholley, Assigned: bholley)

Tracking

(Blocks: 1 bug)

unspecified
mozilla51
Points:
---

Firefox Tracking Flags

(firefox51 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
I think the solution here is to remove StylingStarted(), which is kind of a nebulous concept. I'll post a patch.
(Assignee)

Comment 1

2 years ago
Created attachment 8786559 [details] [diff] [review]
Remove StylingStarted(). v1

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.
(Assignee)

Comment 2

2 years ago
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+

Comment 5

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/3f6b84d701ae
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox51: --- → fixed
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.