Changeset 117788 in webkit

Timestamp:

May 21, 2012 9:06:50 AM (12 years ago)

Author:

schenney@chromium.org

Message:

SVGTextRunRenderingContext can return null font, calling code asserts not null
​https://bugs.webkit.org/show_bug.cgi?id=86738

Reviewed by Nikolas Zimmermann.

SVGTextRunRenderingContext::glyphDataForCharacter was returning a glyph with
null font data for numerous code paths. It seems that it was doing so
whenever it detected null fontData, rather than try to continue.
Calling code would then immediately assert on this null fontData.

This patch refactors SVGTextRunRenderingContext::glyphDataForCharacter
so that it never returns null font data, adding an assertion to that
effect. In particular, when the font data is null the code will reach
the fallback glyph calculations.

Refactoring covered by existing tests. A previously crashing test, svg/custom/acid3-test-77.html, no longer crashes.

• rendering/svg/SVGTextRunRenderingContext.cpp:

(WebCore::SVGTextRunRenderingContext::glyphDataForCharacter):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• rendering/svg/SVGTextRunRenderingContext.cpp (modified) (3 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-05-21  Stephen Chenney  <schenney@chromium.org>
+
+        SVGTextRunRenderingContext can return null font, calling code asserts not null
+        https://bugs.webkit.org/show_bug.cgi?id=86738
+
+        Reviewed by Nikolas Zimmermann.
+
+        SVGTextRunRenderingContext::glyphDataForCharacter was returning a glyph with
+        null font data for numerous code paths. It seems that it was doing so
+        whenever it detected null fontData, rather than try to continue.
+        Calling code would then immediately assert on this null fontData.
+
+        This patch refactors SVGTextRunRenderingContext::glyphDataForCharacter
+        so that it never returns null font data, adding an assertion to that
+        effect. In particular, when the font data is null the code will reach
+        the fallback glyph calculations.
+
+        Refactoring covered by existing tests. A previously crashing test, svg/custom/acid3-test-77.html, no longer crashes.
+
+        * rendering/svg/SVGTextRunRenderingContext.cpp:
+        (WebCore::SVGTextRunRenderingContext::glyphDataForCharacter):
+
 2012-05-21  Ilya Tikhonovsky  <loislo@chromium.org>
 

trunk/Source/WebCore/rendering/svg/SVGTextRunRenderingContext.cpp

@@ -180 +180 @@
     pair<GlyphData, GlyphPage*> pair = font.glyphDataAndPageForCharacter(character, mirror);
     GlyphData glyphData = pair.first;
-    if (!glyphData.fontData)
+
+    // Check if we have the missing glyph data, in which case we can just return.
+    GlyphData missingGlyphData = primaryFont->missingGlyphData();
+    if (glyphData.glyph == missingGlyphData.glyph && glyphData.fontData == missingGlyphData.fontData) {
+        ASSERT(glyphData.fontData);
         return glyphData;
-
-    GlyphData missingGlyphData = primaryFont->missingGlyphData();
-    if (glyphData.glyph == missingGlyphData.glyph && glyphData.fontData == missingGlyphData.fontData)
-        return glyphData;
+    }
 
     // Characters enclosed by an <altGlyph> element, may not be registered in the GlyphPage.
-    if (!glyphData.fontData->isSVGFont()) {
+    if (glyphData.fontData && !glyphData.fontData->isSVGFont()) {
         if (TextRun::RenderingContext* renderingContext = run.renderingContext()) {
             RenderObject* renderObject = static_cast<SVGTextRunRenderingContext*>(renderingContext)->renderer();
@@ -200 +201 @@
     }
 
-    if (!glyphData.fontData || !glyphData.fontData->isSVGFont())
-        return glyphData;
-
     const SimpleFontData* fontData = glyphData.fontData;
-
-    SVGFontElement* fontElement = 0;
-    SVGFontFaceElement* fontFaceElement = 0;
-
-    const SVGFontData* svgFontData = svgFontAndFontFaceElementForFontData(fontData, fontFaceElement, fontElement);
-    if (!fontElement || !fontFaceElement)
-        return glyphData;
-
-    // If we got here, we're dealing with a glyph defined in a SVG Font.
-    // The returned glyph by glyphDataAndPageForCharacter() is a glyph stored in the SVG Font glyph table.
-    // This doesn't necessarily mean the glyph is suitable for rendering/measuring in this context, its
-    // arabic-form/orientation/... may not match, we have to apply SVG Glyph selection to discover that.
-    if (svgFontData->applySVGGlyphSelection(iterator, glyphData, mirror, currentCharacter, advanceLength))
-        return glyphData;
+    if (fontData) {
+        if (!fontData->isSVGFont())
+            return glyphData;
+
+        SVGFontElement* fontElement = 0;
+        SVGFontFaceElement* fontFaceElement = 0;
+
+        const SVGFontData* svgFontData = svgFontAndFontFaceElementForFontData(fontData, fontFaceElement, fontElement);
+        if (!fontElement || !fontFaceElement)
+            return glyphData;
+
+        // If we got here, we're dealing with a glyph defined in a SVG Font.
+        // The returned glyph by glyphDataAndPageForCharacter() is a glyph stored in the SVG Font glyph table.
+        // This doesn't necessarily mean the glyph is suitable for rendering/measuring in this context, its
+        // arabic-form/orientation/... may not match, we have to apply SVG Glyph selection to discover that.
+        if (svgFontData->applySVGGlyphSelection(iterator, glyphData, mirror, currentCharacter, advanceLength))
+            return glyphData;
+    }
 
     GlyphPage* page = pair.second;
@@ -241 +243 @@
     fontList->setGlyphPageZero(originalGlyphPageZero);
     fontList->setGlyphPages(originalGlyphPages);
+    ASSERT(fallbackGlyphData.fontData);
     return fallbackGlyphData;
 }