Inspired by how long it took me to take down: https://bugs.webkit.org/show_bug.cgi?id=76203
Created attachment 122340 [details] Patch
Comment on attachment 122340 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=122340&action=review Looks good, 2 minor nits. > Tools/Scripts/webkitpy/style/checkers/cpp.py:1679 > + 'Use OwnGetDC instead of GetDC to avoid potential memory leaks.') I think OwnGetDC should be called DCRef, similar to RefPtr > Tools/Scripts/webkitpy/style/checkers/cpp.py:1685 > + 'Use OwnPtr<HDC> when calling CreateDC to avoid potential memory leaks.') Maybe call out CreateCompatibleDC as well in the error message
Created attachment 122351 [details] Patch
Comment on attachment 122351 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=122351&action=review > Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py:3357 > + 'Use the class HwndDC instead of calling GetDCEx to avoid potential ' Is there a constructor on HwndDC to match GetDCEx signature? Maybe better to have HwndDC::Get and HwndDC::GetEx instead of constructors?
Created attachment 122378 [details] Patch
(In reply to comment #4) > (From update of attachment 122351 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=122351&action=review > > > Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py:3357 > > + 'Use the class HwndDC instead of calling GetDCEx to avoid potential ' > > Is there a constructor on HwndDC to match GetDCEx signature? Done. > Maybe better to have HwndDC::Get and HwndDC::GetEx instead of constructors? I see what you mean but I think that makes it harder to put on the stack and we're in luck because the parameter order is the same for GetDC and GetDCEx so it fits the typical overload pattern. So I stayed with the constructors as discussed.
Attachment 122378 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1 Source/JavaScriptCore/wtf/win/HwndDCWin.h:40: Use the class HwndDC instead of calling GetDCEx to avoid potential memory leaks. [runtime/leaky_pattern] [5] Total errors found: 1 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 122379 [details] Patch
Attachment 122379 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1 Source/JavaScriptCore/wtf/win/HwndDCWin.h:40: Use the class HwndDC instead of calling GetDCEx to avoid potential memory leaks. [runtime/leaky_pattern] [5] Total errors found: 1 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.
(In reply to comment #9) > Attachment 122379 [details] did not pass style-queue: > > Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1 > > Source/JavaScriptCore/wtf/win/HwndDCWin.h:40: Use the class HwndDC instead of calling GetDCEx to avoid potential memory leaks. [runtime/leaky_pattern] [5] > Total errors found: 1 in 5 files Actually this is expected. It is flagging HWndDC. I suppose I could make that one file exempt from this check but it doesn't seem worth it.
Created attachment 122456 [details] Patch
Attachment 122456 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1 Source/JavaScriptCore/wtf/win/HWndDCWin.h:40: Use the class HWndDC instead of calling GetDCEx to avoid potential memory leaks. [runtime/leaky_pattern] [5] Total errors found: 1 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 122456 [details] Patch Clearing flags on attachment: 122456 Committed r104995: <http://trac.webkit.org/changeset/104995>
All reviewed patches have been landed. Closing bug.