Bug 15397 – Layout tests freeze in HashTable::lookup

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Alexey Proskuryakov]

The below reliably freezes for me:

$ run-webkit-tests svg/W3C-SVG-1.1/text-align-06-b.svg --pixel

The problem is in FontPlatformData constructor: "sending to nil" Cocoa magic doesn't work with non-integral values:

    if (f)
        CFRetain(f);
    m_size = [f pointSize];

This results in emptyValue being non-zero, and deeply confuses WTF::HashTable.

[Alexey Proskuryakov]

Created attachment 16567 [details]
proposed fix

[Alexey Proskuryakov]

Looks like this issue isn't present on trunk - at least, I couldn't find such code there.

[Eric Seidel (no email)]

Comment on attachment 16567 [details]
proposed fix

The change looks sane. I don't understand the removal of the -1 checks (or why they were there in the first place).

[Alexey Proskuryakov]

I have removed the -1 checks because there was an unchecked call to [font pointSize] in this function anyway - so guarding against the same problem in other places wasn't really helpful.

[Darin Adler]

Comment on attachment 16567 [details]
proposed fix

If the checks for -1 really aren't necessary, then perhaps m_font can be changed into a RetainPtr?

r=me

[Alexey Proskuryakov]

> If the checks for -1 really aren't necessary, then perhaps m_font can be
> changed into a RetainPtr?

It's OK for m_font to be -1 (it's a special value used for "Deleted"), but apparently not for setFont() parameter.

[Alexey Proskuryakov]

Committed revision 26105.

[Dave Hyatt]

Thanks for catching this.

[Alexey Proskuryakov]

(In reply to comment #6)
> It's OK for m_font to be -1 (it's a special value used for "Deleted"), but
> apparently not for setFont() parameter.

Oops, one of the removed checks was necessary in fact:
-    if (m_font && m_font != (NSFont*)-1)
+    if (m_font)

Restored it in r26107.