see patch
Created attachment 68260 [details] Patch
Comment on attachment 68260 [details] Patch I'm finding this patch hard to review, which I think is a sign that this patch is trying to do too much at once. Is there a way to break it up into smaller pieces?
Created attachment 68285 [details] Patch (subset of first patch)
Comment on attachment 68285 [details] Patch (subset of first patch) 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.
Created attachment 68516 [details] Patch (subset of first 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? No, no need, because nothing will happen after the "return false".
Comment on attachment 68516 [details] Patch (subset of first patch) Clearing flags on attachment: 68516 Committed r68147: <http://trac.webkit.org/changeset/68147>
All reviewed patches have been landed. Closing bug.