This bug was filed from the Socorro interface and is report bp-83cc9fe3-e3d2-4988-b33b-9bebc0170807. ============================================================= There were about 17 crash reports with this signature. The earliest build ID was 20170722112649.

So presumably the element is becoming unstyled somehow. However, Servo_HasAuthorSpecifiedRules basically just operates on the primary style. So we can just grab that directly off the frame (aFrame->StyleContext()->AsServo()), and then pass that to Rust, side-stepping this crash. Should be a trivial patch. Manish, can you write that up? http://searchfox.org/mozilla-central/rev/b52285fffc13f36eca6b47de735d4e4403b3859e/layout/base/nsPresContext.cpp#2282

Removing crashes from crash-stats (i.e. where we don't have somebody filing with STR) from stylo-site-issues, we can track crash-stats separately.

Untested, waiting on a clean build. Do we have any idea how to repro this crash?

I don't have STR, but three of the crash reports' URLs are direct links to (NSFW) video files (.mp4 or .webm URLs). Perhaps Firefox's full-page video player controls can trigger this crash? OTOH, there are also crash reports with non-video URLs like https://tweetdeck.twitter.com and about:blank.

Comment on attachment 8894639 [details] Bug 1387953 - Exit early before calling Servo_HasAuthorSpecifiedRules without element data; https://reviewboard.mozilla.org/r/165796/#review170980 (In reply to Manish Goregaokar [:manishearth] from comment #5) > Untested, waiting on a clean build. Do we have any idea how to repro this > crash? Probably not worth spending too much time on. I just realized that the HASR implementation still can rely on the style state of elements in the DOM, when walking up the parent chain [1], which means the approach here will only work of the unstyled element's parent has styles. So I'm included to just check HasServoData at the callsite, and warn + return |false| if there are no styles, which will cause us to use native styles in those cases (which I think is a reasonably-safe fallback). Sorry for the misdirection. :-( [1] http://searchfox.org/mozilla-central/rev/bd39b6170f04afeefc751a23bb04e18bbd10352b/servo/components/style/rule_tree/mod.rs#1240

And to be clear, the bug here is that the Gtk widget machinery is somehow calling

(In reply to Bobby Holley (:bholley) (busy with Stylo) from comment #9) > And to be clear, the bug here is that the Gtk widget machinery is somehow > calling Er, somehow calling into HasAuthorSpecifiedRules for an unstyled element during reflow. Not entirely sure how that happens, and doing the check I propose might just move the crash elsewhere, but it seems worth a shot.

Comment on attachment 8894639 [details] Bug 1387953 - Exit early before calling Servo_HasAuthorSpecifiedRules without element data; https://reviewboard.mozilla.org/r/165796/#review171496 ::: servo/ports/geckolib/glue.rs:1628 (Diff revision 3) > - let data = > - element.borrow_data() > - .expect("calling Servo_HasAuthorSpecifiedRules on an unstyled element"); > + let data if let Some(data) = element.borrow_data() { > + data > + } else { > + return false; > + }; Nit: This would probably be better to do in the caller with a HasServoData() check, since it's gecko-specific, and thus doesn't really need to be in the servo tree, but either way. Please add a comment referencing the bug and indicating that this probably shouldn't happen, but appeared in some crash reports. Thanks!

I mean, geckolib code is gecko specific :)

(In reply to Manish Goregaokar [:manishearth] from comment #13) > I mean, geckolib code is gecko specific :) Yes, but there is some code that needs to be in Rust. In general given the choice, if there's something gecko-specific we can do in C++, that's strictly preferable IMO.

(In reply to Bobby Holley (:bholley) (busy with Stylo) from comment #14) > (In reply to Manish Goregaokar [:manishearth] from comment #13) > > I mean, geckolib code is gecko specific :) > > Yes, but there is some code that needs to be in Rust. In general given the > choice, if there's something gecko-specific we can do in C++, that's > strictly preferable IMO. In particular, it means that somebody coming along and deciding to remove this check later can land the fix with less hassle.

try @ https://treeherder.mozilla.org/#/jobs?repo=try&revision=09956c52626716b290dd1a146aea950034636ef8

Pushed by manishearth@gmail.com: https://hg.mozilla.org/integration/autoland/rev/2938b25e3b31 Exit early before calling Servo_HasAuthorSpecifiedRules without element data; r=bholley

Seems like we want Stylo stability fixes on 56 before we start running experiments. Please nominate this for Beta approval when you get a chance.

Comment on attachment 8894639 [details] Bug 1387953 - Exit early before calling Servo_HasAuthorSpecifiedRules without element data; Approval Request Comment [Feature/Bug causing the regression]: Unknown [User impact if declined]: Occasional crashes on stylo beta [Is this code covered by automated tests?]: The crash is not (we don't know what exactly causes it), but the nearby code is [Has the fix been verified in Nightly?]: Yes, no longer seems to occur (nor is it possible, the assert has been removed) [Needs manual test from QE? If yes, steps to reproduce]: No [List of other uplifts needed for the feature/fix]: No [Is the change risky?]: No [Why is the change risky/not risky?]: [String changes made/needed]:

Comment on attachment 8894639 [details] Bug 1387953 - Exit early before calling Servo_HasAuthorSpecifiedRules without element data; Fix a stylo crash. Beta56+. Should be in 56.0b3.