1402285 - stylo: Crash in core::option::expect_failed | geckoservo::glue::Servo_Element_IsPrimaryStyleReusedViaRuleNode

Status: VERIFIED FIXED

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

Description

[Calixte Denizet (:calixte)]

This bug was filed from the Socorro interface and is 
report bp-67b49e6c-d31e-45b3-8b7c-c49380170921.
=============================================================

There are 7 crashes from 2 installations in nightly 57.0a1 with buildid 20170921100141. In analyzing the backtrace, the regression may have been introduced by patch [1] to fix bug 1401317.

[1] https://hg.mozilla.org/mozilla-central/rev?node=4d6c6ad5642940e66b7a6714d3fd16633682fe5d

Comment 1

[Bobby Holley (:bholley)]

Attachment: Don't try to cache pseudo styles with an unstyled originating element. v1

MozReview-Commit-ID: C4JNZFtYHBH

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8911411 [details] [diff] [review]
Don't try to cache pseudo styles with an unstyled originating element. v1

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

This is fine, but we should probably dig into it anyway...

Comment 3

[Bobby Holley (:bholley)]

(In reply to Emilio Cobos Álvarez [:emilio] from comment #2)
> Comment on attachment 8911411 [details] [diff] [review]
> Don't try to cache pseudo styles with an unstyled originating element. v1
> 
> Review of attachment 8911411 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> This is fine, but we should probably dig into it anyway...

You mean the question of "why we have an unstyled originating element in nsMathMLFrame::ResolveMathMLCharStyle"? I was sorta hoping to avoid digging into that part. :-)

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6cb6f41aea09
Don't try to cache pseudo styles with an unstyled originating element. r=emilio

Comment 5

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/6cb6f41aea09

Comment 6

[Ryan VanderMeulen [:RyanVM]]

Please request Beta approval on this when you get a chance.

Comment 7

[Bobby Holley (:bholley)]

Comment on attachment 8911411 [details] [diff] [review]
Don't try to cache pseudo styles with an unstyled originating element. v1

Approval Request Comment
[Feature/Bug causing the regression]: bug 1401317
[User impact if declined]: Crashes
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: (Yes, verified now)
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No.
[Why is the change risky/not risky?]: Basically just a null check to skip a caching optimization.
[String changes made/needed]: None

Comment 8

[Bobby Holley (:bholley)]

(In reply to Bobby Holley (:bholley) (busy with Stylo) from comment #7)
> Approval Request Comment
> [Has the fix been verified in Nightly?]: (Yes, verified now)

Yes, no new crashes since 9/22.

Comment 9

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8911411 [details] [diff] [review]
Don't try to cache pseudo styles with an unstyled originating element. v1

Fix a crash, taking it.
Should be in 57b4

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/edb8232cf035