Changeset 204220 in webkit
- Timestamp:
- Aug 5, 2016 10:37:28 PM (8 years ago)
- Location:
- trunk/Source/WebCore
- Files:
-
- 12 edited
Legend:
- Unmodified
- Added
- Removed
-
trunk/Source/WebCore/ChangeLog
r204219 r204220 1 2016-08-05 Jonathan Bedard <jbedard@apple.com> 2 3 NULL Reference Error in ElementRuleCollector 4 https://bugs.webkit.org/show_bug.cgi?id=160362 5 6 Reviewed by Darin Adler. 7 8 No new tests, existing CSS tests cover this change. 9 10 Undefined behavior sanitizer found a reference bound to a NULL pointer. 11 The root cause of this issue was a discrepancy between whether an author style needed to be defined. In some logic, an undefined author style was considered acceptable, but in other logic, author style was always assumed to be defined. To fix this, a variable was added so that while author style is always defined, there is a flag indicating if this definition occurred in the constructor for use by functions which allow an undefined author style. 12 13 * css/DocumentRuleSets.cpp: 14 (WebCore::DocumentRuleSets::DocumentRuleSets): Define author style by default. 15 (WebCore::DocumentRuleSets::resetAuthorStyle): Switch author style flag. 16 * css/DocumentRuleSets.h: Added author style flag, changed authorStyle accessor to reference from pointer. 17 * css/ElementRuleCollector.cpp: 18 (WebCore::ElementRuleCollector::ElementRuleCollector): Original location of undefined behavior. 19 (WebCore::ElementRuleCollector::matchHostPseudoClassRules): Changed pointer to reference. 20 (WebCore::ElementRuleCollector::matchSlottedPseudoElementRules): Changed pointer to reference. 21 * css/PageRuleCollector.cpp: 22 (WebCore::PageRuleCollector::matchAllPageRules): Check new flag, changed pointer to reference. 23 24 * css/StyleResolver.cpp: Changed pointer to reference. 25 * dom/Document.cpp: Dito. 26 * style/AttributeChangeInvalidation.cpp: Dito. 27 * style/ClassChangeInvalidation.cpp: Dito. 28 * style/IdChangeInvalidation.cpp: Dito. 29 * style/StyleSharingResolver.cpp: Dito. 30 1 31 2016-08-05 Chris Dumez <cdumez@apple.com> 2 32 -
trunk/Source/WebCore/css/DocumentRuleSets.cpp
r202104 r204220 40 40 DocumentRuleSets::DocumentRuleSets() 41 41 { 42 m_authorStyle = std::make_unique<RuleSet>(); 43 m_authorStyle->disableAutoShrinkToFit(); 42 44 } 43 45 … … 79 81 void DocumentRuleSets::resetAuthorStyle() 80 82 { 83 m_isAuthorStyleDefined = true; 81 84 m_authorStyle = std::make_unique<RuleSet>(); 82 85 m_authorStyle->disableAutoShrinkToFit(); -
trunk/Source/WebCore/css/DocumentRuleSets.h
r199735 r204220 45 45 DocumentRuleSets(); 46 46 ~DocumentRuleSets(); 47 RuleSet* authorStyle() const { return m_authorStyle.get(); } 47 48 bool isAuthorStyleDefined() const { return m_isAuthorStyleDefined; } 49 RuleSet& authorStyle() const { return *m_authorStyle.get(); } 48 50 RuleSet* userStyle() const { return m_userStyle.get(); } 49 51 const RuleFeatureSet& features() const; … … 70 72 void collectRulesFromUserStyleSheets(const Vector<RefPtr<CSSStyleSheet>>&, RuleSet& userStyle, const MediaQueryEvaluator&, StyleResolver&); 71 73 74 bool m_isAuthorStyleDefined { false }; 72 75 std::unique_ptr<RuleSet> m_authorStyle; 73 76 std::unique_ptr<RuleSet> m_userStyle; -
trunk/Source/WebCore/css/ElementRuleCollector.cpp
r202091 r204220 83 83 ElementRuleCollector::ElementRuleCollector(const Element& element, const DocumentRuleSets& ruleSets, const SelectorFilter* selectorFilter) 84 84 : m_element(element) 85 , m_authorStyle( *ruleSets.authorStyle())85 , m_authorStyle(ruleSets.authorStyle()) 86 86 , m_userStyle(ruleSets.userStyle()) 87 87 , m_selectorFilter(selectorFilter) … … 235 235 matchRequest.treeContextOrdinal++; 236 236 237 auto& shadowAuthorStyle = *m_element.shadowRoot()->styleResolver().ruleSets().authorStyle();237 auto& shadowAuthorStyle = m_element.shadowRoot()->styleResolver().ruleSets().authorStyle(); 238 238 auto& shadowHostRules = shadowAuthorStyle.hostPseudoClassRules(); 239 239 if (shadowHostRules.isEmpty()) … … 266 266 // In nested case the slot may itself be assigned to a slot. Collect ::slotted rules from all the nested trees. 267 267 maybeSlotted = slot; 268 auto* shadowAuthorStyle = hostShadowRoot->styleResolver().ruleSets().authorStyle(); 269 if (!shadowAuthorStyle) 268 if (!hostShadowRoot->styleResolver().ruleSets().isAuthorStyleDefined()) 270 269 continue; 271 270 // Find out if there are any ::slotted rules in the shadow tree matching the current slot. 272 271 // FIXME: This is really part of the slot style and could be cached when resolving it. 273 ElementRuleCollector collector(*slot, *shadowAuthorStyle, nullptr);272 ElementRuleCollector collector(*slot, hostShadowRoot->styleResolver().ruleSets().authorStyle(), nullptr); 274 273 auto slottedPseudoElementRules = collector.collectSlottedPseudoElementRulesForSlot(matchRequest.includeEmptyRules); 275 274 if (!slottedPseudoElementRules) -
trunk/Source/WebCore/css/PageRuleCollector.cpp
r203322 r204220 71 71 matchPageRules(m_ruleSets.userStyle(), isLeft, isFirst, page); 72 72 // Only consider the global author RuleSet for @page rules, as per the HTML5 spec. 73 matchPageRules(m_ruleSets.authorStyle(), isLeft, isFirst, page); 73 if (m_ruleSets.isAuthorStyleDefined()) 74 matchPageRules(&m_ruleSets.authorStyle(), isLeft, isFirst, page); 74 75 } 75 76 -
trunk/Source/WebCore/css/StyleResolver.cpp
r202738 r204220 1062 1062 bool StyleResolver::checkRegionStyle(const Element* regionElement) 1063 1063 { 1064 unsigned rulesSize = m_ruleSets.authorStyle() ->regionSelectorsAndRuleSets().size();1064 unsigned rulesSize = m_ruleSets.authorStyle().regionSelectorsAndRuleSets().size(); 1065 1065 for (unsigned i = 0; i < rulesSize; ++i) { 1066 ASSERT(m_ruleSets.authorStyle() ->regionSelectorsAndRuleSets().at(i).ruleSet.get());1067 if (checkRegionSelector(m_ruleSets.authorStyle() ->regionSelectorsAndRuleSets().at(i).selector, regionElement))1066 ASSERT(m_ruleSets.authorStyle().regionSelectorsAndRuleSets().at(i).ruleSet.get()); 1067 if (checkRegionSelector(m_ruleSets.authorStyle().regionSelectorsAndRuleSets().at(i).selector, regionElement)) 1068 1068 return true; 1069 1069 } -
trunk/Source/WebCore/dom/AuthorStyleSheets.cpp
r203601 r204220 358 358 359 359 userAgentShadowTreeStyleResolver.ruleSets().resetAuthorStyle(); 360 auto& authorRuleSet = *styleResolver.ruleSets().authorStyle();360 auto& authorRuleSet = styleResolver.ruleSets().authorStyle(); 361 361 if (authorRuleSet.hasShadowPseudoElementRules()) 362 userAgentShadowTreeStyleResolver.ruleSets().authorStyle() ->copyShadowPseudoElementRulesFrom(authorRuleSet);362 userAgentShadowTreeStyleResolver.ruleSets().authorStyle().copyShadowPseudoElementRulesFrom(authorRuleSet); 363 363 } 364 364 -
trunk/Source/WebCore/dom/Document.cpp
r204196 r204220 2206 2206 2207 2207 // FIXME: Filter out shadow pseudo elements we don't want to expose to authors. 2208 auto& documentAuthorStyle = *ensureStyleResolver().ruleSets().authorStyle();2208 auto& documentAuthorStyle = ensureStyleResolver().ruleSets().authorStyle(); 2209 2209 if (documentAuthorStyle.hasShadowPseudoElementRules()) 2210 m_userAgentShadowTreeStyleResolver->ruleSets().authorStyle() ->copyShadowPseudoElementRulesFrom(documentAuthorStyle);2210 m_userAgentShadowTreeStyleResolver->ruleSets().authorStyle().copyShadowPseudoElementRulesFrom(documentAuthorStyle); 2211 2211 } 2212 2212 -
trunk/Source/WebCore/style/AttributeChangeInvalidation.cpp
r202227 r204220 39 39 { 40 40 auto& shadowRuleSets = shadowRoot.styleResolver().ruleSets(); 41 if (shadowRuleSets.authorStyle() ->hostPseudoClassRules().isEmpty())41 if (shadowRuleSets.authorStyle().hostPseudoClassRules().isEmpty()) 42 42 return false; 43 43 … … 69 69 } 70 70 71 if (m_element.shadowRoot() && ruleSets.authorStyle() ->hasShadowPseudoElementRules()) {71 if (m_element.shadowRoot() && ruleSets.authorStyle().hasShadowPseudoElementRules()) { 72 72 m_element.setNeedsStyleRecalc(FullStyleChange); 73 73 return; -
trunk/Source/WebCore/style/ClassChangeInvalidation.cpp
r202227 r204220 89 89 { 90 90 auto& shadowRuleSets = shadowRoot.styleResolver().ruleSets(); 91 if (shadowRuleSets.authorStyle() ->hostPseudoClassRules().isEmpty())91 if (shadowRuleSets.authorStyle().hostPseudoClassRules().isEmpty()) 92 92 return false; 93 93 return shadowRuleSets.features().classesInRules.contains(changedClass); … … 115 115 return; 116 116 117 if (shadowRoot && ruleSets.authorStyle() ->hasShadowPseudoElementRules()) {117 if (shadowRoot && ruleSets.authorStyle().hasShadowPseudoElementRules()) { 118 118 m_element.setNeedsStyleRecalc(FullStyleChange); 119 119 return; -
trunk/Source/WebCore/style/IdChangeInvalidation.cpp
r202227 r204220 38 38 { 39 39 auto& shadowRuleSets = shadowRoot.styleResolver().ruleSets(); 40 if (shadowRuleSets.authorStyle() ->hostPseudoClassRules().isEmpty())40 if (shadowRuleSets.authorStyle().hostPseudoClassRules().isEmpty()) 41 41 return false; 42 42 … … 60 60 return; 61 61 62 if (m_element.shadowRoot() && ruleSets.authorStyle() ->hasShadowPseudoElementRules()) {62 if (m_element.shadowRoot() && ruleSets.authorStyle().hasShadowPseudoElementRules()) { 63 63 m_element.setNeedsStyleRecalc(FullStyleChange); 64 64 return; -
trunk/Source/WebCore/style/StyleSharingResolver.cpp
r203324 r204220 98 98 if (elementHasDirectionAuto(element)) 99 99 return nullptr; 100 if (element.shadowRoot() && !element.shadowRoot()->styleResolver().ruleSets().authorStyle() ->hostPseudoClassRules().isEmpty())100 if (element.shadowRoot() && !element.shadowRoot()->styleResolver().ruleSets().authorStyle().hostPseudoClassRules().isEmpty()) 101 101 return nullptr; 102 102 … … 287 287 return false; 288 288 289 if (element.shadowRoot() && !element.shadowRoot()->styleResolver().ruleSets().authorStyle() ->hostPseudoClassRules().isEmpty())289 if (element.shadowRoot() && !element.shadowRoot()->styleResolver().ruleSets().authorStyle().hostPseudoClassRules().isEmpty()) 290 290 return false; 291 291
Note: See TracChangeset
for help on using the changeset viewer.