Summary: | Subresource loads should not prevent page throttling | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Gavin Barraclough <barraclough> | ||||||
Component: | WebCore Misc. | Assignee: | Gavin Barraclough <barraclough> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | commit-queue, eflews.bot, gtk-ews, gyuyoung.kim, japhet, rego+ews, xan.lopez | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Gavin Barraclough
2013-11-04 14:56:25 PST
Created attachment 215956 [details]
Fix
Comment on attachment 215956 [details] Fix Attachment 215956 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/20328760 Comment on attachment 215956 [details] Fix Attachment 215956 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/19598874 Comment on attachment 215956 [details] Fix Attachment 215956 [details] did not pass gtk-ews (gtk): Output: http://webkit-queues.appspot.com/results/19648854 Comment on attachment 215956 [details] Fix View in context: https://bugs.webkit.org/attachment.cgi?id=215956&action=review Patch looks fine, but seems you missed call sites in the GTK and EFL ports. > Source/WebCore/loader/ResourceLoaderOptions.h:82 > + ResourceLoaderOptions(SendCallbackPolicy sendLoadCallbacks, ContentSniffingPolicy sniffContent, DataBufferingPolicy dataBufferingPolicy, StoredCredentials allowCredentials, ClientCredentialPolicy credentialPolicy, SecurityCheckPolicy securityCheck, RequestOriginPolicy requestOriginPolicy, RequestIsXMLHTTP requestIsXMLHTTP) Good that this argument is an enum. > Source/WebCore/loader/ResourceLoaderOptions.h:100 > + RequestIsXMLHTTP requestIsXMLHTTP; But seems to me this should be a bool. Maciej doesn't like treating XHRs differently from other requests, and has suggested simplifying the page-is-loading check to simply be frame loading + hysteresis. New patch forthcoming. Created attachment 216069 [details]
New fix
Comment on attachment 216069 [details]
New fix
Looks like we should question whether we should take an activity assertion during frame loads either. If we don't need them for subresources, maybe we don't need it for any loads?
r=me
|