1393637 - stylo: WARN:geckoservo::glue: Trying to get change hint from unstyled element

Status: ASSIGNED

(Core :: CSS Parsing and Computation, enhancement, P4)

Description

[Hiroyuki Ikezoe (:hiro)]

The warning led to crash (bug 1391444), the crash has been already fixed by bug 1392170, but we still need to figure out how this warning happens.

Comment 1

[Hiroyuki Ikezoe (:hiro)]

I could catch the case of reloading https://us.battle.net/login/en/ .  I think now I understand what's going on there.
nsXMLContentSink intermittently calls FlushTags [1] (which leads to nsCSSFrameConstructor::ContentAppended) in DidAddContent(). So, in the case where I got the warning;

1) the FlushTags is called against body
2) NODE_DESCENDANTS_NEED_FRAMES is set to the body element
3) a script element in question is added to DOM tree, but at this moment nsCSSFrameConstructor::ContentAppend() is not yet called for the script element
4) FlushThrottledStyles is called due to mouse event
5) the script element is not traversed since it has no chance to have dirty bits.

So, if we can tell whether ContentAppended (or ContentInserted) has been called for the target element, we can check it in Servo_TakeChangeHint(). But I think it's the dirty bit itself...

[1] https://hg.mozilla.org/mozilla-central/file/4caca1d0ba0e/dom/xml/nsXMLContentSink.h#l140

Comment 2

[Hiroyuki Ikezoe (:hiro)]

So, at least for the case what I saw the warning looks harmless to me. Setting priority P4.

Comment 3

[Hiroyuki Ikezoe (:hiro)]

I haven't checked innerText case yet.

Comment 4

[Hiroyuki Ikezoe (:hiro)]

I checked a crash report which caused by GetInnerText call [1]
The function is called in a callback for setTimeout(). I am convinced the step is exactly same as comment 1, the only difference is this case is Element.innerText is called in a callback for setTimeout() instead of mouse events.

[1] https://crash-stats.mozilla.com/report/index/a4bed079-847e-43d7-8b62-c661b0170823

Comment 5

[Chris Peterson [:cpeterson]]

status-firefox57=wontfix unless someone thinks this bug should block 57