Changeset 96122 in webkit

Timestamp:

Sep 27, 2011 9:55:59 AM (13 years ago)

Author:

jchaffraix@webkit.org

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.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/canvas/crash-set-font-expected.txt (added)
• LayoutTests/fast/canvas/crash-set-font.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/css/CSSStyleSelector.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2011-09-27  Julien Chaffraix  <jchaffraix@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  <loislo@chromium.org>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2011-09-27  Julien Chaffraix  <jchaffraix@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.rosenthal@nokia.com>
 

trunk/Source/WebCore/css/CSSStyleSelector.cpp

@@ -3030 +3030 @@
             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());