[Webkit-unassigned] [Bug 116783] Use Vector instead of custom linked list for font families

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=116783





--- Comment #19 from Antti Koivisto <koivisto at iki.fi>  2013-05-26 17:53:26 PST ---
(In reply to comment #18)
> (From update of attachment 202920 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=202920&action=review
> 
> > Source/WebCore/platform/graphics/FontDescription.h:144
> > +    void setOneFamily(const AtomicString& family) { ASSERT(m_families.size() == 1); m_families[0] = family; }
> 
> I am a little confused by this. How do callers know the families list already happens to be one family long in all the cases where this is used?

FontDescription is not meant to be mutated after the initial setup. The size is set to one in constructor to match the old behaviour and limit the refactoring. This could actually assert that the family is null too.

> The name “set one family” also seems a little confusing to me. It sounds like this sets one of the families and leaves the rest alone. I would have maybe called this setSingleFamily. But really I am mystified by the way the code can assume there is only one family.

Yeah, setSingleFamily would be a better name. Removing the whole thing in favour of constructor parameter would be better.

> The old thing was two pointers. Isn’t Vector<AtomicString, 1> much bigger than that?

This is not meant to be the end state. A simplification step will hopefully be helpful to get bigger gains.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.