[Webkit-unassigned] [Bug 144326] ResourceLoadPriority should be enum class

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Apr 29 09:51:03 PDT 2015


https://bugs.webkit.org/show_bug.cgi?id=144326

--- Comment #5 from Antti Koivisto <koivisto at iki.fi> ---

> > Source/WebCore/loader/ResourceLoadScheduler.cpp:377
> > -    for (int priority = ResourceLoadPriorityHighest; priority >= ResourceLoadPriorityLowest; --priority) {  
> > -        RequestQueue::iterator end = m_requestsPending[priority].end();
> > -        for (RequestQueue::iterator it = m_requestsPending[priority].begin(); it != end; ++it) {
> > +    for (auto& requestQueue : m_requestsPending) {
> 
> This now goes from lowest to highest priority. But the old code went from
> highest to lowest. Is that an intentional change? Is it OK?
> 
> The fact that this goes in a particular priority order is now much less
> clear than in the old code. I think this needs a comment, and maybe even an
> assertion.

It is intentional change, the order doesn't matter (it is a search and there is maximum one hit). 

I don't know why the old code used reverse order.


> It’s a little strange that we have Vector::find but we don’t have
> Deque::find, so we have to write out this loop. I suggest a helper function
> or adding something to Deque. Alternatively, I always wonder when I see a
> loop like this if maybe we are using the wrong data structure. Perhaps this
> should be a ListHashSet instead of a Deque?

I think that is a decent argument for not having that helper on Deque (or Vector). This should probably be a ListHashSet.

> > Source/WebKit2/WebProcess/InjectedBundle/InjectedBundle.cpp:640
> >  void InjectedBundle::dispatchPendingLoadRequests()
> >  {
> > -    resourceLoadScheduler()->servePendingRequests();
> >  }
> 
> Why can’t we remove this entirely? If this is unused it seems we should be
> removing more. If it is used, then we shouldn’t be removing it.

This is part of the bundle API. Need to get rid of clients (if any before it can be removed).

> Same question. Can we remove this entirely?a

These override virtual functions with implementations. We want these functions to do nothing. 

We should really get rid of the whole thing where we have WebResourceLoadScheduler inheriting WebCore::WebResourceLoadScheduler.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20150429/983937ad/attachment.html>


More information about the webkit-unassigned mailing list