Bug 64130 – Implement didReceiveDataArray callback for CFNetwork based loader

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Pratik Solanki]

Same as <https://bugs.webkit.org/show_bug.cgi?id=56838> but for the CFNetwork side.

[Pratik Solanki]

Created attachment 100049 [details]
Patch

[Pratik Solanki]

Created attachment 100056 [details]
Patch.v2

Updated patch that should apply cleanly. It includes some of the header file changes from patch in bug 63674.

[Jessie Berlin]

Comment on attachment 100056 [details]
Patch.v2

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

Unofficial r=me!

> Source/WebCore/platform/network/cf/ResourceHandleCFNet.cpp:449
> +    CFURLConnectionClient_V6 client = { 6, this, 0, 0, 0, WebCore::willSendRequest, didReceiveResponse, didReceiveData, 0, didFinishLoading, didFail, willCacheResponse, didReceiveChallenge, didSendBodyData, shouldUseCredentialStorageCallback, 0, 0, 0, didReceiveDataArray};

Is there a reason to name this "V6"? Are there versions 4 and 5 somewhere?

> Source/WebCore/platform/network/cf/ResourceHandleCFNet.cpp:877
> +        client()->didReceiveDataArray(this, dataArray);

I know you are just moving this code, but instead of the extra nesting of and if { ... } else { ... } clause here, it would be better to just do an early return:

if (client()->supportsDataArray()) {
    client()->didReceiveDataArray(this, dataArray);
    return;
}

CFIndex count = CFArrayGetCount(....

> Source/WebCore/platform/network/cf/ResourceHandleCFNet.cpp:885
> +        } else {

Ditto about an early return being better here.

[Pratik Solanki]

Comment on attachment 100056 [details]
Patch.v2

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

>> Source/WebCore/platform/network/cf/ResourceHandleCFNet.cpp:449
>> +    CFURLConnectionClient_V6 client = { 6, this, 0, 0, 0, WebCore::willSendRequest, didReceiveResponse, didReceiveData, 0, didFinishLoading, didFail, willCacheResponse, didReceiveChallenge, didSendBodyData, shouldUseCredentialStorageCallback, 0, 0, 0, didReceiveDataArray};
> 
> Is there a reason to name this "V6"? Are there versions 4 and 5 somewhere?

Yes. There's CFURLConnectionClient_V4 and CFURLConnectionClient_V5 in CFURLConnection.h.

>> Source/WebCore/platform/network/cf/ResourceHandleCFNet.cpp:877
>> +        client()->didReceiveDataArray(this, dataArray);
> 
> I know you are just moving this code, but instead of the extra nesting of and if { ... } else { ... } clause here, it would be better to just do an early return:
> 
> if (client()->supportsDataArray()) {
>     client()->didReceiveDataArray(this, dataArray);
>     return;
> }
> 
> CFIndex count = CFArrayGetCount(....

Ok. I can do that.

>> Source/WebCore/platform/network/cf/ResourceHandleCFNet.cpp:885
>> +        } else {
> 
> Ditto about an early return being better here.

Ok.

[David Kilzer (:ddkilzer)]

Comment on attachment 100056 [details]
Patch.v2

r=me with jessieberlin's comments addressed.

[Pratik Solanki]

Committed r90834: <http://trac.webkit.org/changeset/90834>