Attachment 233077 Details for Bug 133882 – proposed fix

[patch] proposed fix

GmailFreeze.txt (text/plain), 10.51 KB, created by Alexey Proskuryakov on 2014-06-13 14:27:52 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Alexey Proskuryakov

Created: 2014-06-13 14:27:52 PDT

Size: 10.51 KB

patch

obsolete

>Index: Source/WebCore/ChangeLog
>===================================================================
>--- Source/WebCore/ChangeLog	(revision 169948)
>+++ Source/WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,27 @@
>+2014-06-13  Alexey Proskuryakov  <ap@apple.com>
>+
>+        REGRESSION: Mountain Lion: Gmail's "in new window" view of mail threads is very slow to load its content
>+        https://bugs.webkit.org/show_bug.cgi?id=133882
>+        <rdar://problem/17271965>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        This goes to pre-r161796 behavior on 10.8 and 10.9. We schedule the requests internally,
>+        and only give 6 of them to CFNetwork at a time. Except for synchronous requests,
>+        which still use the improved approach, and don't count against 6 connections.
>+
>+        * loader/ResourceLoadScheduler.cpp:
>+        (WebCore::ResourceLoadScheduler::scheduleLoad):
>+        * platform/network/cf/ResourceRequest.h:
>+        (WebCore::ResourceRequest::resourcePrioritiesEnabled):
>+        * platform/network/cf/ResourceRequestCFNet.cpp:
>+        (WebCore::ResourceRequest::doUpdatePlatformRequest):
>+        (WebCore::ResourceRequest::doUpdateResourceRequest):
>+        (WebCore::initializeMaximumHTTPConnectionCountPerHost): Deleted.
>+        * platform/network/mac/ResourceRequestMac.mm:
>+        (WebCore::ResourceRequest::doUpdateResourceRequest):
>+        (WebCore::ResourceRequest::doUpdatePlatformRequest):
>+
> 2014-06-13  Brent Fulgham  <bfulgham@apple.com>
> 
>         Convert LayerPool and TileCoverageMap from ObjC to C++
>Index: Source/WebCore/loader/ResourceLoadScheduler.cpp
>===================================================================
>--- Source/WebCore/loader/ResourceLoadScheduler.cpp	(revision 169900)
>+++ Source/WebCore/loader/ResourceLoadScheduler.cpp	(working copy)
>@@ -163,7 +163,7 @@ void ResourceLoadScheduler::scheduleLoad
>     host->schedule(resourceLoader, priority);
> 
> #if PLATFORM(COCOA) || USE(CFNETWORK)
>-    if (!isSuspendingPendingRequests()) {
>+    if (ResourceRequest::resourcePrioritiesEnabled() && !isSuspendingPendingRequests()) {
>         // Serve all requests at once to keep the pipeline full at the network layer.
>         // FIXME: Does this code do anything useful, given that we also set maxRequestsInFlightPerHost to effectively unlimited on these platforms?
>         servePendingRequests(host, ResourceLoadPriorityVeryLow);
>Index: Source/WebCore/platform/network/cf/ResourceRequest.h
>===================================================================
>--- Source/WebCore/platform/network/cf/ResourceRequest.h	(revision 169900)
>+++ Source/WebCore/platform/network/cf/ResourceRequest.h	(working copy)
>@@ -126,6 +126,8 @@ namespace WebCore {
>         static bool httpPipeliningEnabled();
>         static void setHTTPPipeliningEnabled(bool);
> 
>+        static bool resourcePrioritiesEnabled();
>+
> #if PLATFORM(COCOA)
>         static bool useQuickLookResourceCachingQuirks();
> #endif
>@@ -168,6 +170,24 @@ namespace WebCore {
> #endif
>     };
> 
>+    inline bool ResourceRequest::resourcePrioritiesEnabled()
>+    {
>+#if PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 10100
>+    return true;
>+#elif PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED < 10100
>+    // See <rdar://problem/16518595>, <rdar://problem/17168793> for issues we had before OS X 10.10.
>+    // HTTP Pipelining could be enabled for experiments, but there is no point in doing so on old OS versions,
>+    // and that can't work well because of the above issues.
>+    ASSERT(!httpPipeliningEnabled());
>+    return false;
>+#elif PLATFORM(IOS)
>+    return true;
>+#elif PLATFORM(WIN)
>+    return false;
>+#endif
>+    }
>+
>+
> } // namespace WebCore
> 
> #endif // ResourceRequest_h
>Index: Source/WebCore/platform/network/cf/ResourceRequestCFNet.cpp
>===================================================================
>--- Source/WebCore/platform/network/cf/ResourceRequestCFNet.cpp	(revision 169900)
>+++ Source/WebCore/platform/network/cf/ResourceRequestCFNet.cpp	(working copy)
>@@ -155,7 +155,8 @@ void ResourceRequest::doUpdatePlatformRe
>     if (httpPipeliningEnabled())
>         wkHTTPRequestEnablePipelining(cfRequest);
> 
>-    wkSetHTTPRequestPriority(cfRequest, toPlatformRequestPriority(m_priority));
>+    if (resourcePrioritiesEnabled())
>+        wkSetHTTPRequestPriority(cfRequest, toPlatformRequestPriority(m_priority));
> 
> #if !PLATFORM(WIN)
>     wkCFURLRequestAllowAllPostCaching(cfRequest);
>@@ -282,7 +283,8 @@ void ResourceRequest::doUpdateResourceRe
>     }
>     m_allowCookies = CFURLRequestShouldHandleHTTPCookies(m_cfRequest.get());
> 
>-    m_priority = toResourceLoadPriority(wkGetHTTPRequestPriority(m_cfRequest.get()));
>+    if (resourcePrioritiesEnabled()
>+        m_priority = toResourceLoadPriority(wkGetHTTPRequestPriority(m_cfRequest.get()));
> 
>     m_httpHeaderFields.clear();
>     if (CFDictionaryRef headers = CFURLRequestCopyAllHTTPHeaderFields(m_cfRequest.get())) {
>@@ -398,27 +400,32 @@ void ResourceRequest::doPlatformAdopt(Pa
> #endif
> }
> 
>+// FIXME: It is confusing that this function both sets connection count and determines maximum request count at network layer. This can and should be done separately.
> unsigned initializeMaximumHTTPConnectionCountPerHost()
> {
>     static const unsigned preferredConnectionCount = 6;
>-    static const unsigned unlimitedConnectionCount = 10000;
>+    static const unsigned unlimitedRequestCount = 10000;
> 
>-    wkInitializeMaximumHTTPConnectionCountPerHost(preferredConnectionCount);
>+    unsigned maximumHTTPConnectionCountPerHost = wkInitializeMaximumHTTPConnectionCountPerHost(preferredConnectionCount);
> 
>     Boolean keyExistsAndHasValidFormat = false;
>     Boolean prefValue = CFPreferencesGetAppBooleanValue(CFSTR("WebKitEnableHTTPPipelining"), kCFPreferencesCurrentApplication, &keyExistsAndHasValidFormat);
>     if (keyExistsAndHasValidFormat)
>         ResourceRequest::setHTTPPipeliningEnabled(prefValue);
> 
>+    // Use WebCore scheduler when we can't use request priorities with CFNetwork.
>+    if (!ResourceRequest::resourcePrioritiesEnabled())
>+        return maximumHTTPConnectionCountPerHost;
>+
>     wkSetHTTPRequestMaximumPriority(toPlatformRequestPriority(ResourceLoadPriorityHighest));
> #if !PLATFORM(WIN)
>     // FIXME: <rdar://problem/9375609> Implement minimum fast lane priority setting on Windows
>     wkSetHTTPRequestMinimumFastLanePriority(toPlatformRequestPriority(ResourceLoadPriorityMedium));
> #endif
> 
>-    return unlimitedConnectionCount;
>+    return unlimitedRequestCount;
> }
>-    
>+
> #if PLATFORM(IOS)
> void initializeHTTPConnectionSettingsOnStartup()
> {
>Index: Source/WebCore/platform/network/mac/ResourceRequestMac.mm
>===================================================================
>--- Source/WebCore/platform/network/mac/ResourceRequestMac.mm	(revision 169900)
>+++ Source/WebCore/platform/network/mac/ResourceRequestMac.mm	(working copy)
>@@ -108,7 +108,8 @@ void ResourceRequest::doUpdateResourceRe
>         m_httpMethod = method;
>     m_allowCookies = [m_nsRequest.get() HTTPShouldHandleCookies];
> 
>-    m_priority = toResourceLoadPriority(wkGetHTTPRequestPriority([m_nsRequest.get() _CFURLRequest]));
>+    if (ResourceRequest::resourcePrioritiesEnabled())
>+        m_priority = toResourceLoadPriority(wkGetHTTPRequestPriority([m_nsRequest.get() _CFURLRequest]));
> 
>     NSDictionary *headers = [m_nsRequest.get() allHTTPHeaderFields];
>     NSEnumerator *e = [headers keyEnumerator];
>@@ -166,7 +167,8 @@ void ResourceRequest::doUpdatePlatformRe
>     if (ResourceRequest::httpPipeliningEnabled())
>         wkHTTPRequestEnablePipelining([nsRequest _CFURLRequest]);
> 
>-    wkSetHTTPRequestPriority([nsRequest _CFURLRequest], toPlatformRequestPriority(m_priority));
>+    if (ResourceRequest::resourcePrioritiesEnabled())
>+        wkSetHTTPRequestPriority([nsRequest _CFURLRequest], toPlatformRequestPriority(m_priority));
> 
>     [nsRequest setCachePolicy:(NSURLRequestCachePolicy)cachePolicy()];
>     wkCFURLRequestAllowAllPostCaching([nsRequest _CFURLRequest]);
>Index: Source/WebKit2/ChangeLog
>===================================================================
>--- Source/WebKit2/ChangeLog	(revision 169948)
>+++ Source/WebKit2/ChangeLog	(working copy)
>@@ -1,3 +1,15 @@
>+2014-06-13  Alexey Proskuryakov  <ap@apple.com>
>+
>+        REGRESSION: Mountain Lion: Gmail's "in new window" view of mail threads is very slow to load its content
>+        https://bugs.webkit.org/show_bug.cgi?id=133882
>+        <rdar://problem/17271965>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * NetworkProcess/mac/NetworkResourceLoadSchedulerMac.mm:
>+        (WebKit::NetworkResourceLoadScheduler::platformInitializeMaximumHTTPConnectionCountPerHost):
>+        Same fix as in WebCore.
>+
> 2014-06-13  Anders Carlsson  <andersca@apple.com>
> 
>         Add a HTTPHeaderMap::get overload that takes an HTTPHeaderName
>Index: Source/WebKit2/NetworkProcess/mac/NetworkResourceLoadSchedulerMac.mm
>===================================================================
>--- Source/WebKit2/NetworkProcess/mac/NetworkResourceLoadSchedulerMac.mm	(revision 169900)
>+++ Source/WebKit2/NetworkProcess/mac/NetworkResourceLoadSchedulerMac.mm	(working copy)
>@@ -40,19 +40,23 @@ namespace WebKit {
> void NetworkResourceLoadScheduler::platformInitializeMaximumHTTPConnectionCountPerHost()
> {
>     static const unsigned preferredConnectionCount = 6;
>-    static const unsigned unlimitedConnectionCount = 10000;
>+    static const unsigned unlimitedRequestCount = 10000;
> 
>-    WKInitializeMaximumHTTPConnectionCountPerHost(preferredConnectionCount);
>+    unsigned maximumHTTPConnectionCountPerHost = WKInitializeMaximumHTTPConnectionCountPerHost(preferredConnectionCount);
> 
>     Boolean keyExistsAndHasValidFormat = false;
>     Boolean prefValue = CFPreferencesGetAppBooleanValue(CFSTR("WebKitEnableHTTPPipelining"), kCFPreferencesCurrentApplication, &keyExistsAndHasValidFormat);
>     if (keyExistsAndHasValidFormat)
>         ResourceRequest::setHTTPPipeliningEnabled(prefValue);
> 
>-    WKSetHTTPRequestMaximumPriority(toPlatformRequestPriority(ResourceLoadPriorityHighest));
>-    WKSetHTTPRequestMinimumFastLanePriority(toPlatformRequestPriority(ResourceLoadPriorityMedium));
>-
>-    m_maxRequestsInFlightPerHost = unlimitedConnectionCount;
>+    if (ResourceRequest::resourcePrioritiesEnabled()) {
>+        WKSetHTTPRequestMaximumPriority(toPlatformRequestPriority(ResourceLoadPriorityHighest));
>+        WKSetHTTPRequestMinimumFastLanePriority(toPlatformRequestPriority(ResourceLoadPriorityMedium));
>+        m_maxRequestsInFlightPerHost = unlimitedRequestCount;
>+    } else {
>+        // Use WebKit scheduler when we can't use request priorities with CFNetwork.
>+        m_maxRequestsInFlightPerHost = maximumHTTPConnectionCountPerHost;
>+    }
> }
> 
> } // namespace WebKit

Flags:

beidson: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 133882: 233077 | 233080 | 233091 | 233933