Bug 109351 – Cache XHRs is not working well

• Status: UNCONFIRMED
• Component: Page Loading

[Hyungwook Lee]

Cache XHRs is not working well as we expected caused by wrong httpBody() comparison.

bool CachedRawResource::canReuse(const ResourceRequest& newRequest) const 
{
...
    if (m_resourceRequest.httpBody() != newRequest.httpBody())
        return false;
...
}

FormData* ResourceRequestBase::httpBody() returns pointer.
Hence we need to compare its value instead of pointer.

[Hyungwook Lee]

Created attachment 187425 [details]
patches for 109351

[WebKit Review Bot]

Attachment 187425 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/loader/cache/CachedRawResource.cpp']" exit_code: 1
Source/WebCore/loader/cache/CachedRawResource.cpp:183:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/loader/cache/CachedRawResource.cpp:184:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/loader/cache/CachedRawResource.cpp:186:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/loader/cache/CachedRawResource.cpp:187:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/loader/cache/CachedRawResource.cpp:189:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/loader/cache/CachedRawResource.cpp:190:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/loader/cache/CachedRawResource.cpp:191:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/loader/cache/CachedRawResource.cpp:192:  Tab found; better to use spaces  [whitespace/tab] [1]
Total errors found: 8 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Hyungwook Lee]

Created attachment 187426 [details]
patches for 109351

[Alexey Proskuryakov]

What is the expected behavior when just sending two identical POST requests with XHR? I expect them to be delivered to the server.

Caching POST responses makes sense for back/forward navigation, but probably not for XHR.

So it seems like this mistake was actually resulting in correct behavior.

[Hyungwook Lee]

Oh I see,

I got your point and I can understand why it uses pointer instead of value.

[Nate Chapin]

(In reply to comment #4)
> What is the expected behavior when just sending two identical POST requests with XHR? I expect them to be delivered to the server.
> 
> Caching POST responses makes sense for back/forward navigation, but probably not for XHR.
> 
> So it seems like this mistake was actually resulting in correct behavior.

For reasons I don't remember, canReuse() permits reuse of POST requests if the new request is POST as well. Should we change it to support GET only?

[Alexey Proskuryakov]

> For reasons I don't remember, canReuse() permits reuse of POST requests if the new request is POST as well

Perhaps that's for b/f navigation?

[Hyungwook Lee]

I think my patches is not fully considered real world's situation. 
Using cached XHRs (GET/POST both) easily break many web services which uses XHRs to update their web services.