Title: [89142] trunk/Source/WebCore
- Revision
- 89142
- Author
- commit-qu...@webkit.org
- Date
- 2011-06-17 10:36:43 -0700 (Fri, 17 Jun 2011)
Log Message
2011-06-17 Julien Chaffraix <jchaffr...@google.com>
Reviewed by Darin Adler.
Avoid extra work in RenderStyle::visitedDependentColor
https://bugs.webkit.org/show_bug.cgi?id=62868
Refactoring only, no new test required.
The code used to cache the result of borderStyleForColorProperty. However
the value was either overwritten inside colorIncludingFallback or there was
not border. Thus I removed borderStyleForColorProperty and inlined the logic in
colorIncludingFallback.
This shows some nice performance improvements on the bug page (table of 22k rows with a link
for each row). Using pprof, the time spend in RenderStyle::visitedDependentColor is reduced
by ~10%, mostly due to removing the call to borderStyleForColorProperty.
* rendering/style/RenderStyle.cpp:
(WebCore::RenderStyle::colorIncludingFallback): We now calculate the borderStyle
inside this function (which was already the case I just made it explicit). Also
simplified the final 'if' as the border will be set only for the right CSS border
properties.
(WebCore::RenderStyle::visitedDependentColor): Removed the |borderStyle| variable
as it was never read.
* rendering/style/RenderStyle.h: Removed the parameter.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (89141 => 89142)
--- trunk/Source/WebCore/ChangeLog 2011-06-17 17:19:32 UTC (rev 89141)
+++ trunk/Source/WebCore/ChangeLog 2011-06-17 17:36:43 UTC (rev 89142)
@@ -1,3 +1,32 @@
+2011-06-17 Julien Chaffraix <jchaffr...@google.com>
+
+ Reviewed by Darin Adler.
+
+ Avoid extra work in RenderStyle::visitedDependentColor
+ https://bugs.webkit.org/show_bug.cgi?id=62868
+
+ Refactoring only, no new test required.
+
+ The code used to cache the result of borderStyleForColorProperty. However
+ the value was either overwritten inside colorIncludingFallback or there was
+ not border. Thus I removed borderStyleForColorProperty and inlined the logic in
+ colorIncludingFallback.
+
+ This shows some nice performance improvements on the bug page (table of 22k rows with a link
+ for each row). Using pprof, the time spend in RenderStyle::visitedDependentColor is reduced
+ by ~10%, mostly due to removing the call to borderStyleForColorProperty.
+
+ * rendering/style/RenderStyle.cpp:
+ (WebCore::RenderStyle::colorIncludingFallback): We now calculate the borderStyle
+ inside this function (which was already the case I just made it explicit). Also
+ simplified the final 'if' as the border will be set only for the right CSS border
+ properties.
+
+ (WebCore::RenderStyle::visitedDependentColor): Removed the |borderStyle| variable
+ as it was never read.
+
+ * rendering/style/RenderStyle.h: Removed the parameter.
+
2011-06-16 Pavel Podivilov <podivi...@chromium.org>
Reviewed by Yury Semikhatsky.
Modified: trunk/Source/WebCore/rendering/style/RenderStyle.cpp (89141 => 89142)
--- trunk/Source/WebCore/rendering/style/RenderStyle.cpp 2011-06-17 17:19:32 UTC (rev 89141)
+++ trunk/Source/WebCore/rendering/style/RenderStyle.cpp 2011-06-17 17:36:43 UTC (rev 89142)
@@ -1093,32 +1093,10 @@
}
}
-static EBorderStyle borderStyleForColorProperty(const RenderStyle* style, int colorProperty)
+const Color RenderStyle::colorIncludingFallback(int colorProperty) const
{
- EBorderStyle borderStyle;
- switch (colorProperty) {
- case CSSPropertyBorderLeftColor:
- borderStyle = style->borderLeftStyle();
- break;
- case CSSPropertyBorderRightColor:
- borderStyle = style->borderRightStyle();
- break;
- case CSSPropertyBorderTopColor:
- borderStyle = style->borderTopStyle();
- break;
- case CSSPropertyBorderBottomColor:
- borderStyle = style->borderBottomStyle();
- break;
- default:
- borderStyle = BNONE;
- break;
- }
- return borderStyle;
-}
-
-const Color RenderStyle::colorIncludingFallback(int colorProperty, EBorderStyle borderStyle) const
-{
Color result;
+ EBorderStyle borderStyle = BNONE;
switch (colorProperty) {
case CSSPropertyBackgroundColor:
return backgroundColor(); // Background color doesn't fall back.
@@ -1162,9 +1140,7 @@
}
if (!result.isValid()) {
- if ((colorProperty == CSSPropertyBorderLeftColor || colorProperty == CSSPropertyBorderRightColor
- || colorProperty == CSSPropertyBorderTopColor || colorProperty == CSSPropertyBorderBottomColor)
- && (borderStyle == INSET || borderStyle == OUTSET || borderStyle == RIDGE || borderStyle == GROOVE))
+ if (borderStyle == INSET || borderStyle == OUTSET || borderStyle == RIDGE || borderStyle == GROOVE)
result.setRGB(238, 238, 238);
else
result = color();
@@ -1175,15 +1151,14 @@
const Color RenderStyle::visitedDependentColor(int colorProperty) const
{
- EBorderStyle borderStyle = borderStyleForColorProperty(this, colorProperty);
- Color unvisitedColor = colorIncludingFallback(colorProperty, borderStyle);
+ Color unvisitedColor = colorIncludingFallback(colorProperty);
if (insideLink() != InsideVisitedLink)
return unvisitedColor;
RenderStyle* visitedStyle = getCachedPseudoStyle(VISITED_LINK);
if (!visitedStyle)
return unvisitedColor;
- Color visitedColor = visitedStyle->colorIncludingFallback(colorProperty, borderStyle);
+ Color visitedColor = visitedStyle->colorIncludingFallback(colorProperty);
// FIXME: Technically someone could explicitly specify the color transparent, but for now we'll just
// assume that if the background color is transparent that it wasn't set. Note that it's weird that
Modified: trunk/Source/WebCore/rendering/style/RenderStyle.h (89141 => 89142)
--- trunk/Source/WebCore/rendering/style/RenderStyle.h 2011-06-17 17:19:32 UTC (rev 89141)
+++ trunk/Source/WebCore/rendering/style/RenderStyle.h 2011-06-17 17:36:43 UTC (rev 89142)
@@ -1356,7 +1356,7 @@
const Color& textFillColor() const { return rareInheritedData->textFillColor; }
const Color& textStrokeColor() const { return rareInheritedData->textStrokeColor; }
- const Color colorIncludingFallback(int colorProperty, EBorderStyle) const;
+ const Color colorIncludingFallback(int colorProperty) const;
void appendContent(PassOwnPtr<ContentData>);
};
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes
