Bug 121100

Summary: [Windows] Revise GDI Create Functions to use GDIObject Smart Pointer
Product: WebKit Reporter: Brent Fulgham <bfulgham>
Component: WebKit Misc.Assignee: Brent Fulgham <bfulgham>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, bfulgham, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 120778    
Bug Blocks:    
Attachments:
Description Flags
Patch andersca: review+

Description Brent Fulgham 2013-09-10 09:58:48 PDT 
Revise various Windows create functions to use GDIObject smart pointer.
Comment 1 Brent Fulgham 2013-09-10 14:40:23 PDT 
1. Change FontPlatformData to take a GDIObject<HFONT> and use std::move.
2. 'createGDIFont' should return a GDIObject<HFONT> - as should any other create functions we have that return GDI objects.
Comment 2 Radar WebKit Bug Importer 2013-09-10 14:43:05 PDT 
<rdar://problem/14958562>
Comment 3 Brent Fulgham 2013-09-11 12:42:36 PDT 
Created attachment 211341 [details]
Patch
Comment 4 Anders Carlsson 2013-09-11 12:47:13 PDT 
Comment on attachment 211341 [details]
Patch

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

> Source/WebCore/platform/graphics/win/FontCacheWin.cpp:491
> +        return GDIObject<HFONT>();

If you add a GDIObject constructor that takes std::nullptr_t you can just return nullptr here.

> Source/WebCore/platform/graphics/win/FontPlatformDataWin.cpp:58
> +    ::SelectObject(hdc, m_font.get()->get());

I think SharedGDIObject’s get function should just return T, then this would just be m_font.get().

> Source/WebCore/platform/graphics/win/FontPlatformDataWin.cpp:69
> +        platformDataInit(m_font.get()->get(), size, hdc, faceName);

Same thing here.
Comment 5 Brent Fulgham 2013-09-11 13:18:55 PDT 
Comment on attachment 211341 [details]
Patch

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

>> Source/WebCore/platform/graphics/win/FontCacheWin.cpp:491
>> +        return GDIObject<HFONT>();
> 
> If you add a GDIObject constructor that takes std::nullptr_t you can just return nullptr here.

Great!  I'll do that.

>> Source/WebCore/platform/graphics/win/FontPlatformDataWin.cpp:58
>> +    ::SelectObject(hdc, m_font.get()->get());
> 
> I think SharedGDIObject’s get function should just return T, then this would just be m_font.get().

m_font is a RefPtr; SharedGDIObject.get() already returns T.  I should have written this as ::SelectObject(hdc, m_font->get());

>> Source/WebCore/platform/graphics/win/FontPlatformDataWin.cpp:69
>> +        platformDataInit(m_font.get()->get(), size, hdc, faceName);
> 
> Same thing here.

OK!
Comment 6 Brent Fulgham 2013-09-11 13:30:49 PDT 
Committed r155557: <http://trac.webkit.org/changeset/155557>