Closed Bug 1377975 Opened 7 years ago Closed 7 years ago

stylo: debug_assert!(cascade_visited.has_rules(primary_inputs)) in cascade_eager_pseudo

Categories

(Core :: CSS Parsing and Computation, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: hiro, Assigned: jryans)

References

(Blocks 1 open bug, )

Details

Attachments

(2 files)

I just confirmed that bug 1376352 fixed the panic bug (bug 1377197) on http://prestodb.rocks/code/simd/ . But the site causes another panic [1] on debug build. To reproduce this, you need to apply the patch in bug 1374175, otherwise you will see the crash while loading the site. STR: 1) Load http://prestodb.rocks/code/simd/ 2) After finish the loading, move mouse cursor on some links #6 0x00007fffeadc61ab in std::panicking::begin_panic<&str> (msg=..., file_line=0x0) at /checkout/src/libstd/panicking.rs:511 #7 0x00007fffeaa5d110 in style::matching::PrivateMatchMethods::cascade_eager_pseudo<style::gecko::wrapper::GeckoElement> (self=<optimized out>, context=0x7fffca0f6380, data=<optimized out>, pseudo=0x7fffca0f5ed6, cascade_visited=<optimized out>) at /home/ikezoe/autoland/servo/components/style/matching.rs:618 #8 0x00007fffeaa5eff6 in style::matching::MatchMethods::cascade_pseudos<style::gecko::wrapper::GeckoElement> (self=<optimized out>, context=0x7fffca0f6380, data=0x7fffca2222e8, cascade_visited=<optimized out>) at /home/ikezoe/autoland/servo/components/style/matching.rs:1647 #9 0x00007fffeaa5d9f7 in style::matching::MatchMethods::cascade_primary_and_pseudos<style::gecko::wrapper::GeckoElement> (self=0x7fffca0f6198, context=0x7fffca0f6380, data=0x7fffca2222e8, important_rules_changed=<optimized out>) at /home/ikezoe/autoland/servo/components/style/matching.rs:1094 #10 0x00007fffeaa69f82 in style::traversal::compute_style<style::gecko::wrapper::GeckoElement,style::gecko::traversal::RecalcStyleOnly> (_traversal=<optimized out>, traversal_data=<optimized out>, context=<optimized out>, element=..., data=<optimized out>) at /home/ikezoe/autoland/servo/components/style/traversal.rs:876 #11 0x00007fffeaa698a6 in style::traversal::recalc_style_at<style::gecko::wrapper::GeckoElement,style::gecko::traversal::RecalcStyleOnly> (traversal=<optimized out>, traversal_data=<optimized out>, context=<optimized out>, element=..., data=<optimized out>) at /home/ikezoe/autoland/servo/components/style/traversal.rs:707 #12 0x00007fffeaa51a7c in style::gecko::traversal::{{impl}}::process_preorder (self=<optimized out>, traversal_data=0x7fffca0f70f0, thread_local=0x7fffce0f8bd8, node=...) at /home/ikezoe/autoland/servo/components/style/gecko/traversal.rs:46 #13 0x00007fffea8d7d6b in style::parallel::top_down_dom<style::gecko::wrapper::GeckoElement,style::gecko::traversal::RecalcStyleOnly> (nodes=..., recursion_depth=9, traversal_data=..., root=..., scope=<optimized out>, pool=<optimized out>, traversal=<optimized out>, tls=<optimized out>) at /home/ikezoe/autoland/servo/components/style/parallel.rs:202 [1] https://hg.mozilla.org/mozilla-central/file/a3b192dc8344/servo/components/style/matching.rs#l615
I'll look into this.
Assignee: nobody → jryans
Well, this was pretty interesting to track down. :) It's related to a CSS transition on a link ending, which blows away the visited style (partly because that was deferred for transitions) since it's currently not included in the after change style used when a transition update is needed. Working on a reduced test case.
Attachment #8884444 - Flags: review?(hikezoe) → review+
Comment on attachment 8884445 [details] Bug 1377975 - Test cascading pseudos during link transitions. https://reviewboard.mozilla.org/r/155356/#review160526 ::: layout/style/crashtests/link-transition-before.html:20 (Diff revision 1) > +</style> > +<a href="http://www.example.com/">example</a> > +<script> > +let a0 = document.querySelectorAll("a")[0]; > +a0.classList.add("start"); > +setTimeout(() => { The reason we can't use requestAnimationFrame is that visited check is asynchronously processed? Can't we use requestIdleCallback instead of timeout? ::: layout/style/crashtests/link-transition-before.html:23 (Diff revision 1) > + a0.classList.add("start"); > + document.documentElement.removeAttribute("class"); I am wondering if the crash happens after this classList.add() call we need to wait for one more restyling process?
Attachment #8884445 - Flags: review?(hikezoe) → review+
Comment on attachment 8884444 [details] Bug 1377975 - Pass through visited style for after change. https://reviewboard.mozilla.org/r/155354/#review160546 ::: servo/components/style/matching.rs:719 (Diff revision 1) > new_values: &mut Arc<ComputedValues>, > important_rules_changed: bool) { > use context::{CASCADE_RESULTS, CSS_ANIMATIONS, CSS_TRANSITIONS, EFFECT_PROPERTIES}; > use context::UpdateAnimationsTasks; > > + // Bug 868975: These steps should examine and update the visited styles (just a side note, probably worth waiting until bug 1379505 for working on this one, since I'm refactoring a bunch of this file to make it less error-prone).
Comment on attachment 8884445 [details] Bug 1377975 - Test cascading pseudos during link transitions. https://reviewboard.mozilla.org/r/155356/#review160526 > The reason we can't use requestAnimationFrame is that visited check is asynchronously processed? Can't we use requestIdleCallback instead of timeout? With `requestAnimationFrame`, it does crash, but only about 50% of the time, so it seems like there's a race there causing it to be intermittent. With `requestIdleCallback`, it appears to crash consistently, so I'll change to that. > I am wondering if the crash happens after this classList.add() call we need to wait for one more restyling process? That actually seems to make the crash less likely, so I'll leave this as-is.
Pushed by jryans@gmail.com: https://hg.mozilla.org/integration/autoland/rev/c1e41e798465 Test cascading pseudos during link transitions. r=hiro
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: