1366142 - stylo: Crash while clicking a line on waaark.com: Assertion failure: aFrame || (aHint & nsChangeHint_ReconstructFrame)

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect, P1)

Description

[Shing Lyu [:shinglyu]]

Attachment: crash.log

STR:
* Open http://waaark.com/
* Click on the "works" link

Expected
* Navigates to http://waaark.com/works/

Actual
* Crashed

Notes:
* If we open http://waaark.com/works/ directly, it will not crash, so probably due to some animation/transition after the link was clicked

Comment 1

[Hiroyuki Ikezoe (:hiro)]

Comment on attachment 8869299 [details]
crash.log

>Assertion failure: aFrame || (aHint & nsChangeHint_ReconstructFrame) (must have frame), at /home/shinglyu/workspace/stylo/mozilla-central-2/layout/base/nsStyleChangeList.cpp:20
>#01: ???[/home/shinglyu/workspace/stylo/mozilla-central-2/stylo-icecc/dist/bin/libxul.so +0x5217e4f]

I saw this assertion before.
And if I remember correctly the target content was SVGPathElement.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Yes, this is when getting reentrant changes to a pre-existing content. In this case to an SVG element.

The hint in this case is nsChangeHint_InvalidateRenderingObservers.

In any case, what's happening is that when invalidating SVG elements, we post change hints from change hint processing, and either assume that the frames are there (which isn't true because some hints are posted from DestroyFrame).

I think Gecko just ignores these hints (in the sense that Gecko will never find the frame while restyling).

We should probably do the same.

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1366142: Ignore reentrant change hints without primary frame.

Review commit: https://reviewboard.mozilla.org/r/141288/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/141288/

Comment 4

[Chris Peterson [:cpeterson]]

Assigning to Emilio because he submitted a patch.

Comment 5

[Cameron McCormack (:heycam)]

(In reply to Emilio Cobos Álvarez [:emilio] from comment #2)
> I think Gecko just ignores these hints (in the sense that Gecko will never
> find the frame while restyling).

By this do you mean that we'll be traversing the frame tree to do restyling, and because the target frame is now gone, we'll never pick out the change hint from the table?  Sounds plausible.

Comment 6

[Cameron McCormack (:heycam)]

Comment on attachment 8869703 [details]
Bug 1366142: Ignore reentrant change hints without primary frame.

https://reviewboard.mozilla.org/r/141288/#review144896

Comment 7

[Emilio Cobos Álvarez (:emilio)]

(In reply to Cameron McCormack (:heycam) from comment #5)
> (In reply to Emilio Cobos Álvarez [:emilio] from comment #2)
> > I think Gecko just ignores these hints (in the sense that Gecko will never
> > find the frame while restyling).
> 
> By this do you mean that we'll be traversing the frame tree to do restyling,
> and because the target frame is now gone, we'll never pick out the change
> hint from the table?  Sounds plausible.

Yes, I meant exactly that, thanks for rewording it :)

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0665f9f01eb2
Ignore reentrant change hints without primary frame. r=heycam

Comment 9

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/0665f9f01eb2