Bug 76737 – Allow delayed DC allocation in HWndDC.

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[David Levin]

See summary.

[David Levin]

Created attachment 123375 [details]
Patch

[David Levin]

I'll gladly accept a review from anyone who will provide one.

[WebKit Review Bot]

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.

[Dmitry Lomov]

Comment on attachment 123375 [details]
Patch

Looks good to me. Any usages in the code?

[David Levin]

(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.

[Adam Roben (:aroben)]

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.

[David Levin]

(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.

[David Levin]

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.