Based on feedback in https://bugs.webkit.org/show_bug.cgi?id=76227#c2 and subsequent discussion.
Created attachment 122349 [details] Patch
Comment on attachment 122349 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=122349&action=review Looks good. > Source/WebKit/chromium/src/win/WebScreenInfoFactory.cpp:65 > + HwndDC hdc(0); Probably makes sense to make a pass and replace more usages of ::GetDC with this new class.
Created attachment 122355 [details] Patch
Comment on attachment 122355 [details] Patch r=me, I understand there is a separate bug filed to follow up on Dmitry's suggestion to comb the files for other usages of ::GetDC()
Comment on attachment 122355 [details] Patch Clearing flags on attachment: 122355 Committed r104895: <http://trac.webkit.org/changeset/104895>
All reviewed patches have been landed. Closing bug.
Since HWND is “handle to window”, I think it should be HWnd, when things are mixed case, not Hwnd.
(In reply to comment #7) > Since HWND is “handle to window”, I think it should be HWnd, when things are mixed case, not Hwnd. Patch: https://bugs.webkit.org/show_bug.cgi?id=76281
I have two concerns with this patch: 1) It is missing a header guard. 2) I don't think WTF is the right layer for it. It might fit nicely in WebCore/platform though.
(In reply to comment #9) > I have two concerns with this patch: > > 1) It is missing a header guard. Of course :( > 2) I don't think WTF is the right layer for it. It might fit nicely in WebCore/platform though. I'm fine with its 3rd move (OwnGetDC->HwndDC -> HWndDC -> platform/win/HWndDC). However, it feels odd to me that we have OwnPtr<HDC> in the wtf layer (which works for Create*DC) but avoid the thing that can own a GetDC result.
(In reply to comment #10) > (In reply to comment #9) > > I have two concerns with this patch: > > > > 1) It is missing a header guard. > > Of course :( Fixed http://trac.webkit.org/changeset/104976 Waiting on #2 based on your thoughts (and I might land a few other patches first and then do the file move since I already have them done).
(In reply to comment #10) > (In reply to comment #9) > > I have two concerns with this patch: > > > > 1) It is missing a header guard. > > Of course :( > > > 2) I don't think WTF is the right layer for it. It might fit nicely in WebCore/platform though. > > I'm fine with its 3rd move (OwnGetDC->HwndDC -> HWndDC -> platform/win/HWndDC). > > However, it feels odd to me that we have OwnPtr<HDC> in the wtf layer (which works for Create*DC) but avoid the thing that can own a GetDC result. Memory management smart pointers have always had an odd relationship with WTF, nevertheless I still think this one should move up to platform/win.
(In reply to comment #12) > Memory management smart pointers have always had an odd relationship with WTF, nevertheless I still think this one should move up to platform/win. Patch: https://bugs.webkit.org/show_bug.cgi?id=76314
(In reply to comment #10) > However, it feels odd to me that we have OwnPtr<HDC> in the wtf layer That is a mistake I made. I don’t think we should be using OwnPtr for things that aren't simple new/delete, but I couldn’t resist expanding it for various Windows handle types. I suppose there’s more to be said on the subject, but that’s the basics.