Bug 126789

Summary: [Mac] [Windows] Stop scheduling network requests in WebCore
Product: WebKit Reporter: Alexey Proskuryakov <ap>
Component: Page LoadingAssignee: Alexey Proskuryakov <ap>
Status: RESOLVED FIXED    
Severity: Normal CC: beidson, buildbot, commit-queue, japhet, koivisto, psolanki, rniwa, sam
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
proposed patch sam: review+, buildbot: commit-queue-

Description Alexey Proskuryakov 2014-01-10 16:16:13 PST
We already have the code to set request priorities, can just pass the job down on CFNetwork.

<rdar://problem/15114727>
Comment 1 Alexey Proskuryakov 2014-01-10 16:28:05 PST
Created attachment 220903 [details]
proposed patch

This patch will fail to build, because it needs WebKitSystemInterface changes. Please review anyway.
Comment 2 Build Bot 2014-01-10 17:08:18 PST
Comment on attachment 220903 [details]
proposed patch

Attachment 220903 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/4533502531862528
Comment 3 Build Bot 2014-01-10 17:30:59 PST
Comment on attachment 220903 [details]
proposed patch

Attachment 220903 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/4673190437584896
Comment 4 Sam Weinig 2014-01-10 23:57:35 PST
Comment on attachment 220903 [details]
proposed patch

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.
Comment 5 Alexey Proskuryakov 2014-01-11 20:16:45 PST
Committed <http://trac.webkit.org/r161796>.