Title: [145928] branches/safari-536.30-branch

Diff

Modified: branches/safari-536.30-branch/LayoutTests/ChangeLog (145927 => 145928)


--- branches/safari-536.30-branch/LayoutTests/ChangeLog	2013-03-15 18:52:27 UTC (rev 145927)
+++ branches/safari-536.30-branch/LayoutTests/ChangeLog	2013-03-15 18:58:02 UTC (rev 145928)
@@ -1,3 +1,21 @@
+2013-03-15  Lucas Forschler  <lforsch...@apple.com>
+
+        Merge r130999
+
+    2012-10-10  Stephen Chenney  <schen...@chromium.org>
+
+            SVGTextRunRenderingContext changes font data in the glyph page, but it shouldn't
+            https://bugs.webkit.org/show_bug.cgi?id=98755
+
+            Reviewed by Eric Seidel.
+
+            New test case that includes an alt-glyph that comes from the system
+            fallback font (because the alt-glyph doesn't reference anything). This
+            test crashes on Chromium linux without the patch, and may crash on
+            other platforms too.
+
+            * svg/text/alt-glpyh-on-fallback-font-crash.html: Added.
+
 2013-03-12  Lucas Forschler  <lforsch...@apple.com>
 
         Merge r142657

Copied: branches/safari-536.30-branch/LayoutTests/svg/text/alt-glpyh-on-fallback-font-crash-expected.txt (from rev 130999, trunk/LayoutTests/svg/text/alt-glpyh-on-fallback-font-crash-expected.txt) (0 => 145928)


--- branches/safari-536.30-branch/LayoutTests/svg/text/alt-glpyh-on-fallback-font-crash-expected.txt	                        (rev 0)
+++ branches/safari-536.30-branch/LayoutTests/svg/text/alt-glpyh-on-fallback-font-crash-expected.txt	2013-03-15 18:58:02 UTC (rev 145928)
@@ -0,0 +1 @@
+A A: This test passes if you see two As at the start of this message.

Copied: branches/safari-536.30-branch/LayoutTests/svg/text/alt-glpyh-on-fallback-font-crash.svg (from rev 130999, trunk/LayoutTests/svg/text/alt-glpyh-on-fallback-font-crash.svg) (0 => 145928)


--- branches/safari-536.30-branch/LayoutTests/svg/text/alt-glpyh-on-fallback-font-crash.svg	                        (rev 0)
+++ branches/safari-536.30-branch/LayoutTests/svg/text/alt-glpyh-on-fallback-font-crash.svg	2013-03-15 18:58:02 UTC (rev 145928)
@@ -0,0 +1,23 @@
+<svg xmlns="http://www.w3.org/2000/svg">
+  <g>
+    <defs>
+      <font>
+        <font-face font-family="HappySad">
+        </font-face>
+      </font>
+    </defs>
+    <g font-family="HappySad" style="-webkit-writing-mode: vertical-rl; ">
+      <text>
+        <altGlyph>A</altGlyph>
+        A: This test passes if you see two As at the start of this message.
+      </text>
+    </g>
+  </g>
+  <script>
+    if (window.testRunner)
+      testRunner.dumpAsText();
+
+    var docElement = document.body ? document.body : document.documentElement;
+    if (docElement) docElement.offsetTop;
+  </script>
+</svg>

Modified: branches/safari-536.30-branch/Source/WebCore/ChangeLog (145927 => 145928)


--- branches/safari-536.30-branch/Source/WebCore/ChangeLog	2013-03-15 18:52:27 UTC (rev 145927)
+++ branches/safari-536.30-branch/Source/WebCore/ChangeLog	2013-03-15 18:58:02 UTC (rev 145928)
@@ -1,3 +1,43 @@
+2013-03-15  Lucas Forschler  <lforsch...@apple.com>
+
+        Merge r130999
+
+    2012-10-10  Stephen Chenney  <schen...@chromium.org>
+
+            SVGTextRunRenderingContext changes font data in the glyph page, but it shouldn't
+            https://bugs.webkit.org/show_bug.cgi?id=98755
+
+            Reviewed by Eric Seidel.
+
+            The code in SVGTextRunRenderingContext::glyphDataForCharacter, when it
+            encounters an <altglyph> tag, immediately replaces the font data for a
+            glyph with font data for the primary font, presumably to meet the SVG
+            spec requirement: "If the references to alternate glyphs do not result
+            in successful identification of alternate glyphs to use, then the
+            character(s) that are inside of the ‘altGlyph’ element are rendered as
+            if the ‘altGlyph’ element were a ‘tspan’ element instead."
+
+            If the alt glyph is not then found we are in the case from the spec
+            and indeed we should use the primary font. However, we end up replacing the GlyphPage
+            entry for the character with primary font data, which we should not do
+            because the glyph page might be used in some place that does not have
+            the alt glyph tag.
+
+            Furthermore, this causes object lifetime problems for font data, because
+            in cases where the font data that is replaced is for the system fallback
+            font the GlyphPage will live forever with no knowldege that it contains
+            font data pointers into font data other that the system fallback. The
+            replaced font data may be deleted while the pointer lives on in the
+            system fallback page.
+
+            The fix is simply not to replace the font data in the page.
+
+            Test: svg/text/alt-glpyh-on-fallback-font-crash.html
+
+            * rendering/svg/SVGTextRunRenderingContext.cpp:
+            (WebCore::SVGTextRunRenderingContext::glyphDataForCharacter): Keep track of the original font data and put it back
+            in the glyph page when the method has finished.
+
 2013-03-12  Lucas Forschler  <lforsch...@apple.com>
 
         Merge r142657

Modified: branches/safari-536.30-branch/Source/WebCore/rendering/svg/SVGTextRunRenderingContext.cpp (145927 => 145928)


--- branches/safari-536.30-branch/Source/WebCore/rendering/svg/SVGTextRunRenderingContext.cpp	2013-03-15 18:52:27 UTC (rev 145927)
+++ branches/safari-536.30-branch/Source/WebCore/rendering/svg/SVGTextRunRenderingContext.cpp	2013-03-15 18:58:02 UTC (rev 145928)
@@ -187,6 +187,7 @@
         return glyphData;
 
     // Characters enclosed by an <altGlyph> element, may not be registered in the GlyphPage.
+    const SimpleFontData* originalFontData = glyphData.fontData;
     if (!glyphData.fontData->isSVGFont()) {
         if (TextRun::RenderingContext* renderingContext = run.renderingContext()) {
             RenderObject* renderObject = static_cast<SVGTextRunRenderingContext*>(renderingContext)->renderer();
@@ -237,7 +238,7 @@
 
     // Restore original state of the SVG Font glyph table and the current font fallback list,
     // to assure the next lookup of the same glyph won't immediately return the fallback glyph.
-    page->setGlyphDataForCharacter(character, glyphData.glyph, fontData);
+    page->setGlyphDataForCharacter(character, glyphData.glyph, originalFontData);
     fontList->setGlyphPageZero(originalGlyphPageZero);
     fontList->setGlyphPages(originalGlyphPages);
     return fallbackGlyphData;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to