When an SVG font is used in a text run, and "text-rendering: optimizelegibility;" is used (or there is kerning or ligature information on the font), and the text is selected, we crash. This is a linux only bug, I believe, or at least HarfBuzz backed by Skia only. There are two problems: - An assert not reached in SimpleFontData::applyTransform that is too assertive. SVG fonts may come through this path and it is not an error for them to do so. - The code in HarfBuzzShaper::shapeHarfBuzzRuns assumes it is always dealing with HarfBuxx font data, which is not the case for SVG fonts. Patch shortly to prevent the crash. We still do not select properly in such cases, or on other platforms.
Created attachment 188353 [details] Patch
Comment on attachment 188353 [details] Patch Attachment 188353 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/16538665 New failing tests: svg/css/font-face-crash.html
Comment on attachment 188353 [details] Patch Attachment 188353 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/16542657 New failing tests: svg/css/font-face-crash.html
Created attachment 188382 [details] Patch
Comment on attachment 188382 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=188382&action=review > LayoutTests/svg/css/font-face-crash.html:17 > + if (window.testRunner) > + testRunner.waitUntilDone(); Can we add testRunner.dumpAsText() and share the results?
(In reply to comment #5) > (From update of attachment 188382 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=188382&action=review > > > LayoutTests/svg/css/font-face-crash.html:17 > > + if (window.testRunner) > > + testRunner.waitUntilDone(); > > Can we add testRunner.dumpAsText() and share the results? It only fails if we paint (trying to paint the selection rect) and I don't think we can construct a reliable ref-test due to fonts and the selection rect (when we end up having the right selection rect).
(In reply to comment #6) > (In reply to comment #5) > > (From update of attachment 188382 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=188382&action=review > > > > > LayoutTests/svg/css/font-face-crash.html:17 > > > + if (window.testRunner) > > > + testRunner.waitUntilDone(); > > > > Can we add testRunner.dumpAsText() and share the results? > > It only fails if we paint (trying to paint the selection rect) and I don't think we can construct a reliable ref-test due to fonts and the selection rect (when we end up having the right selection rect). And we need the timeout too.
dumpAsText() tests still paint, we just don't compare the pixels when we're done. I think your test will still crash with dumpAsText(). Also, you can use dumpAsText and waitUntilDone together.
(In reply to comment #8) > dumpAsText() tests still paint, we just don't compare the pixels when we're done. I think your test will still crash with dumpAsText(). I verified locally on my Linux machine that we still crash on this test (release build) with dumpAsText() and without the code change.
Created attachment 188435 [details] Patch It does paint after all. I must have had some other issue when it was earlier faiiing to crash.
Comment on attachment 188435 [details] Patch Clearing flags on attachment: 188435 Committed r142928: <http://trac.webkit.org/changeset/142928>
All reviewed patches have been landed. Closing bug.