910376 - Should FontTableHashEntry copy mSharedBlobData and/or initialize mSharedBlobData to null?

Status: RESOLVED FIXED

(Core :: Graphics: Text, defect)

Description

[Justin Lebar (not reading bugmail)]

I'd normally ask about this on IRC, but the relevant people don't seem to be around at the moment.

I noticed that FontTableHashEntry's copy constructor doesn't copy mSharedBlobData.

This seems odd to me, because it means that if the hashtable is mutated, mSharedBlobData will be undefined.

Similarly, mSharedBlobData is not initialized to null in the default FontTableHashEntry constructor, so I don't see how the |if (mSharedBlobData)| check in gfxFontEntry::FontTableHashEntry::Clear() and in SizeOfEntryExcludingThis() is sensible.

OTOH this code is doing something clever with the hashtable that I don't grok, so maybe this is all right.

Comment 1

[Jonathan Kew [:jfkthame]]

Attachment: don't risk leaving mSharedBlobData uninitialized.

Thanks, Justin - this sure looks suspicious to me. I wonder if it might lie behind some of the unexplained font-related crashes we've seen over the past months, in bugs such as 875878 and 894798.

Comment 2

[Jonathan Kew [:jfkthame]]

Attachment: don't risk leaving mSharedBlobData uninitialized.

Oops... initializers were in the wrong order. Fixed that.

Comment 3

[Jonathan Kew [:jfkthame]]

Rebased and pushed:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f88c529008d0

Comment 4

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/f88c529008d0