Summary: | Stop copying author shadow pseudo rules into shadow tree style resolver | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Antti Koivisto <koivisto> | ||||||||||
Component: | CSS | Assignee: | Nobody <webkit-unassigned> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | buildbot, rniwa | ||||||||||
Priority: | P2 | ||||||||||||
Version: | WebKit Nightly Build | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Attachments: |
|
Description
Antti Koivisto
2016-10-10 12:01:19 PDT
Created attachment 291141 [details]
patch
Created attachment 291144 [details]
patch
Comment on attachment 291144 [details] patch Attachment 291144 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/2257435 New failing tests: media/track/track-css-matching-lang.html media/track/track-css-property-whitelist.html media/track/track-css-cue-lifetime.html media/track/track-css-matching.html media/track/track-css-matching-timestamps.html Created attachment 291152 [details]
Archive of layout-test-results from ews101 for mac-yosemite
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101 Port: mac-yosemite Platform: Mac OS X 10.10.5
Created attachment 291155 [details]
patch
Comment on attachment 291155 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=291155&action=review > Source/WebCore/css/ElementRuleCollector.cpp:229 > + auto& shadowRoot = *m_element.containingShadowRoot(); What guarantees this is non-null? > Source/WebCore/css/ElementRuleCollector.cpp:233 > + auto& hostAuthorRules = Style::Scope::forNode(*shadowRoot.host()).resolver().ruleSets().authorStyle(); What guarantees shadowRoot.host() is non-null? > Source/WebCore/css/ElementRuleCollector.cpp:234 > + MatchRequest hostAuthorRequest(&hostAuthorRules, matchRequest.includeEmptyRules); Anders and I would write this in this new coding style: MatchRequest hostAuthorRequest { &hostAuthorRules, matchRequest.includeEmptyRules }; Has the benefit of not looking anything like a function call and another benefit is that it won’t do implicit type conversions like silently converting an int to a boolean. > Source/WebCore/css/ElementRuleCollector.cpp:296 > + auto& rules = *matchRequest.ruleSet; What guarantees ruleSet is non-null? > What guarantees this is non-null? The call site tests we are in a shadow tree. Added an assert. > What guarantees shadowRoot.host() is non-null? We should never be resolving style for a disconnected shadow tree. > Anders and I would write this in this new coding style: > > MatchRequest hostAuthorRequest { &hostAuthorRules, > matchRequest.includeEmptyRules }; > > Has the benefit of not looking anything like a function call and another > benefit is that it won’t do implicit type conversions like silently > converting an int to a boolean. I'm not sure I understand the first point. Invoking a constructor is a function call so looking like one doesn't seem so wrong to me. > What guarantees ruleSet is non-null? Call sites assert or set it explicitly. |