| Summary: | [Gtk] ResourceHandleSoup: do not wait for streams to close to issue didFinishLoading | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Sergio Villar Senin <svillar> | ||||
| Component: | WebKitGTK | Assignee: | Sergio Villar Senin <svillar> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | abarth, eric, mrobinson, webkit.review.bot | ||||
| Priority: | P2 | ||||||
| Version: | 528+ (Nightly build) | ||||||
| Hardware: | Other | ||||||
| OS: | OS X 10.5 | ||||||
| Attachments: |
|
||||||
|
Description
Sergio Villar Senin
2011-01-21 04:13:47 PST
Created attachment 79723 [details]
Patch
Comment on attachment 79723 [details] 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? (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. (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. :) Committed r76498: <http://trac.webkit.org/changeset/76498> 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 |