63916 2011-07-04 12:51:57 -0700 Coalesce data array into one NSData before calling didReceiveData 2011-07-05 10:35:03 -0700 1 1 1 Unclassified WebKit Platform 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 psolanki psolanki ap koivisto mitz psolanki oldest_to_newest 432017 0 psolanki 2011-07-04 12:51:57 -0700 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. 432018 1 psolanki 2011-07-04 12:52:15 -0700 <rdar://problem/9715181> 432023 2 99651 psolanki 2011-07-04 13:05:44 -0700 Created attachment 99651 Patch 432026 3 99651 mitz 2011-07-04 13:12:03 -0700 Comment on attachment 99651 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. 432046 4 ap 2011-07-04 13:48:59 -0700 > Instead of doing that we should coalesce all the elements Why should we do that??? 432119 5 ap 2011-07-05 00:00:57 -0700 It seems that it would be more straightforward to remove connection:didReceiveDataArray:, and let CFNetwork do the same coalescing itself before calling connection:didReceiveData:. 432305 6 psolanki 2011-07-05 10:01:12 -0700 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! 432322 7 psolanki 2011-07-05 10:35:03 -0700 Committed r90400: <http://trac.webkit.org/changeset/90400> 99651 2011-07-04 13:05:44 -0700 2011-07-04 13:12:03 -0700 Patch bug-63916-20110704130542.patch text/plain 2734 psolanki U3VidmVyc2lvbiBSZXZpc2lvbjogOTAzNzEKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwppbmRleCAwNDhjMDE0ODQ1NWY5MDkx N2Y3ZDIyYjQzOTc1M2I4MzhiOGFjZjkwLi4yN2I4ZjdhZGY0M2IzM2RmNTA4OWRlYTZlZDU1Yzkz OTY2YjdmNWJiIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvU291 cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTkgQEAKKzIwMTEtMDctMDQgIFByYXRp ayBTb2xhbmtpICA8cHNvbGFua2lAYXBwbGUuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5P Qk9EWSAoT09QUyEpLgorCisgICAgICAgIENvYWxlc2NlIGRhdGEgYXJyYXkgaW50byBvbmUgTlNE YXRhIGJlZm9yZSBjYWxsaW5nIGRpZFJlY2VpdmVEYXRhCisgICAgICAgIGh0dHBzOi8vYnVncy53 ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD02MzkxNgorICAgICAgICA8cmRhcjovL3Byb2JsZW0v OTcxNTE4MT4KKworICAgICAgICBJbnN0ZWFkIG9mIGNhbGxpbmcgZGlkUmVjZWl2ZURhdGEgbXVs dGlwbGUgdGltZXMgd2l0aCBzbWFsbGVyIGNodW5rcyBvZiBkYXRhLCB3ZSBjYWxsIG1lcmdlCisg ICAgICAgIHRoZSBkYXRhIGJ1ZmZlcnMgaW50byBvbmUgYW5jIGFsbCBpdCB3aXRoIGFsbCB0aGUg ZGF0YSBhdCBvbmNlLgorCisgICAgICAgIE5vIG5ldyB0ZXN0cyBiZWNhdXNlIHRoZSBmbGFnIGlz bid0IGVuYWJsZWQgeWV0LgorCisgICAgICAgICogcGxhdGZvcm0vbmV0d29yay9tYWMvUmVzb3Vy Y2VIYW5kbGVNYWMubW06CisgICAgICAgICgtW1dlYkNvcmVSZXNvdXJjZUhhbmRsZUFzRGVsZWdh dGUgY29ubmVjdGlvbjpkaWRSZWNlaXZlRGF0YUFycmF5Ol0pOgorCiAyMDExLTA3LTA0ICBKZWZm IFRpbWFudXMgIDx0d2l6QGNocm9taXVtLm9yZz4KIAogICAgICAgICBSZXZpZXdlZCBieSBTdGVw aGVuIFdoaXRlLgpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vbmV0d29yay9t YWMvUmVzb3VyY2VIYW5kbGVNYWMubW0gYi9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9uZXR3b3Jr L21hYy9SZXNvdXJjZUhhbmRsZU1hYy5tbQppbmRleCA3MjQ2MzFiZDY5OTg4ODkyM2FhYTNhMDBh MjNhZGRkYWJhZWM0MjI4Li5lNjY5ZTA2MjIzYzk2ZTQxNjM4YzYwY2ExNmQyYjZlYTZlZDI2MGRi IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9uZXR3b3JrL21hYy9SZXNvdXJj ZUhhbmRsZU1hYy5tbQorKysgYi9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9uZXR3b3JrL21hYy9S ZXNvdXJjZUhhbmRsZU1hYy5tbQpAQCAtODExLDEzICs4MTEsMjEgQEAgU3RyaW5nIFJlc291cmNl SGFuZGxlOjpwcml2YXRlQnJvd3NpbmdTdG9yYWdlU2Vzc2lvbklkZW50aWZpZXJEZWZhdWx0QmFz ZSgpCiAgICAgaWYgKG1faGFuZGxlLT5jbGllbnQoKS0+c3VwcG9ydHNEYXRhQXJyYXkoKSkKICAg ICAgICAgbV9oYW5kbGUtPmNsaWVudCgpLT5kaWRSZWNlaXZlRGF0YUFycmF5KG1faGFuZGxlLCBy ZWludGVycHJldF9jYXN0PENGQXJyYXlSZWY+KGRhdGFBcnJheSkpOwogICAgIGVsc2UgewotICAg ICAgICAvLyBUaGUgY2FsbCB0byBkaWRSZWNlaXZlRGF0YSBiZWxvdyBjb3VsZCBjYW5jZWwgYSBs b2FkLCB3aGljaCB3b3VsZCByZXN1bHQgaW4gdGhlIGRlbGVnYXRlCi0gICAgICAgIC8vIChzZWxm KSBiZWluZyByZWxlYXNlZC4KLSAgICAgICAgUmV0YWluUHRyPFdlYkNvcmVSZXNvdXJjZUhhbmRs ZUFzRGVsZWdhdGU+IHByb3RlY3Qoc2VsZik7Ci0gICAgICAgIGZvciAoTlNEYXRhICpkYXRhIGlu IGRhdGFBcnJheSkgewotICAgICAgICAgICAgaWYgKCFtX2hhbmRsZSkKLSAgICAgICAgICAgICAg ICBicmVhazsKKyAgICAgICAgTlNVSW50ZWdlciBjb3VudCA9IFtkYXRhQXJyYXkgY291bnRdOwor ICAgICAgICBBU1NFUlQoY291bnQpOworICAgICAgICBpZiAoY291bnQgPT0gMSkgeworICAgICAg ICAgICAgTlNEYXRhICpkYXRhID0gW2RhdGFBcnJheSBvYmplY3RBdEluZGV4OjBdOwogICAgICAg ICAgICAgbV9oYW5kbGUtPmNsaWVudCgpLT5kaWRSZWNlaXZlRGF0YShtX2hhbmRsZSwgc3RhdGlj X2Nhc3Q8Y29uc3QgY2hhcio+KFtkYXRhIGJ5dGVzXSksIFtkYXRhIGxlbmd0aF0sIHN0YXRpY19j YXN0PGludD4oW2RhdGEgbGVuZ3RoXSkpOworICAgICAgICB9IGVsc2UgeworICAgICAgICAgICAg TlNVSW50ZWdlciB0b3RhbFNpemUgPSAwOworICAgICAgICAgICAgZm9yIChOU0RhdGEgKmRhdGEg aW4gZGF0YUFycmF5KQorICAgICAgICAgICAgICAgIHRvdGFsU2l6ZSArPSBbZGF0YSBsZW5ndGhd OworCisgICAgICAgICAgICBSZXRhaW5QdHI8TlNNdXRhYmxlRGF0YT4gbWVyZ2VkRGF0YShBZG9w dE5TLCBbW05TTXV0YWJsZURhdGEgYWxsb2NdIGluaXRXaXRoQ2FwYWNpdHk6dG90YWxTaXplXSk7 CisgICAgICAgICAgICBmb3IgKE5TRGF0YSAqZGF0YSBpbiBkYXRhQXJyYXkpCisgICAgICAgICAg ICAgICAgW21lcmdlZERhdGEuZ2V0KCkgYXBwZW5kRGF0YTpkYXRhXTsKKworICAgICAgICAgICAg bV9oYW5kbGUtPmNsaWVudCgpLT5kaWRSZWNlaXZlRGF0YShtX2hhbmRsZSwgc3RhdGljX2Nhc3Q8 Y29uc3QgY2hhcio+KFttZXJnZWREYXRhLmdldCgpIGJ5dGVzXSksIHRvdGFsU2l6ZSwgc3RhdGlj X2Nhc3Q8aW50Pih0b3RhbFNpemUpKTsKICAgICAgICAgfQogICAgIH0KICAgICByZXR1cm47Cg==