Bug 112902 – [Soup] Use ResourceLoadPriority

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Sergio Villar Senin]

WebCore already provides a priority for each resource. Since libsoup uses a message queue it should be quite convenient to use that priority to sort messages in the queue and prioritize urgent requests.

See https://bugzilla.gnome.org/show_bug.cgi?id=696277 for the libsoup part of this bug.

[Sergio Villar Senin]

Created attachment 194237 [details]
This won't build because it depends on a non-committed libsoup feature. I am also not setting any libsoup guard until we decide whether or not this change gets into the next libsoup release.

[Sergio Villar Senin]

Created attachment 194239 [details]
Patch

This won't build because it depends on a non-committed libsoup feature. I am also not setting any libsoup guard until we decide whether or not this change gets into the next libsoup release.

[EFL EWS Bot]

Comment on attachment 194239 [details]
Patch

Attachment 194239 [details] did not pass efl-ews (efl):
Output: http://webkit-commit-queue.appspot.com/results/17291025

[Martin Robinson]

Comment on attachment 194239 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=194239&action=review

> Source/WebCore/platform/network/soup/ResourceRequest.h:102
> +    switch (priority) {
> +    case ResourceLoadPriorityUnresolved:

What is does unresolved mean? I'm curious, because perhaps it should have SOUP_MESSAGE_PRIORITY_NORMAL instead.

[Sergio Villar Senin]

(In reply to comment #4)
> (From update of attachment 194239 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=194239&action=review
> 
> > Source/WebCore/platform/network/soup/ResourceRequest.h:102
> > +    switch (priority) {
> > +    case ResourceLoadPriorityUnresolved:
> 
> What is does unresolved mean? I'm curious, because perhaps it should have SOUP_MESSAGE_PRIORITY_NORMAL instead.

It's basically the value used by the loader to initialize the priority, and if I understand the loader code correctly, we should never get such a priority because the resource load scheduler has ASSERTs to control that, so we might even break and ASSERT_NOT_REACHED in debug builds and return the default value (which might be NORMAL again).

[Sergio Villar Senin]

Created attachment 198484 [details]
Patch

[Sergio Villar Senin]

Adding a couple of guys who might have an opinion on the ResourceLoadPriorityUnresolved thing.

[Sergio Villar Senin]

ping

[Carlos Garcia Campos]

(In reply to comment #4)
> (From update of attachment 194239 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=194239&action=review
> 
> > Source/WebCore/platform/network/soup/ResourceRequest.h:102
> > +    switch (priority) {
> > +    case ResourceLoadPriorityUnresolved:
> 
> What is does unresolved mean? I'm curious, because perhaps it should have SOUP_MESSAGE_PRIORITY_NORMAL instead.

Yes, I agree, if the priority is unresolved I would use the default one in libsoup that is NORMAL.

[Sergio Villar Senin]

Created attachment 204421 [details]
Patch

[Carlos Garcia Campos]

Comment on attachment 204421 [details]
Patch

LGTM

[Sergio Villar Senin]

Comment on attachment 204421 [details]
Patch

Clearing flags on attachment: 204421

Committed r151493: <http://trac.webkit.org/changeset/151493>

[Sergio Villar Senin]

All reviewed patches have been landed.  Closing bug.