Bug 76750

Summary: [windows] Convert usage of GetDC to HWndDC Part 2.
Product: WebKit Reporter: David Levin <levin>
Component: PlatformAssignee: David Levin <levin>
Status: RESOLVED FIXED    
Severity: Normal CC: aroben, ddkilzer, dslomov, levin+threading, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 76303    
Attachments:
Description Flags
Patch aroben: review+, aroben: commit-queue-

Description David Levin 2012-01-20 16:10:39 PST
These usages are where I changed (usually buggy) functionality. 

Usually these were leaks. In one case, I reduced the lifetime of a dc because it allowed for simpler code (when using an RAII class like HWndDC). In another case, the code was using OwnPtr<HDC> when it should not have been so the HDC was getting deleted when it should have been released.
Comment 1 David Levin 2012-01-20 16:16:54 PST
Created attachment 123402 [details]
Patch
Comment 2 WebKit Review Bot 2012-01-20 16:19:10 PST
Attachment 123402 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebKit/win/WebNodeHighlight.cpp:138:  Use OwnPtr<HDC> when calling CreateCompatibleDC to avoid potential memory leaks.  [runtime/leaky_pattern] [5]
Source/WebKit/win/FullscreenVideoController.cpp:489:  Use OwnPtr<HDC> when calling CreateCompatibleDC to avoid potential memory leaks.  [runtime/leaky_pattern] [5]
Source/WebKit2/Shared/win/ShareableBitmapWin.cpp:46:  Use OwnPtr<HDC> when calling CreateCompatibleDC to avoid potential memory leaks.  [runtime/leaky_pattern] [5]
Source/WebKit/chromium/ChangeLog:3:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
Total errors found: 4 in 31 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 David Levin 2012-01-20 16:29:51 PST
(In reply to comment #2)
> Attachment 123402 [details] did not pass style-queue:
> 
> Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
> 
> Source/WebKit/win/WebNodeHighlight.cpp:138:  Use OwnPtr<HDC> when calling CreateCompatibleDC to avoid potential memory leaks.  [runtime/leaky_pattern] [5]
> Source/WebKit/win/FullscreenVideoController.cpp:489:  Use OwnPtr<HDC> when calling CreateCompatibleDC to avoid potential memory leaks.  [runtime/leaky_pattern] [5]
> Source/WebKit2/Shared/win/ShareableBitmapWin.cpp:46:  Use OwnPtr<HDC> when calling CreateCompatibleDC to avoid potential memory leaks.  [runtime/leaky_pattern] [5]
> Source/WebKit/chromium/ChangeLog:3:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
> Total errors found: 4 in 31 files
> 
> 
> If any of these errors are false positives, please file a bug against check-webkit-style.

Filed: https://bugs.webkit.org/show_bug.cgi?id=76752
Comment 4 Adam Roben (:aroben) 2012-01-23 08:30:30 PST
Comment on attachment 123402 [details]
Patch

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

> Source/WebKit/win/FullscreenVideoController.cpp:494
> -    HDC windowDC = GetDC(m_hudWindow);
> -    HDC bitmapDC = CreateCompatibleDC(windowDC);
> -    ::ReleaseDC(m_hudWindow, windowDC);
> -    HGDIOBJ oldBitmap = SelectObject(bitmapDC, m_bitmap.get());
> +    OwnPtr<HDC> bitmapDC = createCompatibleDCForWindow(m_hudWindow);

I don't think the separate function is needed. You can just do: OwnPtr<HDC> bitmapDC = CreateCompatibleDC(HwndDC(m_hudWindow));
Comment 5 David Levin 2012-01-23 17:32:01 PST
Committed as http://trac.webkit.org/changeset/105667

(In reply to comment #4)
> (From update of attachment 123402 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=123402&action=review
> > +    OwnPtr<HDC> bitmapDC = createCompatibleDCForWindow(m_hudWindow);
> 
> I don't think the separate function is needed. You can just do: OwnPtr<HDC> bitmapDC = CreateCompatibleDC(HwndDC(m_hudWindow));

Done.