Closed Bug 1363244 Opened 3 years ago Closed 3 years ago
stylo: Optimize Servo
_Has Author Specified Rules to avoid calling lazy _pseudo _rules when possible
59 bytes, text/x-review-board-request
This is a follow-up to bug 1349651. From bug 1349651 comment #28: > In the case that IsNativeAnonymous() returns true, we should see if > element->GetPseudoElementType() matches the pseudo on the style context. If > so, we should just ask servo for the primary (not pseudo) rule node for that > element.
Assignee: nobody → mbrubeck
Status: NEW → ASSIGNED
This patch caused 8 tests to pass and 1 to fail: https://treeherder.mozilla.org/#/jobs?repo=try&revision=727be9ca583da2809807cab0f086efb5479f1233 In the failing test "widget/reftests/progressbar-fallback-default-style.html", `elem` and `aFrame->StyleContext()` both have their pseudo type set to ":-moz-progress-bar", but has_author_style_rules does not find any border properties in the primary rule nodes for `elem`.
Hm, so this patch definitely looks right, but it's supposed to be a transparent optimization, so the fact that it changes behavior means that there's something wrong somewhere (not necessarily in this patch). Emilio, WDYT?
Can you retry this on top of emilio's patch in bug 1364377 (with the appropriate expectation adjustments from his try push) and see if this is now a transparent optimization?
(In reply to Bobby Holley (:bholley) (busy with Stylo) from comment #4) > Can you retry this on top of emilio's patch in bug 1364377 (with the > appropriate expectation adjustments from his try push) and see if this is > now a transparent optimization? https://treeherder.mozilla.org/#/jobs?repo=try&revision=5abe1441370f18cb5b33f539f860704ad7e05466
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Is this actually fixed?
It's probably not. That being said, in bug 1364412 I'm simplifying a lot of this to avoid resolving style in HasAuthorSpecifiedRules at all.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
When combined with the patch from bug 1364377, this causes even more unexpected passes, and zero new failures. (Perhaps these tests were previously failing because the original code was not resolving the correct rule node, maybe because it passed an incorrect combination of element+pseudo?). Emilio, do you want to fold this into bug 1364412?
Yes, I think that makes sense, since bug 1364412 also contains those new passes.
Status: REOPENED → RESOLVED
Closed: 3 years ago → 3 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1364412
You need to log in before you can comment on or make changes to this bug.