Bug 35071 - Disable loader limiting of requests per host for chromium port
Summary: Disable loader limiting of requests per host for chromium port
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-02-17 20:38 PST by William Chan
Modified: 2010-02-18 11:21 PST (History)
3 users (show)

See Also:


Attachments
Disable loader limiting of requests per host for the chromium port. (1.38 KB, patch)
2010-02-17 21:01 PST, William Chan
no flags Details | Formatted Diff | Diff
Disable webkit loader throttling of requests per host for the chromium port. (708 bytes, patch)
2010-02-17 21:03 PST, William Chan
no flags Details | Formatted Diff | Diff
Disable webkit loader limiting of requests per host for the chromium port. (1.39 KB, patch)
2010-02-17 21:07 PST, William Chan
no flags Details | Formatted Diff | Diff
Disable loader limiting of requests per host for the chromium port. (1.41 KB, patch)
2010-02-17 21:29 PST, William Chan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description William Chan 2010-02-17 20:38:20 PST
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.
Comment 1 William Chan 2010-02-17 21:01:08 PST
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(-)
Comment 2 William Chan 2010-02-17 21:03:08 PST
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(-)
Comment 3 William Chan 2010-02-17 21:07:36 PST
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(-)
Comment 4 William Chan 2010-02-17 21:13:11 PST
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.
Comment 5 WebKit Review Bot 2010-02-17 21:17:35 PST
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.
Comment 6 William Chan 2010-02-17 21:29:38 PST
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(-)
Comment 7 William Chan 2010-02-17 21:32:04 PST
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 @@
> +2010-02-17  William Chan  <willchan@chromium.org>
> +
> +        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  <simon.fraser@apple.com>
>  
>          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
Comment 8 Adam Barth 2010-02-18 00:08:21 PST
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.
Comment 9 Adam Barth 2010-02-18 00:09:15 PST
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.
Comment 10 WebKit Commit Bot 2010-02-18 01:10:59 PST
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>
Comment 11 WebKit Commit Bot 2010-02-18 01:11:03 PST
All reviewed patches have been landed.  Closing bug.
Comment 12 William Chan 2010-02-18 09:08:22 PST
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.
Comment 13 Eric Seidel (no email) 2010-02-18 11:21:07 PST
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.