Bug 52886 - Web Inspector: size is wrong for cached resources in Network panel
Summary: Web Inspector: size is wrong for cached resources in Network panel
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Andrey Kosyakov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-01-21 04:47 PST by Andrey Kosyakov
Modified: 2011-01-26 09:00 PST (History)
13 users (show)

See Also:


Attachments
patch (8.55 KB, patch)
2011-01-25 10:11 PST, Andrey Kosyakov
pfeldman: review-
Details | Formatted Diff | Diff
patch (6.02 KB, patch)
2011-01-26 03:43 PST, Andrey Kosyakov
pfeldman: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andrey Kosyakov 2011-01-21 04:47:18 PST
The numbers displayed in Size column of Network panel are wrong for cached resources:
- Size should be the actual size of cached resource (currently 0)
- Transfer Size should be the size of response headers (currently 0)
Comment 1 Andrey Kosyakov 2011-01-25 10:11:25 PST
Created attachment 80075 [details]
patch
Comment 2 Pavel Feldman 2011-01-25 10:53:01 PST
Comment on attachment 80075 [details]
patch

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

> Source/WebCore/inspector/Inspector.idl:110
> +        [notify, domain=Network] void didReceiveResponse(out long identifier, out double time, out String resourceType, out long cachedSize, out Object response);

didReceiveResponse should not special-case cached resources. It would be better to call didReceiveContentLength for cached resources instead.

> Source/WebCore/inspector/front-end/NetworkManager.js:179
> +        if (resource.revalidated)

This would go away.

> Source/WebCore/inspector/front-end/Resource.js:241
> +        var size = this.cached || this.revalidated ? 0 : Number(this.resourceSize || this.responseHeaders["Content-Length"] || 0);

Could you replace ternary operators with guards for ease of understanding?

> Source/WebCore/inspector/front-end/Resource.js:309
> +    get revalidated()

get statusIsNotModified?
Comment 3 Andrey Kosyakov 2011-01-26 03:43:33 PST
Created attachment 80181 [details]
patch

Removed additional resource size parameter to didReceiveResponse, issue additional call to didReceiveContentLength in case we got 304 and had a cached resource.
Comment 4 Pavel Feldman 2011-01-26 05:27:09 PST
Comment on attachment 80181 [details]
patch

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

> Source/WebCore/inspector/front-end/Resource.js:245
> +        var bodySize = Number(this.resourceSize || this.responseHeaders["Content-Length"] || 0);

In what cases do we use Content-Length header Is it more accurate? Was is used for gzip?
Comment 5 WebKit Review Bot 2011-01-26 08:56:47 PST
http://trac.webkit.org/changeset/76690 might have broken Leopard Intel Release (Tests)
Comment 6 Andrey Kosyakov 2011-01-26 09:00:24 PST
Manually committed r76690: http://trac.webkit.org/changeset/76690