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

[bugzilla-daemon at webkit.org]

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





--- Comment #81 from Sergio Villar Senin <svillar at igalia.com>  2010-09-27 02:12:32 PST ---
(From update of attachment 68664)
View in context: https://bugs.webkit.org/attachment.cgi?id=68664&action=review

>> WebCore/platform/network/ResourceHandleInternal.h:51
>> +#include <GRefPtr.h>
> 
> Nitpick, but requester.h already includes request.h

Indeed. Will remove it.

>> WebCore/platform/network/soup/ResourceHandleSoup.cpp:196
>> +    String location = String::fromUTF8(uri.get());
> 
> Can't we just commit this bit now?

Yes. Martin asked for the change, that's why it's included in this patch

>> WebCore/platform/network/soup/ResourceHandleSoup.cpp:305
>>  // parseDataUrl() is taken from the CURL http backend.
> 
> I guess you want to remove this comment now, since the function is wildly different by now (or at least say 'originally' or something).

True

>> WebCore/platform/network/soup/ResourceHandleSoup.cpp:387
>>  
> 
> GINT_TO_POINTER(!isBase64) ? Could fail?

I just wanted to be ultra-explicit but what you propose would be the same.

>> WebCore/platform/network/soup/ResourceHandleSoup.cpp:453
>> +
> 
> Any reason not to use platform ptrs here too?

They are not platform ptrs right now. Not sure if that change should be included in this patch. I thought it was a general rule not to include changes not directly related to the issue the patch tries to fix.

>> WebCore/platform/network/soup/ResourceHandleSoup.cpp:515
>> +
> 
> These two ifs are almost identical, I think they can be trivially merged.

Indeed they can

>> WebCore/platform/network/soup/ResourceHandleSoup.cpp:544
>> +    d->m_buffer = static_cast<char*>(g_malloc(READ_BUFFER_SIZE));
> 
> Am I the only one worried that we malloc 8k for each outgoing HTTP request? or am I missing something here. Is this just making something explicit that was implicit before or we adding an extra copy to the whole process?

:m really good point here. I think you're right and we're adding an extra copy for http requests when reading from the input stream. With the old code all the didReceiveData calls were using buffers provided by libsoup without any extra copy. I guess we need something better then...

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