Bug 35071 – Disable loader limiting of requests per host for chromium port

• Status: RESOLVED
• Resolution: FIXED
• Component: Page Loading

[William Chan]

Chromium's network stack already limits the number of requests per host.  It's unnecessary to do it here.  We're starting to play around with SPDY where we want to have many more requests per host than just 6, so we need to remove this limit.

[William Chan]

Created attachment 48964 [details]
Disable loader limiting of requests per host for the chromium port.

 WebCore/ChangeLog                                  |   12 ++++++++++++
 .../platform/network/chromium/ResourceRequest.cpp  |    5 +++--
 2 files changed, 15 insertions(+), 2 deletions(-)

[William Chan]

Created attachment 48965 [details]
Disable webkit loader throttling of requests per host for the chromium port.

 .../platform/network/chromium/ResourceRequest.cpp  |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

[William Chan]

Created attachment 48966 [details]
Disable webkit loader limiting of requests per host for the chromium port.

 WebCore/ChangeLog                                  |   12 ++++++++++++
 .../platform/network/chromium/ResourceRequest.cpp  |    5 ++++-
 2 files changed, 16 insertions(+), 1 deletions(-)

[William Chan]

Apologies for the few retries on patch submission.  It's my first time making a webkit change and I couldn't find / figure out git-send-bugzilla at first.  Let me know if I'm doing anything else silly.

[WebKit Review Bot]

Attachment 48966 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebCore/ChangeLog:5:  Line contains tab character.  [whitespace/tab] [5]
WebCore/ChangeLog:6:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 2 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[William Chan]

Created attachment 48968 [details]
Disable loader limiting of requests per host for the chromium port.

 WebCore/ChangeLog                                  |   12 ++++++++++++
 .../platform/network/chromium/ResourceRequest.cpp  |    5 ++++-
 2 files changed, 16 insertions(+), 1 deletions(-)

[William Chan]

Comment on attachment 48966 [details]
Disable webkit loader limiting of requests per host for the chromium port.

> fbdbcc22020afdbf0d706ce1ecf1a4220931f436
> diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
> index 8f6b450..b3b6281 100644
> --- a/WebCore/ChangeLog
> +++ b/WebCore/ChangeLog
> @@ -1,3 +1,15 @@
> [phone]  William Chan  <[e-mail]>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +	https://bugs.webkit.org/show_bug.cgi?id=35071
> +	Disable loader limiting of requests per host for the chromium port.

Bleaugh, used tabs on accident.  I'll fix this.  Do I need to R- this patch?  Or should I just let a reviewer do that for me?

> +
> +        No tests because we're only changing a constant.
> +
> +        * platform/network/chromium/ResourceRequest.cpp:
> +        (WebCore::initializeMaximumHTTPConnectionCountPerHost):
> +
>  2010-02-16  Simon Fraser  <[e-mail]>
>  
>          Reviewed by Dan Bernstein.
> diff --git a/WebCore/platform/network/chromium/ResourceRequest.cpp b/WebCore/platform/network/chromium/ResourceRequest.cpp
> index 5b27c1b..016bd34 100644
> --- a/WebCore/platform/network/chromium/ResourceRequest.cpp
> +++ b/WebCore/platform/network/chromium/ResourceRequest.cpp
> @@ -31,7 +31,10 @@ namespace WebCore {
>  // This is used by the loader to control the number of issued parallel load requests. 
>  unsigned initializeMaximumHTTPConnectionCountPerHost()
>  {
> -    return 6;
> +    // The chromium network stack already handles limiting the number of
> +    // parallel requests per host, so there's no need to do it here.  Therefore,
> +    // this is set to a high value that should never be hit in practice.
> +    return 10000;
>  }
>  
>  } // namespace WebCore

[Adam Barth]

Comment on attachment 48966 [details]
Disable webkit loader limiting of requests per host for the chromium port.

As noted above, this patch as tabs and there's a non-tab version attached.

In the future, you should feel free to mark your old patches obsolete when uploading new versions.  You might be interested in trying the WebKitTools/Scripts/webkit-patch upload command, which will do that automagically for you.

[Adam Barth]

Comment on attachment 48968 [details]
Disable loader limiting of requests per host for the chromium port.

We often add tests when changing constants.  However, in this case, I don't believe this change is testable.

Thanks for the patch.

[WebKit Commit Bot]

Comment on attachment 48968 [details]
Disable loader limiting of requests per host for the chromium port.

Clearing flags on attachment: 48968

Committed r54946: <http://trac.webkit.org/changeset/54946>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[William Chan]

Thanks for the review Adam.  And sorry for the newbie mistakes.  Perhaps this is the wrong place to ask it, but how do I mark the patch as obsolete?  It's not clear to me from the interface.  Or is that the same thing setting the review flag to '-'?  Also, does webkit-patch work with git?  I didn't see webkit-patch mentioned at all in http://webkit.org/coding/contributing.html.  Is that page out of date?  Anyway, I'll try out webkit-patch on my next patch.  Thanks for the advice.

[Eric Seidel (no email)]

Yes, that page is somewhat out of date given how many people use webkit-patch these days.

You should look at webkit-patch help for more information and feel free to post patches to the contributing.html page if you see fit.