Modified: trunk/LayoutTests/ChangeLog (153346 => 153347)
--- trunk/LayoutTests/ChangeLog 2013-07-25 21:13:28 UTC (rev 153346)
+++ trunk/LayoutTests/ChangeLog 2013-07-25 21:40:55 UTC (rev 153347)
@@ -1,3 +1,16 @@
+2013-07-25 Ryosuke Niwa <[email protected]>
+
+ Don't force layout when querying a fixed or non-box margin/padding property
+ https://bugs.webkit.org/show_bug.cgi?id=118032
+
+ Reviewed by David Hyatt.
+
+ Add a regression test inspired by the one added in
+ https://chromium.googlesource.com/chromium/blink/+/ff234b1593b2b493d47f38f687d09a87bc42c9eb.
+
+ * fast/css/computed-width-without-renderer-expected.txt: Added.
+ * fast/css/computed-width-without-renderer.html: Added.
+
2013-07-25 Bear Travis <[email protected]>
[CSS Shapes] Fix typo in simple-polygon.js
Added: trunk/LayoutTests/fast/css/computed-width-without-renderer-expected.txt (0 => 153347)
--- trunk/LayoutTests/fast/css/computed-width-without-renderer-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/css/computed-width-without-renderer-expected.txt 2013-07-25 21:40:55 UTC (rev 153347)
@@ -0,0 +1,7 @@
+This tests obtaining widths of the same CSSComputedStyleDeclaration twice immediately after inserting a stylesheet. They should match.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS document.body.appendChild(div); style = getComputedStyle(div); style.width is style.width
+
Added: trunk/LayoutTests/fast/css/computed-width-without-renderer.html (0 => 153347)
--- trunk/LayoutTests/fast/css/computed-width-without-renderer.html (rev 0)
+++ trunk/LayoutTests/fast/css/computed-width-without-renderer.html 2013-07-25 21:40:55 UTC (rev 153347)
@@ -0,0 +1,26 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src=""
+<script>
+if (window.testRunner)
+ testRunner.dumpAsText();
+
+var div;
+
+function runTest() {
+ description("This tests obtaining widths of the same CSSComputedStyleDeclaration twice immediately after inserting a stylesheet. They should match.");
+
+ var link = document.createElement('link');
+ link.rel = 'stylesheet';
+ link.href = ""
+ document.head.appendChild(link);
+
+ div = document.createElement('div');
+ shouldBe("document.body.appendChild(div); style = getComputedStyle(div); style.width", "style.width");
+}
+</script>
+</head>
+<body _onload_="runTest()">
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (153346 => 153347)
--- trunk/Source/WebCore/ChangeLog 2013-07-25 21:13:28 UTC (rev 153346)
+++ trunk/Source/WebCore/ChangeLog 2013-07-25 21:40:55 UTC (rev 153347)
@@ -1,3 +1,27 @@
+2013-07-25 Ryosuke Niwa <[email protected]>
+
+ Don't force layout when querying a fixed or non-box margin/padding property
+ https://bugs.webkit.org/show_bug.cgi?id=118032
+
+ Reviewed by David Hyatt.
+
+ Merge https://chromium.googlesource.com/chromium/blink/+/66427d0825fcc2975bd50220cdcaa2504d6f36e5.
+
+ This patch avoids layout in ComputedStyleExtractor::propertyValue for margin and padding properties
+ when they are of fixed length. According to the Blink patch's author, this improves the page load
+ time of economist.com by 27%.
+
+ The actual code change is significantly different from the original Blink patch since we've done
+ some refactorins in r152938 and r153067 to make this change more self-contained.
+
+ Test: fast/css/computed-width-without-renderer.html
+
+ * css/CSSComputedStyleDeclaration.cpp:
+ (WebCore::zoomAdjustedPaddingOrMarginPixelValue):
+ (WebCore::paddingOrMarginIsRendererDependent):
+ (WebCore::isLayoutDependent):
+ (WebCore::ComputedStyleExtractor::propertyValue):
+
2013-07-25 Sam Weinig <[email protected]>
-[WebHTMLView attributedSubstringForProposedRange:actualRange:] does not include strikethrough attribute in the returned attributed string
Modified: trunk/Source/WebCore/css/CSSComputedStyleDeclaration.cpp (153346 => 153347)
--- trunk/Source/WebCore/css/CSSComputedStyleDeclaration.cpp 2013-07-25 21:13:28 UTC (rev 153346)
+++ trunk/Source/WebCore/css/CSSComputedStyleDeclaration.cpp 2013-07-25 21:40:55 UTC (rev 153347)
@@ -1520,21 +1520,31 @@
return cssValuePool().createIdentifierValue(CSSValueNormal);
}
-static bool isLayoutDependentProperty(CSSPropertyID propertyID)
+typedef Length (RenderStyle::*RenderStyleLengthGetter)() const;
+typedef LayoutUnit (RenderBoxModelObject::*RenderBoxComputedCSSValueGetter)() const;
+
+template<RenderStyleLengthGetter lengthGetter, RenderBoxComputedCSSValueGetter computedCSSValueGetter>
+inline PassRefPtr<CSSValue> zoomAdjustedPaddingOrMarginPixelValue(RenderStyle* style, RenderObject* renderer)
{
+ Length unzoomzedLength = (style->*lengthGetter)();
+ if (!renderer || !renderer->isBox() || unzoomzedLength.isFixed())
+ return zoomAdjustedPixelValueForLength(unzoomzedLength, style);
+ return zoomAdjustedPixelValue((toRenderBox(renderer)->*computedCSSValueGetter)(), style);
+}
+
+template<RenderStyleLengthGetter lengthGetter>
+inline bool paddingOrMarginIsRendererDependent(RenderStyle* style, RenderObject* renderer)
+{
+ if (!renderer || !renderer->isBox())
+ return false;
+ return !(style && (style->*lengthGetter)().isFixed());
+}
+
+static bool isLayoutDependent(CSSPropertyID propertyID, RenderStyle* style, RenderObject* renderer)
+{
switch (propertyID) {
case CSSPropertyWidth:
case CSSPropertyHeight:
- case CSSPropertyMargin:
- case CSSPropertyMarginTop:
- case CSSPropertyMarginBottom:
- case CSSPropertyMarginLeft:
- case CSSPropertyMarginRight:
- case CSSPropertyPadding:
- case CSSPropertyPaddingTop:
- case CSSPropertyPaddingBottom:
- case CSSPropertyPaddingLeft:
- case CSSPropertyPaddingRight:
case CSSPropertyWebkitPerspectiveOrigin:
case CSSPropertyWebkitTransformOrigin:
case CSSPropertyWebkitTransform:
@@ -1542,6 +1552,34 @@
case CSSPropertyWebkitFilter:
#endif
return true;
+ case CSSPropertyMargin: {
+ if (!renderer || !renderer->isBox())
+ return false;
+ return !(style && style->marginTop().isFixed() && style->marginRight().isFixed()
+ && style->marginBottom().isFixed() && style->marginLeft().isFixed());
+ }
+ case CSSPropertyMarginTop:
+ return paddingOrMarginIsRendererDependent<&RenderStyle::marginTop>(style, renderer);
+ case CSSPropertyMarginRight:
+ return paddingOrMarginIsRendererDependent<&RenderStyle::marginRight>(style, renderer);
+ case CSSPropertyMarginBottom:
+ return paddingOrMarginIsRendererDependent<&RenderStyle::marginBottom>(style, renderer);
+ case CSSPropertyMarginLeft:
+ return paddingOrMarginIsRendererDependent<&RenderStyle::marginLeft>(style, renderer);
+ case CSSPropertyPadding: {
+ if (!renderer || !renderer->isBox())
+ return false;
+ return !(style && style->paddingTop().isFixed() && style->paddingRight().isFixed()
+ && style->paddingBottom().isFixed() && style->paddingLeft().isFixed());
+ }
+ case CSSPropertyPaddingTop:
+ return paddingOrMarginIsRendererDependent<&RenderStyle::paddingTop>(style, renderer);
+ case CSSPropertyPaddingRight:
+ return paddingOrMarginIsRendererDependent<&RenderStyle::paddingRight>(style, renderer);
+ case CSSPropertyPaddingBottom:
+ return paddingOrMarginIsRendererDependent<&RenderStyle::paddingBottom>(style, renderer);
+ case CSSPropertyPaddingLeft:
+ return paddingOrMarginIsRendererDependent<&RenderStyle::paddingLeft>(style, renderer);
default:
return false;
}
@@ -1596,47 +1634,48 @@
return styledNode->computedStyle(styledNode->isPseudoElement() ? NOPSEUDO : pseudoElementSpecifier);
}
-typedef Length (RenderStyle::*RenderStyleLengthGetter)() const;
-typedef LayoutUnit (RenderBoxModelObject::*RenderBoxComputedCSSValueGetter)() const;
-
-template<RenderStyleLengthGetter lengthGetter, RenderBoxComputedCSSValueGetter computedCSSValueGetter>
-inline PassRefPtr<CSSValue> zoomAdjustedPaddingOrMarginPixelValue(RenderStyle* style, RenderObject* renderer)
-{
- Length unzoomedLength = (style->*lengthGetter)();
- if (unzoomedLength.isFixed() || !renderer || !renderer->isBox())
- return zoomAdjustedPixelValueForLength(unzoomedLength, style);
- return zoomAdjustedPixelValue((toRenderBox(renderer)->*computedCSSValueGetter)(), style);
-}
-
PassRefPtr<CSSValue> ComputedStyleExtractor::propertyValue(CSSPropertyID propertyID, EUpdateLayout updateLayout) const
{
Node* styledNode = this->styledNode();
if (!styledNode)
return 0;
+ RefPtr<RenderStyle> style;
+ RenderObject* renderer;
+ bool forceFullLayout = false;
if (updateLayout) {
Document* document = styledNode->document();
+
+ if (nodeOrItsAncestorNeedsStyleRecalc(styledNode)) {
+ document->updateStyleIfNeeded();
+ // The style recalc could have caused the styled node to be discarded or replaced
+ // if it was a PseudoElement so we need to update it.
+ styledNode = this->styledNode();
+ }
+
+ style = computeRenderStyleForProperty(styledNode, m_pseudoElementSpecifier, propertyID);
+ renderer = styledNode->renderer();
+
// FIXME: Some of these cases could be narrowed down or optimized better.
- bool forceFullLayout = isLayoutDependentProperty(propertyID)
+ forceFullLayout = isLayoutDependent(propertyID, style.get(), renderer)
|| styledNode->isInShadowTree()
|| (document->styleResolverIfExists() && document->styleResolverIfExists()->hasViewportDependentMediaQueries() && document->ownerElement())
|| document->seamlessParentIFrame();
- if (forceFullLayout)
+ if (forceFullLayout) {
document->updateLayoutIgnorePendingStylesheets();
- else if (nodeOrItsAncestorNeedsStyleRecalc(styledNode))
- document->updateStyleIfNeeded();
+ styledNode = this->styledNode();
+ }
+ }
- // The style recalc could have caused the styled node to be discarded or replaced
- // if it was a PseudoElement so we need to update it.
- styledNode = this->styledNode();
+ if (!updateLayout || forceFullLayout) {
+ style = computeRenderStyleForProperty(styledNode, m_pseudoElementSpecifier, propertyID);
+ renderer = styledNode->renderer();
}
- RefPtr<RenderStyle> style = computeRenderStyleForProperty(styledNode, m_pseudoElementSpecifier, propertyID);
if (!style)
return 0;
- RenderObject* renderer = styledNode->renderer();
propertyID = CSSProperty::resolveDirectionAwareProperty(propertyID, style->direction(), style->writingMode());
switch (propertyID) {
