Bug 139836

Summary: [SVG -> OTF Converter] Make placeholders more robust
Product: WebKit Reporter: Myles C. Maxfield <mmaxfield>
Component: New BugsAssignee: Myles C. Maxfield <mmaxfield>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, dino, d-r, fmalita, gyuyoung.kim, jonlee, mitz, pdr, schenney, sergio, simon.fraser, thorton
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch mitz: review+

Description Myles C. Maxfield 2014-12-19 15:06:03 PST
[SVG -> OTF Converter] Make placeholders more robust
Comment 1 Myles C. Maxfield 2014-12-19 15:07:13 PST
Created attachment 243580 [details]
Patch
Comment 2 mitz 2014-12-19 15:35:13 PST
Comment on attachment 243580 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=243580&action=review

> Source/WebCore/svg/SVGToOTFFontConversion.cpp:93
> +        {

Perhaps ASSERT(!m_written) here?

> Source/WebCore/svg/SVGToOTFFontConversion.cpp:614
> +    Placeholder toCoverageTable(*this, subtableLocation);

I think it would have been a little nicer if SVGToOTFFontConverter had a member function that returned a placeholder (and then the Placeholder constructor could be private and friend with that member function), so here you’d just say
auto toCoverageTable = appendPlaceholder(subtableLocation);
Comment 3 Myles C. Maxfield 2014-12-19 20:28:40 PST
Comment on attachment 243580 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=243580&action=review

>> Source/WebCore/svg/SVGToOTFFontConversion.cpp:93
>> +        {
> 
> Perhaps ASSERT(!m_written) here?

Done.

>> Source/WebCore/svg/SVGToOTFFontConversion.cpp:614
>> +    Placeholder toCoverageTable(*this, subtableLocation);
> 
> I think it would have been a little nicer if SVGToOTFFontConverter had a member function that returned a placeholder (and then the Placeholder constructor could be private and friend with that member function), so here you’d just say
> auto toCoverageTable = appendPlaceholder(subtableLocation);

Done.
Comment 4 Myles C. Maxfield 2014-12-19 20:42:11 PST
Committed r177620: <http://trac.webkit.org/changeset/177620>