52885 2011-01-21 04:13:47 -0800 [Gtk] ResourceHandleSoup: do not wait for streams to close to issue didFinishLoading 2011-01-24 03:23:14 -0800 1 1 1 Unclassified WebKit WebKitGTK 528+ (Nightly build) Other OS X 10.5 RESOLVED FIXED P2 Normal --- 1 svillar svillar abarth eric mrobinson webkit.review.bot oldest_to_newest 338001 0 svillar 2011-01-21 04:13:47 -0800 [Gtk] ResourceHandleSoup: do not wait for streams to close to issue didFinishLoading 338004 1 79723 svillar 2011-01-21 04:17:10 -0800 Created attachment 79723 Patch 338067 2 79723 mrobinson 2011-01-21 08:20:29 -0800 Comment on attachment 79723 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=79723&action=review This makes a lot of sense! > Source/WebCore/ChangeLog:9 > + No new tests as it does not change functionality. We will not wait for > + the the input stream to close to issue didFinishLoading to WebCore You probably want to mention here what the motiviation is then. Do you see a performance improvement, if so, is it significant? 338150 3 svillar 2011-01-21 10:33:19 -0800 (In reply to comment #2) > (From update of attachment 79723 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=79723&action=review > > This makes a lot of sense! > > > Source/WebCore/ChangeLog:9 > > + No new tests as it does not change functionality. We will not wait for > > + the the input stream to close to issue didFinishLoading to WebCore > > You probably want to mention here what the motiviation is then. Do you see a performance improvement, if so, is it significant? The performance improvement exists but it's very subtle as it won't be noticed unless we have a lot of asynchronous operations taking place at the same time, something that, on the other hand, is very likely to happen with complex web pages. I think even an small improvement worths the change as it's very simple but no strong feelings. At the same time it also seems more correct to issue the notification there than in a stream closing callback. 338345 4 mrobinson 2011-01-21 14:26:52 -0800 (In reply to comment #3) > I think even an small improvement worths the change as it's very simple but no strong feelings. At the same time it also seems more correct to issue the notification there than in a stream closing callback. I definitely agree that this patch is worthwhile. It should be fine to just mention this performance improvment in the ChangeLog, so that those reading later can understand the motivation for the change. :) 338921 5 svillar 2011-01-24 02:57:31 -0800 Committed r76498: <http://trac.webkit.org/changeset/76498> 338926 6 webkit.review.bot 2011-01-24 03:23:14 -0800 http://trac.webkit.org/changeset/76498 might have broken Qt Linux Release The following tests are not passing: inspector/debugger-autocontinue-on-syntax-error.html inspector/debugger-scripts.html inspector/storage-panel-dom-storage.html inspector/styles-add-blank-property.html 79723 2011-01-21 04:17:10 -0800 2011-01-21 08:20:29 -0800 Patch bug-52885-20110121131708.patch text/plain 2395 svillar ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCA2OTM1NmFiYzI2YmZiNjIyOTk5Yjk2MTg0MDk5NzAzNTg0OGI5NzBiLi5l NTVhNTNiY2Q5NTg1NjcwNTllZjc1Yzk0ZTkxNTE0Y2NiNzQ2ZGY4IDEwMDY0NAotLS0gYS9Tb3Vy Y2UvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0x LDMgKzEsMTcgQEAKKzIwMTEtMDEtMjEgIFNlcmdpbyBWaWxsYXIgU2VuaW4gIDxzdmlsbGFyQGln YWxpYS5jb20+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAg ICAgW0d0a10gUmVzb3VyY2VIYW5kbGVTb3VwOiBkbyBub3Qgd2FpdCBmb3Igc3RyZWFtcyB0byBj bG9zZSB0byBpc3N1ZSBkaWRGaW5pc2hMb2FkaW5nCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJr aXQub3JnL3Nob3dfYnVnLmNnaT9pZD01Mjg4NQorCisgICAgICAgIE5vIG5ldyB0ZXN0cyBhcyBp dCBkb2VzIG5vdCBjaGFuZ2UgZnVuY3Rpb25hbGl0eS4gV2Ugd2lsbCBub3Qgd2FpdCBmb3IKKyAg ICAgICAgdGhlIHRoZSBpbnB1dCBzdHJlYW0gdG8gY2xvc2UgdG8gaXNzdWUgZGlkRmluaXNoTG9h ZGluZyB0byBXZWJDb3JlCisKKyAgICAgICAgKiBwbGF0Zm9ybS9uZXR3b3JrL3NvdXAvUmVzb3Vy Y2VIYW5kbGVTb3VwLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OmNsb3NlQ2FsbGJhY2spOgorICAg ICAgICAoV2ViQ29yZTo6cmVhZENhbGxiYWNrKToKKwogMjAxMS0wMS0xMiAgUGF2ZWwgUG9kaXZp bG92ICA8cG9kaXZpbG92QGNocm9taXVtLm9yZz4KIAogICAgICAgICBSZXZpZXdlZCBieSBQYXZl bCBGZWxkbWFuLgpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vbmV0d29yay9z b3VwL1Jlc291cmNlSGFuZGxlU291cC5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9uZXR3 b3JrL3NvdXAvUmVzb3VyY2VIYW5kbGVTb3VwLmNwcAppbmRleCBhNzE3MGZlODU5MWZjYjY2YWRk Y2QxNThlZTA1NWVkYzVmYjBmYmQ3Li40ZWU5ZWY2Mjc4NTVmYzE3NmE2NTYwYTA1YTI2ZTExOTAw MmNmNGE4IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9uZXR3b3JrL3NvdXAv UmVzb3VyY2VIYW5kbGVTb3VwLmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9uZXR3 b3JrL3NvdXAvUmVzb3VyY2VIYW5kbGVTb3VwLmNwcApAQCAtODExLDE4ICs4MTEsOCBAQCBzdGF0 aWMgdm9pZCBjbG9zZUNhbGxiYWNrKEdPYmplY3QqIHNvdXJjZSwgR0FzeW5jUmVzdWx0KiByZXMs IGdwb2ludGVyKQogICAgICAgICByZXR1cm47CiAKICAgICBSZXNvdXJjZUhhbmRsZUludGVybmFs KiBkID0gaGFuZGxlLT5nZXRJbnRlcm5hbCgpOwotICAgIFJlc291cmNlSGFuZGxlQ2xpZW50KiBj bGllbnQgPSBoYW5kbGUtPmNsaWVudCgpOwotCiAgICAgZ19pbnB1dF9zdHJlYW1fY2xvc2VfZmlu aXNoKGQtPm1faW5wdXRTdHJlYW0uZ2V0KCksIHJlcywgMCk7CiAgICAgY2xlYW51cFNvdXBSZXF1 ZXN0T3BlcmF0aW9uKGhhbmRsZS5nZXQoKSk7Ci0KLSAgICAvLyBUaGUgbG9hZCBtYXkgaGF2ZSBi ZWVuIGNhbmNlbGxlZCwgdGhlIGNsaWVudCBtYXkgaGF2ZSBiZWVuCi0gICAgLy8gZGVzdHJveWVk IGFscmVhZHkuIEluIHN1Y2ggY2FzZXMgY2FsbGluZyBkaWRGaW5pc2hMb2FkaW5nIGlzIGEKLSAg ICAvLyBiYWQgaWRlYS4KLSAgICBpZiAoZC0+bV9jYW5jZWxsZWQgfHwgIWNsaWVudCkKLSAgICAg ICAgcmV0dXJuOwotCi0gICAgY2xpZW50LT5kaWRGaW5pc2hMb2FkaW5nKGhhbmRsZS5nZXQoKSwg MCk7CiB9CiAKIHN0YXRpYyB2b2lkIHJlYWRDYWxsYmFjayhHT2JqZWN0KiBzb3VyY2UsIEdBc3lu Y1Jlc3VsdCogYXN5bmNSZXN1bHQsIGdwb2ludGVyIGRhdGEpCkBAIC04NTQsNiArODQ0LDEwIEBA IHN0YXRpYyB2b2lkIHJlYWRDYWxsYmFjayhHT2JqZWN0KiBzb3VyY2UsIEdBc3luY1Jlc3VsdCog YXN5bmNSZXN1bHQsIGdwb2ludGVyIGRhCiAgICAgfQogCiAgICAgaWYgKCFieXRlc1JlYWQpIHsK KyAgICAgICAgLy8gRmluaXNoIHRoZSBsb2FkLiBXZSBkbyBub3Qgd2FpdCBmb3IgdGhlIHN0cmVh bSB0bworICAgICAgICAvLyBjbG9zZS4gSW5zdGVhZCB3ZSBiZXR0ZXIgbm90aWZ5IFdlYkNvcmUg YXMgc29vbiBhcyBwb3NzaWJsZQorICAgICAgICBjbGllbnQtPmRpZEZpbmlzaExvYWRpbmcoaGFu ZGxlLmdldCgpLCAwKTsKKwogICAgICAgICBnX2lucHV0X3N0cmVhbV9jbG9zZV9hc3luYyhkLT5t X2lucHV0U3RyZWFtLmdldCgpLCBHX1BSSU9SSVRZX0RFRkFVTFQsCiAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgIDAsIGNsb3NlQ2FsbGJhY2ssIDApOwogICAgICAgICByZXR1cm47 Cg==