Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(42)

Issue 53143002: Avoid calling SVGFontFaceElement::rebuildFontFace() for non font-face specific attributes (Closed)

Created:
7 years, 1 month ago by Inactive
Modified:
7 years, 1 month ago
CC:
blink-reviews, pdr, Stephen Chennney, f(malita), lgombos, rwlbuis
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Visibility:
Public.

Description

Avoid calling SVGFontFaceElement::rebuildFontFace() for non font-face specific attributes When enabling warnings at compile time, the compiler complains that the cssPropertyIdForSVGAttributeName() function in SVGFontFaceElement.cpp is unused, even though SVGFontFaceElement.cpp is referring to this function. The reason is that a function with the same name exists in the parent SVGElement class and this one ends up getting called instead. The proposed solution is to rename the function to cssPropertyIdForFontFaceAttributeName() instead in SVGFontFaceElement.cpp to avoid the name clash and make sure SVGFontFaceElement::parseAttribute() calls the right function. cssPropertyIdForFontFaceAttributeName() returns the CSSPropertyID only for attribute names specific to the FontFace element, while cssPropertyIdForSVGAttributeName() in SVGElement was returning the CSSPropertyID for ALL styling attributes. As a consequence, we avoid unnecessary calls to SVGFontFaceElement::rebuildFontFace() for non font-face specific attributes. BUG=312287 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=160963

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+2 lines, -2 lines) Patch
M Source/core/svg/SVGFontFaceElement.cpp View 2 chunks +2 lines, -2 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
Inactive
7 years, 1 month ago (2013-10-30 19:01:24 UTC) #1
Stephen Chennney
lgtm
7 years, 1 month ago (2013-10-30 19:05:53 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/ch.dumez@samsung.com/53143002/1
7 years, 1 month ago (2013-10-30 19:06:55 UTC) #3
commit-bot: I haz the power
7 years, 1 month ago (2013-10-30 20:07:08 UTC) #4
Message was sent while issue was closed.
Change committed as 160963

Powered by Google App Engine
This is Rietveld 408576698