[Webkit-unassigned] [Bug 141508] [SOUP] Synchronous XMLHttpRequests can time out when we reach the max connections limit

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=141508

Sergio Villar Senin <svillar at igalia.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #247751|review?                     |review+
              Flags|                            |

--- Comment #9 from Sergio Villar Senin <svillar at igalia.com> ---
Comment on attachment 247751
  --> https://bugs.webkit.org/attachment.cgi?id=247751
Updated patch

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

Nice!

> Source/WebCore/ChangeLog:10
> +        connections allowed if the soup version is recent enough.

Perhaps we should explain here why the increase/decrease was not a good idea sometimes.

> Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp:935
> +    // Ignore the connection limits in synchronous loads to avoid freezing the networking process.

This comment doesn't add much as the code bellow is really explicit. I'd better include a link to the bug or a more detailed description of the issue. I'd prefer the former.

> Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp:942
>      g_signal_connect(d->m_soupMessage.get(), "network-event", G_CALLBACK(networkEventCallback), handle);

As you're fixing some other stuff like override,final, etc... I think we could use this bug to also store the SoupMessage in a local variable and use it all along this method which is full of the ugly d->m_soupMessage.get().

> Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestResources.cpp:704
> +        GUniquePtr<char> xhr(g_strdup_printf("xhr = new XMLHttpRequest; xhr.open('GET', '/sync-request-on-max-conns-xhr%u', false); xhr.send();", i));

If it were me I'd have stored this string in a static variable putting each instruction in a different line for better readability. But since this is a unit test I could live with it.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20150303/ac93c72a/attachment-0002.html>