| Summary: | Allow delayed DC allocation in HWndDC. | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | David Levin <levin> | ||||
| Component: | Platform | Assignee: | David Levin <levin> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | aroben, ddkilzer, dimich, dslomov, webkit.review.bot | ||||
| Priority: | P2 | ||||||
| Version: | 528+ (Nightly build) | ||||||
| Hardware: | PC | ||||||
| OS: | Windows Vista | ||||||
| Bug Depends on: | |||||||
| Bug Blocks: | 76303, 76889 | ||||||
| Attachments: |
|
||||||
|
Description
David Levin
2012-01-20 13:59:49 PST
Created attachment 123375 [details]
Patch
I'll gladly accept a review from anyone who will provide one. Attachment 123375 [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/WebCore/platform/win/HWndDC.h:64: Use the class HWndDC instead of calling GetDC to avoid potential memory leaks. [runtime/leaky_pattern] [5]
Total errors found: 1 in 2 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 123375 [details]
Patch
Looks good to me. Any usages in the code?
(In reply to comment #4) > (From update of attachment 123375 [details]) > Looks good to me. Any usages in the code? See the last patch in https://bugs.webkit.org/show_bug.cgi?id=76303 I was asked to make it into smaller patches so this is part of my attempt to do so. Comment on attachment 123375 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=123375&action=review > Source/WebCore/platform/win/HWndDC.h:37 > + explicit HWndDC() "explicit" is only needed for constructs that have one (and only one) required parameter. > Source/WebCore/platform/win/HWndDC.h:60 > + HDC changeWindowDC(HWND hwnd) "changeWindowDC" sounds like a function that takes an HDC, not an HWND. Maybe "setHwnd" would be a clearer name? > Source/WebCore/platform/win/HWndDC.h:68 > + void clearDC() I think this could just be called "clear". It would be nice if "myHwndDC = nullptr" did the same thing as "myHwndDC.clear()", to match classes like RefPtr and OwnPtr. (In reply to comment #6) > (From update of attachment 123375 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=123375&action=review > > > Source/WebCore/platform/win/HWndDC.h:68 > > + void clearDC() > > I think this could just be called "clear". It would be nice if "myHwndDC = nullptr" did the same thing as "myHwndDC.clear()", to match classes like RefPtr and OwnPtr. My concern with this is that I don't know what the operator= should do if it isn't a nullptr. Maybe I could make it only work for nullptr.... I'll have to try to figure that out. Committed as http://trac.webkit.org/changeset/105656 Addressed everything except I didn't add the operator= for now. It wouldn't be used anywhere in my current set of changes anyway. We could add it later if it becomes important. |