Summary: | Add didChangePriority() to ResourceHandle | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | James Simonsen <simonjam> | ||||||
Component: | New Bugs | Assignee: | James Simonsen <simonjam> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | abarth, ap, dglazkov, fishd, jamesr, japhet, tkent+wkapi, webkit-ews, webkit.review.bot, willchan | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
James Simonsen
2013-01-25 17:25:53 PST
This is adding more layering violations to WebKit :( This was just my idea. I'm not stuck on it. What's the correct way to do it without violating layering? Basically, we'd like Chromium's network stack to know: 1. When a resource is being speculatively loaded (from the preload scanner). 2. When a preload becomes non-speculative (the parser discovers it). We need these to properly prioritize requests. Another idea I had was to have WebCore dynamically change the resource's priority. Is that cleaner? ResourceHandle is a cross-platform wrapper around platform networking library, which is not designed to have any idea about Web concepts. It is a huge layering violation that Chromium uses it as IPC injection point, and marshals more and more high level information through it. I don't know what the good way forward is, given that this has been articulated years ago, and Chromium project leaders apparently decided to stay on the wrong course. That discussion apparently occurred way before I joined. I don't know anything about how that decision was made. Regardless, any networking layer should care about this. It's important that the speculative preloads remain a lower priority than normal loads. Messing this up leads to a 5% drop in Speed Index [1] performance. No matter which network stack you're using, it needs to know the proper priority for preloads. The priority is already included in the ResourceRequest. The only pieces missing are identifying preloads and updating their priority when a preload becomes a normal load. I don't care whether that signal uses the term "preload" or just dynamically changes the priority. From my perspective, ResourceHandle seemed like the obvious place for that signal. I'd called it promotePreloadToNormalLoad(), but we could call it didChangeRequestPriority(newPriority) instead. WDYT? [1] A measure of amount of pixels rendered during page load. https://sites.google.com/a/webpagetest.org/docs/using-webpagetest/metrics/speed-index Created attachment 186184 [details]
Patch
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI. Comment on attachment 186184 [details] Patch Attachment 186184 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/16342242 Created attachment 186188 [details]
Patch
Comment on attachment 186188 [details] Patch Clearing flags on attachment: 186188 Committed r141684: <http://trac.webkit.org/changeset/141684> All reviewed patches have been landed. Closing bug. > I don't know what the good way forward is, given that this has been articulated years ago, and Chromium project leaders apparently decided to stay on the wrong course.
Instead, I would say that we disagree about which course is right.
|