Bug 63916 – Coalesce data array into one NSData before calling didReceiveData

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

[Pratik Solanki]

If the ResourceHandleClient does not support accepting data array, we call its didReceiveData() method in a loop. Instead of doing that we should coalesce all the elements in the array and call didReceiveData once.

[Pratik Solanki]

<rdar://problem/9715181>

[Pratik Solanki]

Created attachment 99651 [details]
Patch

Comment on attachment 99651 [details]
Patch

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

> Source/WebCore/ChangeLog:10
> +        the data buffers into one anc all it with all the data at once.

Typo: anc.

> Source/WebCore/platform/network/mac/ResourceHandleMac.mm:-815
> -        // The call to didReceiveData below could cancel a load, which would result in the delegate
> -        // (self) being released.

Perhaps this is worth replacing with a comment where there is a currently useless return statement saying that at that point self may have been deallocated.

[Alexey Proskuryakov]

> Instead of doing that we should coalesce all the elements

Why should we do that???

[Alexey Proskuryakov]

It seems that it would be more straightforward to remove connection:didReceiveDataArray:, and let CFNetwork do the same coalescing itself before calling connection:didReceiveData:.

[Pratik Solanki]

I discussed this with Alexey offline and explained how we are modifying the else part of

   if (m_handle->client()->supportsDataArray())
       m_handle->client()->didReceiveDataArray(m_handle, reinterpret_cast<CFArrayRef>(dataArray));
   else {
       <other clients that don't handle data arrays>
   }

The patch made more sense to him now. He adds "The problem with not coalescing probably comes from TextResourceDecoder, which scans the source multiple times if it gets small chunks."

I'll check in the patch shortly. Thanks for the review, Dan!

[Pratik Solanki]

Committed r90400: <http://trac.webkit.org/changeset/90400>