Bug 135149 – Reproducible ASSERT_NOT_REACHED in SelectorChecker::checkOne on my.yahoo.com

• Component: CSS
• Status: RESOLVED FIXED

[Tim Horton]

Steps to Reproduce:

1. Load my.yahoo.com while logged into yahoo.
2. Wait.

Expected: No assert!
Actual: An assertion failure:

Notes: I'm using UI-side compositing in Minibrowser but it seems unlikely either of those are prerequisites for this failure.

SHOULD NEVER BE REACHED
/Users/thorton/src/WebKit/OpenSource/Source/WebCore/css/SelectorChecker.cpp(793) : bool WebCore::SelectorChecker::checkOne(const WebCore::SelectorChecker::SelectorCheckingContext &) const
1   0x10abecf10 WTFCrash
2   0x10d912b13 WebCore::SelectorChecker::checkOne(WebCore::SelectorChecker::SelectorCheckingContext const&) const
3   0x10d910c1e WebCore::SelectorChecker::matchRecursively(WebCore::SelectorChecker::SelectorCheckingContext const&, WebCore::PseudoId&) const
4   0x10d9112cf WebCore::SelectorChecker::matchRecursively(WebCore::SelectorChecker::SelectorCheckingContext const&, WebCore::PseudoId&) const
5   0x10d9112cf WebCore::SelectorChecker::matchRecursively(WebCore::SelectorChecker::SelectorCheckingContext const&, WebCore::PseudoId&) const
6   0x10d910b1a WebCore::SelectorChecker::match(WebCore::SelectorChecker::SelectorCheckingContext const&) const
7   0x10c5689a0 WebCore::ElementRuleCollector::ruleMatches(WebCore::RuleData const&)
8   0x10c566ec5 WebCore::ElementRuleCollector::collectMatchingRulesForList(WTF::Vector<WebCore::RuleData, 0ul, WTF::CrashOnOverflow> const*, WebCore::MatchRequest const&, WebCore::StyleResolver::RuleRange&)
9   0x10c566c76 WebCore::ElementRuleCollector::collectMatchingRules(WebCore::MatchRequest const&, WebCore::StyleResolver::RuleRange&)
10  0x10c5674c8 WebCore::ElementRuleCollector::matchAuthorRules(bool)
11  0x10da346ce WebCore::invalidateStyleRecursively(WebCore::Element&, WebCore::SelectorFilter&, WebCore::DocumentRuleSets const&)
[ ... a bunch of levels of recursion elided ... ]
27  0x10da347e7 WebCore::invalidateStyleRecursively(WebCore::Element&, WebCore::SelectorFilter&, WebCore::DocumentRuleSets const&)
28  0x10da3462d WebCore::StyleInvalidationAnalysis::invalidateStyle(WebCore::Document&)
29  0x10c42f368 WebCore::DocumentStyleSheetCollection::analyzeStyleSheetChange(WebCore::DocumentStyleSheetCollection::UpdateFlag, WTF::Vector<WTF::RefPtr<WebCore::CSSStyleSheet>, 0ul, WTF::CrashOnOverflow> const&, WebCore::DocumentStyleSheetCollection::StyleResolverUpdateType&, bool&)
30  0x10c42f4bf WebCore::DocumentStyleSheetCollection::updateActiveStyleSheets(WebCore::DocumentStyleSheetCollection::UpdateFlag)
31  0x10c3acb04 WebCore::Document::styleResolverChanged(WebCore::StyleResolverUpdateFlag)

[Tim Horton]

(lldb) p selector->pseudoClassType()
(WebCore::CSSSelector::PseudoClassType) $0 = PseudoClassWindowInactive

Which does indeed seem to be missing from that switch.

[Alex Christensen]

I'll look into this.  It is quite upsetting that this was not caught with a test :(

[Alex Christensen]

Created attachment 235327 [details]
Patch

[Tim Horton]

Comment on attachment 235327 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=235327&action=review

> Source/WebCore/css/SelectorChecker.cpp:-792
> -        default:

A++

[WebKit Commit Bot]

Comment on attachment 235327 [details]
Patch

Clearing flags on attachment: 235327

Committed r171378: <http://trac.webkit.org/changeset/171378>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Benjamin Poulain]

Comment on attachment 235327 [details]
Patch

I am only catching up with my mails now. This is amazing. The lack of tests for something this simple baffles me.

Can you please add PseudoClassWindowInactive as an unoptimizedPseudoClass to the CSS JIT?

In SelectorChecker, you should remove the branch "if (context.hasSelectionPseudo) -> check for :window-inactive".

Can you please also add a test for style resolution? It is common to find bugs that only appear in one of style-resolution/querySelector.

[Alex Christensen]

(In reply to comment #7)
> (From update of attachment 235327 [details])
> I am only catching up with my mails now. This is amazing. The lack of tests for something this simple baffles me.
> 
> Can you please add PseudoClassWindowInactive as an unoptimizedPseudoClass to the CSS JIT?
https://bugs.webkit.org/show_bug.cgi?id=135200
> 
> In SelectorChecker, you should remove the branch "if (context.hasSelectionPseudo) -> check for :window-inactive".
> 
> Can you please also add a test for style resolution? It is common to find bugs that only appear in one of style-resolution/querySelector.
There already was a test for style resolution, but not for querySelector, which is why this bug existed.