Bug 19542 – Crash in Font::glyphDataForCharacter when getting small caps data

• Component: Text
• Status: RESOLVED FIXED

[Brett Wilson (Google)]

In Font::glyphDataForCharacter there is this code

  GlyphPageTreeNode* smallCapsNode =
      GlyphPageTreeNode::getRootChild(smallCapsFontData, pageNumber);
  const GlyphData& data = smallCapsNode->page()->glyphDataForCharacter(c);

I got a crash report that shows page() returning NULL here, which caused a crash, although I do not have a repro. In GlyphPageTreeNode.h, it says:

// Returns a page of glyphs (or NULL if there are no glyphs in this page's character range).
GlyphPage* page() const { return m_page.get(); }

So it looks like this Font.cpp code is wrong. Other callers of page() in this function NULL check it, but not this small caps case. It looks like we just need to add a check here.

[Mark Rowe (bdash)]

<rdar://problem/6007976>

[Eric Seidel (no email)]

This should get the page cleared:

<script>
 var ifr = document.createElement('iframe');
 ifr.onload = function() {
   var win = ifr.contentWindow;
   // Remove, pow pageDestroyed.
   ifr.parentNode.removeChild(ifr);
   // Your Page is now cleared, do as you will to reproduce the rest of the bug!
 };
 document.body.appendChild(ifr);
</script>

probably setting some font, or something to get it to relayout/resize a form control in smallcaps should trigger it.

(In reply to comment #2)
> This should get the page cleared:

I think it's a different kind of page :)

[Eric Seidel (no email)]

Oh yeah.  You're right... :)

[Brett Wilson (Google)]

Created attachment 21842 [details]
Patch

This patch just adds a NULL check for the page() of glyphs like the rest of the file. If this fails, it does the same thing it would do if the GlyphData in the page is NULL.

I did not add a test. This patch is based on a crash report I saw. The stack is clear that the crash is dereferencing a NULL from the page() here, but I can not reproduce, even opening the page that triggered the crash report. I also tried to generate some small caps text in a funny language that wouldn't be in the font, but I could not trigger it. It is probably highly dependent on the WebKit port, OS, and installed fonts. If you have an idea for a test, I'll be happy to write it.

[Dave Hyatt]

Comment on attachment 21842 [details]
Patch

r=me

Landed in <http://trac.webkit.org/changeset/34717>.

[Brett Wilson (Google)]

Thanks mitz!