Title: [96122] trunk
- Revision
- 96122
- Author
- jchaffr...@webkit.org
- Date
- 2011-09-27 09:55:59 -0700 (Tue, 27 Sep 2011)
Log Message
Crash because CSSPrimitiveValue::computeLengthDouble assumes fontMetrics are available
https://bugs.webkit.org/show_bug.cgi?id=66291
Reviewed by Darin Adler.
Source/WebCore:
Test: fast/canvas/crash-set-font.html
This is Yet Another Missing updateFont (similar to bug 57756 and likely others). Here the issue is that
applying one of the font properties could mutate the parent style's font if m_parentStyle == m_style.
We would then query the newly created font when applying CSSPropertyFontSize, which has no font fallback
list as Font::update was never called.
The right fix would be to refactor of how we handle fonts to avoid such manual updates (see bug 62390).
Until this happens, it is better not to crash.
* css/CSSStyleSelector.cpp:
(WebCore::CSSStyleSelector::applyProperty): Added updateFont() here as the fonts could have been
mutated by the previous property change. Also added a comment explaining why it is safe to do it
this way.
LayoutTests:
* fast/canvas/crash-set-font-expected.txt: Added.
* fast/canvas/crash-set-font.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (96121 => 96122)
--- trunk/LayoutTests/ChangeLog 2011-09-27 16:40:03 UTC (rev 96121)
+++ trunk/LayoutTests/ChangeLog 2011-09-27 16:55:59 UTC (rev 96122)
@@ -1,3 +1,13 @@
+2011-09-27 Julien Chaffraix <jchaffr...@webkit.org>
+
+ Crash because CSSPrimitiveValue::computeLengthDouble assumes fontMetrics are available
+ https://bugs.webkit.org/show_bug.cgi?id=66291
+
+ Reviewed by Darin Adler.
+
+ * fast/canvas/crash-set-font-expected.txt: Added.
+ * fast/canvas/crash-set-font.html: Added.
+
2011-09-27 Ilya Tikhonovsky <loi...@chromium.org>
Unreviewed followupfix for r96110.
Added: trunk/LayoutTests/fast/canvas/crash-set-font-expected.txt (0 => 96122)
--- trunk/LayoutTests/fast/canvas/crash-set-font-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/canvas/crash-set-font-expected.txt 2011-09-27 16:55:59 UTC (rev 96122)
@@ -0,0 +1,3 @@
+Test for bug 66291: Crash because CSSPrimitiveValue::computeLengthDouble assumes fontMetrics are available
+
+This test passed as it did not crash.
Added: trunk/LayoutTests/fast/canvas/crash-set-font.html (0 => 96122)
--- trunk/LayoutTests/fast/canvas/crash-set-font.html (rev 0)
+++ trunk/LayoutTests/fast/canvas/crash-set-font.html 2011-09-27 16:55:59 UTC (rev 96122)
@@ -0,0 +1,16 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <script>
+ if (window.layoutTestController)
+ layoutTestController.dumpAsText();
+
+ oContext2d = document.getCSSCanvasContext("2d","",0);
+ oContext2d.font = "small-caps .0ex G";
+ oContext2d.font = "italic .0ex G";
+ oContext2d.font = "italic 400 .0ex G";
+ </script>
+</head>
+<body>
+<p>Test for bug <a href="" Crash because CSSPrimitiveValue::computeLengthDouble assumes fontMetrics are available</p>
+<p>This test passed as it did not crash.</p>
Modified: trunk/Source/WebCore/ChangeLog (96121 => 96122)
--- trunk/Source/WebCore/ChangeLog 2011-09-27 16:40:03 UTC (rev 96121)
+++ trunk/Source/WebCore/ChangeLog 2011-09-27 16:55:59 UTC (rev 96122)
@@ -1,3 +1,25 @@
+2011-09-27 Julien Chaffraix <jchaffr...@webkit.org>
+
+ Crash because CSSPrimitiveValue::computeLengthDouble assumes fontMetrics are available
+ https://bugs.webkit.org/show_bug.cgi?id=66291
+
+ Reviewed by Darin Adler.
+
+ Test: fast/canvas/crash-set-font.html
+
+ This is Yet Another Missing updateFont (similar to bug 57756 and likely others). Here the issue is that
+ applying one of the font properties could mutate the parent style's font if m_parentStyle == m_style.
+ We would then query the newly created font when applying CSSPropertyFontSize, which has no font fallback
+ list as Font::update was never called.
+
+ The right fix would be to refactor of how we handle fonts to avoid such manual updates (see bug 62390).
+ Until this happens, it is better not to crash.
+
+ * css/CSSStyleSelector.cpp:
+ (WebCore::CSSStyleSelector::applyProperty): Added updateFont() here as the fonts could have been
+ mutated by the previous property change. Also added a comment explaining why it is safe to do it
+ this way.
+
2011-09-27 No'am Rosenthal <noam.rosent...@nokia.com>
[Texmap] Code cleanup: remove unused boundingRect/visibleRect calculations
Modified: trunk/Source/WebCore/css/CSSStyleSelector.cpp (96121 => 96122)
--- trunk/Source/WebCore/css/CSSStyleSelector.cpp 2011-09-27 16:40:03 UTC (rev 96121)
+++ trunk/Source/WebCore/css/CSSStyleSelector.cpp 2011-09-27 16:55:59 UTC (rev 96122)
@@ -3029,6 +3029,11 @@
applyProperty(CSSPropertyFontStyle, font->style.get());
applyProperty(CSSPropertyFontVariant, font->variant.get());
applyProperty(CSSPropertyFontWeight, font->weight.get());
+ // The previous properties can dirty our font but they don't try to read the font's
+ // properties back, which is safe. However if font-size is using the 'ex' unit, it will
+ // need query the dirtied font's x-height to get the computed size. To be safe in this
+ // case, let's just update the font now.
+ updateFont();
applyProperty(CSSPropertyFontSize, font->size.get());
m_lineHeightValue = font->lineHeight.get();
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes
