[Webkit-unassigned] [Bug 44261] [GTK] Add HTTP caching support

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=44261


Martin Robinson <mrobinson at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #66190|review?                     |review-
               Flag|                            |




--- Comment #47 from Martin Robinson <mrobinson at webkit.org>  2010-09-06 09:51:44 PST ---
(From update of attachment 66190)
This patch is looking good. I'm excited for this to land.

View in context: https://bugs.webkit.org/attachment.cgi?id=66190&action=prettypatch

> WebCore/platform/network/soup/ResourceHandleSoup.cpp:138
> +        m_soupRequest.clear();
m_soupRequest.clear(); is unecessary here, I believe. The destructor will take care of decrementing the smart pointer reference count. If this is needed otherwise there should really be a comment explaining why.

> WebCore/platform/network/soup/ResourceHandleSoup.cpp:196
> +    String location = String(uri.get());
Again, I believe this should be ::fromUTF8(...).

> WebCore/platform/network/soup/ResourceHandleSoup.cpp:305
> +#define BUFFER_SIZE 8192
If this is a constant, why do we need to store m_bufferSize? Let's just get rid of that member if possible. Again, I think this should be called READ_BUFFER_SIZE and be at the top of the file too.

> WebCore/platform/network/soup/ResourceHandleSoup.cpp:689
> +                            sendRequestCallback, 0);
This newline seems unecessary. I usually split the line when it goes over 120 characters.

> WebCore/platform/network/soup/ResourceHandleSoup.cpp:846
> +        client->didReceiveData(handle.get(), reinterpret_cast<const char*>(unicodeStr.get()), g_utf8_strlen(d->m_buffer, bytesRead) * sizeof(UChar), 0);
Would it be possible to simply use WTFString here?

String data = String::fromUTF8(d->m_buffer, bytesRead);
client->didReceiveData(handle.get(), reinterpret_cast<const char*>(data.characters()), data.length() * sizeof(UChar), 0);

> WebCore/platform/network/soup/ResourceHandleSoup.cpp:867
> +    GOwnPtr<char> urlStr;
Please use CString here, if possible. It was made just for this purpose.

> WebCore/platform/network/soup/ResourceHandleSoup.cpp:901
> +
Please remove the newline.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.