Bug 37952

Summary: Create a template for reference-counted Windows GDI handles.
Product: WebKit Reporter: Andy Estes <aestes>
Component: WebCore Misc.Assignee: Andy Estes <aestes>
Status: RESOLVED FIXED    
Severity: Enhancement CC: aroben, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Windows XP   
Bug Depends on:    
Bug Blocks: 37954    
Attachments:
Description Flags
patch darin: review+

Description Andy Estes 2010-04-21 14:47:51 PDT
There currently exists a class called RefCountedHFONT, which wraps a GDI HFONT in a reference-counted container.  There is a need to reference count other GDI handles in the Windows port, so it would be useful to have a generic class that can reference count any GDI handle.
Comment 1 Andy Estes 2010-04-21 16:28:26 PDT
Created attachment 54003 [details]
patch
Comment 2 WebKit Review Bot 2010-04-21 16:30:40 PDT
Attachment 54003 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebCore/platform/graphics/cairo/FontPlatformData.h:41:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Darin Adler 2010-04-21 17:49:24 PDT
Comment on attachment 54003 [details]
patch

This is OK as is, but there is one thing we should improve. The hash function here should be:

    return PtrHash<T>(m_handle);

Once you change it to work that way you can remove the include of StringImpl.h.

If this wasn't all inlined, I would also suggest making a base class, since the only functions in RefCountedGDIHandle that are different based on type are the create() function, the constructor, and the handle() function. But since everything is inlined anyway, I don't think there's any real benefit to doing so.
Comment 4 Andy Estes 2010-04-21 20:19:54 PDT
Committed revision 58045.
Comment 5 Adam Roben (:aroben) 2010-04-22 08:00:30 PDT
Using OwnPtr in the implementation of your new class template would have made things even simpler, and would allow this class template to be used for non-GDI objects.