Changeset 201159 in webkit
- Timestamp:
- May 19, 2016 6:22:18 AM (8 years ago)
- Location:
- trunk
- Files:
-
- 7 edited
Legend:
- Unmodified
- Added
- Removed
-
trunk/LayoutTests/ChangeLog
r201130 r201159 1 2016-05-18 Antti Koivisto <antti@apple.com> 2 3 Style resolution for explicitly inherited properties is inefficient 4 https://bugs.webkit.org/show_bug.cgi?id=157860 5 6 Reviewed by Andreas Kling. 7 8 * platform/ios-simulator/fast/dom/HTMLProgressElement/progress-bar-value-pseudo-element-expected.txt: 9 * platform/mac/fast/dom/HTMLProgressElement/progress-bar-value-pseudo-element-expected.txt: 10 11 This is a progression. 12 1 13 2016-05-19 Yoav Weiss <yoav@yoav.ws> 2 14 -
trunk/LayoutTests/platform/ios-simulator/fast/dom/HTMLProgressElement/progress-bar-value-pseudo-element-expected.txt
r179104 r201159 57 57 RenderText {#text} at (0,0) size 235x19 58 58 text run at (0,0) width 235: "Removing appearance dynamically: " 59 Render Progress{PROGRESS} at (234,2) size 161x1759 RenderBlock {PROGRESS} at (234,2) size 161x17 60 60 RenderProgress {DIV} at (0,0) size 160x16 61 61 RenderBlock {DIV} at (0,0) size 160x16 [bgcolor=#808080] -
trunk/LayoutTests/platform/mac/fast/dom/HTMLProgressElement/progress-bar-value-pseudo-element-expected.txt
r177774 r201159 57 57 RenderText {#text} at (0,0) size 235x18 58 58 text run at (0,0) width 235: "Removing appearance dynamically: " 59 Render Progress{PROGRESS} at (234,1) size 161x1759 RenderBlock {PROGRESS} at (234,1) size 161x17 60 60 RenderProgress {DIV} at (0,0) size 160x16 61 61 RenderBlock {DIV} at (0,0) size 160x16 [bgcolor=#808080] -
trunk/Source/WebCore/ChangeLog
r201129 r201159 1 2016-05-18 Antti Koivisto <antti@apple.com> 2 3 Style resolution for explicitly inherited properties is inefficient 4 https://bugs.webkit.org/show_bug.cgi?id=157860 5 6 Reviewed by Andreas Kling. 7 8 We mark the parent style with hasExplicitlyInheritedProperties bit rather than the style that is actually 9 affected by inherited properties. This leads to various inefficiencies including unnecessarily wide style recalcs. 10 11 * css/StyleResolver.cpp: 12 (WebCore::isCacheableInMatchedPropertiesCache): 13 14 Check the style itself rather than the parent. This allows more caching. 15 16 (WebCore::StyleResolver::applyProperty): 17 18 Mark the style rather than the parent. 19 20 * style/StyleChange.cpp: 21 (WebCore::Style::determineChange): 22 23 Remove hasExplicitlyInheritedProperties test and just return NoInherit. Having explicitly inherited 24 properties doesn't make the children inherit them automatically. 25 26 This allows smaller style recalcs. 27 28 * style/StyleTreeResolver.cpp: 29 (WebCore::Style::TreeResolver::resolveElement): 30 31 This patch exposed a bug with appearance property in meter and progress elements. 32 They may construct different renderer based on appearance so we need to force 33 render tree reconstruction when it changes. 34 35 (WebCore::Style::TreeResolver::popParentsToDepth): 36 (WebCore::Style::shouldResolvePseudoElement): 37 38 Don't clear the style recalc bits here. 39 40 (WebCore::Style::shouldResolveElement): 41 42 Add a helper. 43 If the parent had a NoInherit style change, test if the element has existing style with 44 hasExplicitlyInheritedProperties bit. If so we need to re-resolve this element. 45 46 (WebCore::Style::clearNeedsStyleResolution): 47 48 Also clear pseudo elements. 49 50 (WebCore::Style::TreeResolver::resolveComposedTree): 51 1 52 2016-05-19 Youenn Fablet <youenn.fablet@crf.canon.fr> 2 53 -
trunk/Source/WebCore/css/StyleResolver.cpp
r201113 r201159 1256 1256 return false; 1257 1257 // The cache assumes static knowledge about which properties are inherited. 1258 if ( parentStyle->hasExplicitlyInheritedProperties())1258 if (style->hasExplicitlyInheritedProperties()) 1259 1259 return false; 1260 1260 return true; … … 1637 1637 } 1638 1638 1639 if (isInherit && ! state.parentStyle()->hasExplicitlyInheritedProperties() && !CSSProperty::isInheritedProperty(id))1640 const_cast<RenderStyle*>(state.parentStyle())->setHasExplicitlyInheritedProperties();1639 if (isInherit && !CSSProperty::isInheritedProperty(id)) 1640 state.style()->setHasExplicitlyInheritedProperties(); 1641 1641 1642 1642 if (id == CSSPropertyCustom) { -
trunk/Source/WebCore/style/StyleChange.cpp
r194584 r201159 64 64 if (s1.inheritedNotEqual(&s2)) 65 65 return Inherit; 66 if (s1.hasExplicitlyInheritedProperties() || s2.hasExplicitlyInheritedProperties())67 return Inherit;68 66 69 67 return NoInherit; -
trunk/Source/WebCore/style/StyleTreeResolver.cpp
r200633 r201159 33 33 #include "ElementIterator.h" 34 34 #include "HTMLBodyElement.h" 35 #include "HTMLMeterElement.h" 36 #include "HTMLProgressElement.h" 35 37 #include "HTMLSlotElement.h" 36 38 #include "LoaderStrategy.h" … … 199 201 update.isSynthetic = true; 200 202 203 auto* existingStyle = element.renderStyle(); 204 201 205 if (&element == m_document.documentElement()) { 202 206 m_documentElementStyle = RenderStyle::clonePtr(*update.style); … … 205 209 // If "rem" units are used anywhere in the document, and if the document element's font size changes, then force font updating 206 210 // all the way down the tree. This is simpler than having to maintain a cache of objects (and such font size changes should be rare anyway). 207 if (m_document.authorStyleSheets().usesRemUnits() && update.change != NoChange && e lement.renderer() && element.renderer()->style().fontSize() != update.style->fontSize()) {211 if (m_document.authorStyleSheets().usesRemUnits() && update.change != NoChange && existingStyle && existingStyle->fontSize() != update.style->fontSize()) { 208 212 // Cached RenderStyles may depend on the rem units. 209 213 scope().styleResolver.invalidateMatchedPropertiesCache(); … … 216 220 if (&element == m_document.body()) 217 221 m_document.setTextColor(update.style->visitedDependentColor(CSSPropertyColor)); 222 223 // FIXME: These elements should not change renderer based on appearance property. 224 if (is<HTMLMeterElement>(element) || is<HTMLProgressElement>(element)) { 225 if (existingStyle && update.style->appearance() != existingStyle->appearance()) 226 update.change = Detach; 227 } 218 228 219 229 if (update.change != Detach && (parent().change == Force || element.styleChangeType() >= FullStyleChange)) … … 342 352 } 343 353 344 static bool shouldResolvePseudoElement( PseudoElement* pseudoElement)354 static bool shouldResolvePseudoElement(const PseudoElement* pseudoElement) 345 355 { 346 356 if (!pseudoElement) 347 357 return false; 348 bool needsStyleRecalc = pseudoElement->needsStyleRecalc(); 349 pseudoElement->clearNeedsStyleRecalc(); 350 return needsStyleRecalc; 358 return pseudoElement->needsStyleRecalc(); 359 } 360 361 static bool shouldResolveElement(const Element& element, Style::Change parentChange) 362 { 363 if (parentChange >= Inherit) 364 return true; 365 if (parentChange == NoInherit) { 366 auto* existingStyle = element.renderStyle(); 367 if (existingStyle && existingStyle->hasExplicitlyInheritedProperties()) 368 return true; 369 } 370 if (element.needsStyleRecalc()) 371 return true; 372 if (element.hasDisplayContents()) 373 return true; 374 if (shouldResolvePseudoElement(element.beforePseudoElement())) 375 return true; 376 if (shouldResolvePseudoElement(element.afterPseudoElement())) 377 return true; 378 379 return false; 380 } 381 382 static void clearNeedsStyleResolution(Element& element) 383 { 384 element.clearNeedsStyleRecalc(); 385 if (auto* before = element.beforePseudoElement()) 386 before->clearNeedsStyleRecalc(); 387 if (auto* after = element.afterPseudoElement()) 388 after->clearNeedsStyleRecalc(); 351 389 } 352 390 … … 396 434 parent.elementNeedingStyleRecalcAffectsNextSiblingElementStyle = element.affectsNextSiblingElementStyle(); 397 435 398 bool shouldResolveForPseudoElement = shouldResolvePseudoElement(element.beforePseudoElement()) || shouldResolvePseudoElement(element.afterPseudoElement()); 399 400 const RenderStyle* style; 401 Change change; 402 403 bool shouldResolve = parent.change >= Inherit || element.needsStyleRecalc() || shouldResolveForPseudoElement || affectedByPreviousSibling || element.hasDisplayContents(); 436 auto* style = element.renderStyle(); 437 auto change = NoChange; 438 439 bool shouldResolve = shouldResolveElement(element, parent.change) || affectedByPreviousSibling; 404 440 if (shouldResolve) { 405 441 #if PLATFORM(IOS) … … 429 465 m_update->addElement(element, parent.element, WTFMove(elementUpdate)); 430 466 431 element.clearNeedsStyleRecalc(); 432 } else { 433 style = element.renderStyle(); 434 change = NoChange; 467 clearNeedsStyleResolution(element); 435 468 } 436 469
Note: See TracChangeset
for help on using the changeset viewer.