Bug 123757 - Subresource loads should not prevent page throttling
Summary: Subresource loads should not prevent page throttling
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Gavin Barraclough
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-11-04 14:56 PST by Gavin Barraclough
Modified: 2013-11-05 17:25 PST (History)
7 users (show)

See Also:


Attachments
Fix (10.88 KB, patch)
2013-11-04 15:01 PST, Gavin Barraclough
darin: review+
eflews.bot: commit-queue-
Details | Formatted Diff | Diff
New fix (3.46 KB, patch)
2013-11-05 13:30 PST, Gavin Barraclough
ap: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Gavin Barraclough 2013-11-04 14:56:25 PST
Some pages leave long running XHRs open, thee should not inhibit processes suspension.
Comment 1 Gavin Barraclough 2013-11-04 15:01:02 PST
Created attachment 215956 [details]
Fix
Comment 2 EFL EWS Bot 2013-11-04 15:33:01 PST
Comment on attachment 215956 [details]
Fix

Attachment 215956 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/20328760
Comment 3 EFL EWS Bot 2013-11-04 15:51:08 PST
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 4 kov's GTK+ EWS bot 2013-11-04 15:52:44 PST
Comment on attachment 215956 [details]
Fix

Attachment 215956 [details] did not pass gtk-ews (gtk):
Output: http://webkit-queues.appspot.com/results/19648854
Comment 5 Darin Adler 2013-11-04 17:19:32 PST
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.
Comment 6 Gavin Barraclough 2013-11-05 13:23:07 PST
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.
Comment 7 Gavin Barraclough 2013-11-05 13:30:33 PST
Created attachment 216069 [details]
New fix
Comment 8 Alexey Proskuryakov 2013-11-05 15:15:26 PST
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
Comment 9 Gavin Barraclough 2013-11-05 17:25:03 PST
Fixed in r158702