[webkit-reviews] review granted: [Bug 46200] Cleanup network code in ResourceHandleWin : [Attachment 68285] Patch (subset of first patch)
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu Sep 23 06:53:38 PDT 2010
Adam Roben (aroben) <aroben at apple.com> has granted Patrick R. Gansterer
<paroga at paroga.com>'s request for review:
Bug 46200: Cleanup network code in ResourceHandleWin
https://bugs.webkit.org/show_bug.cgi?id=46200
Attachment 68285: Patch (subset of first patch)
https://bugs.webkit.org/attachment.cgi?id=68285&action=review
------- Additional Comments from Adam Roben (aroben) <aroben at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=68285&action=review
> WebCore/platform/network/win/ResourceHandleWin.cpp:405
> + DWORD flags =
> + INTERNET_FLAG_KEEP_CONNECTION |
> + INTERNET_FLAG_IGNORE_REDIRECT_TO_HTTPS |
> + INTERNET_FLAG_IGNORE_REDIRECT_TO_HTTP;
Operators should be at the start of each line, not the end.
> WebCore/platform/network/win/ResourceHandleWin.cpp:414
> + d->m_connectHandle = InternetConnectW(d->m_internetHandle,
> +
firstRequest().url().host().charactersWithNullTermination(),
> + firstRequest().url().port(),
> + 0, // no username
> + 0, // no password
> + INTERNET_SERVICE_HTTP,
> + flags,
> +
reinterpret_cast<DWORD_PTR>(this));
We don't normally wrap and line up parameters like this. (Same comment applies
elsewhere in this patch.)
> WebCore/platform/network/win/ResourceHandleWin.cpp:444
> + if (!d->m_requestHandle) {
> + InternetCloseHandle(d->m_connectHandle);
> + return false;
> + }
Do we need to null out d->m_connectHandle here, too?
> WebCore/platform/network/win/ResourceHandleWin.cpp:448
> + INTERNET_BUFFERSW internetBuffers;
> + ZeroMemory(&internetBuffers, sizeof(INTERNET_BUFFERSW));
> + internetBuffers.dwStructSize = sizeof(INTERNET_BUFFERSW);
If you use sizeof(internetBuffers) instead, you won't have to change three
lines if the type changes in the future.
More information about the webkit-reviews
mailing list