[webkit-reviews] review granted: [Bug 126789] [Mac] [Windows] Stop scheduling network requests in WebCore : [Attachment 220903] proposed patch

[bugzilla-daemon at webkit.org]

Sam Weinig <sam at webkit.org> has granted Alexey Proskuryakov <ap at webkit.org>'s
request for review:
Bug 126789: [Mac] [Windows] Stop scheduling network requests in WebCore
https://bugs.webkit.org/show_bug.cgi?id=126789

Attachment 220903: proposed patch
https://bugs.webkit.org/attachment.cgi?id=220903&action=review

------- Additional Comments from Sam Weinig <sam at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=220903&action=review


> Source/WebCore/platform/network/mac/ResourceHandleMac.mm:178
> +	   // Synchronous requests should not be subject to regular connection
count limit to avoid deadlocks.

A bit more about these deadlocks would be useful here.

> Source/WebKit2/NetworkProcess/mac/NetworkResourceLoadSchedulerMac.mm:45
> +    // FIXME: What is the purpose of these assignments? WebKit normally uses
upper case "WK" functions directly.
>      wkInitializeMaximumHTTPConnectionCountPerHost =
WKInitializeMaximumHTTPConnectionCountPerHost;
> -    wkSetHTTPPipeliningMaximumPriority = WKSetHTTPPipeliningMaximumPriority;

> -    wkSetHTTPPipeliningMinimumFastLanePriority =
WKSetHTTPPipeliningMinimumFastLanePriority;
> +    wkSetHTTPRequestMaximumPriority = WKSetHTTPRequestMaximumPriority;
> +    wkSetHTTPRequestMinimumFastLanePriority =
WKSetHTTPRequestMinimumFastLanePriority;

I'd just go ahead and remove them rather than add the FIXME.

> Source/WebKit2/UIProcess/API/C/WKContext.cpp:-63
> -extern "C" {
> -// For binary compatibility with Safari 5.1. Should be removed eventually.
> -WK_EXPORT void _WKContextSetAdditionalPluginsDirectory(WKContextRef context,
WKStringRef pluginsDirectory);
> -WK_EXPORT void _WKContextRegisterURLSchemeAsEmptyDocument(WKContextRef
context, WKStringRef urlScheme);
> -WK_EXPORT void _WKContextSetAlwaysUsesComplexTextCodePath(WKContextRef
context, bool alwaysUseComplexTextCodePath);
> -WK_EXPORT void _WKContextSetHTTPPipeliningEnabled(WKContextRef context, bool
enabled);

I would save this for a different patch.