Bug 141726

Summary: Access FontCache global instance via singleton() static member function
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: WebCore Misc.Assignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, dbates, kling, koivisto, rniwa
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Chris Dumez 2015-02-17 14:40:51 PST 
Access FontCache global instance via singleton() static member function, as per coding style.
Comment 1 Chris Dumez 2015-02-17 15:54:21 PST 
Created attachment 246773 [details]
Patch
Comment 2 Chris Dumez 2015-02-17 16:44:45 PST 
Created attachment 246780 [details]
Patch
Comment 3 Chris Dumez 2015-02-17 16:53:36 PST 
Created attachment 246781 [details]
Patch
Comment 4 Daniel Bates 2015-02-17 21:15:27 PST 
Comment on attachment 246781 [details]
Patch

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

We also need to update the Symbol Ordering Flags file Source/WebCore/WebCore.order. Did you intend to omit the list of changed functions from each ChangeLog entry?

> Source/WebCore/platform/graphics/FontGlyphs.cpp:40
> +    : m_cachedPrimaryFont(nullptr)

Would it make sense to use in-class initialization for this instance variable?

> Source/WebCore/platform/graphics/FontGlyphs.cpp:48
> +    : m_cachedPrimaryFont(nullptr)

Ditto.
Comment 5 Chris Dumez 2015-02-17 21:18:06 PST 
Comment on attachment 246781 [details]
Patch

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

>> Source/WebCore/platform/graphics/FontGlyphs.cpp:40
>> +    : m_cachedPrimaryFont(nullptr)
> 
> Would it make sense to use in-class initialization for this instance variable?

I don't know what's the policy for this. I like in-class initialization when this means we can get rid of the explicit constructor. However, if we need an explicit constructor, using also in-class initialization means we initialize members in 2 places.
Comment 6 Chris Dumez 2015-02-17 21:20:31 PST 
(In reply to comment #4)
> Comment on attachment 246781 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=246781&action=review
> 
> We also need to update the Symbol Ordering Flags file
> Source/WebCore/WebCore.order.

Is this some very recent policy change? I was told not long ago that I should not update that file.

> Did you intend to omit the list of changed
> functions from each ChangeLog entry?

Yes, I was told keeping them makes little sense unless you add per method changelogs.
Comment 7 WebKit Commit Bot 2015-02-18 12:05:24 PST 
Comment on attachment 246781 [details]
Patch

Clearing flags on attachment: 246781

Committed r180281: <http://trac.webkit.org/changeset/180281>
Comment 8 WebKit Commit Bot 2015-02-18 12:05:34 PST 
All reviewed patches have been landed.  Closing bug.