Closed Bug 1382025 Opened 4 years ago Closed 4 years ago

'Transferred' size in Network panel does not include HTTP headers

Categories

(DevTools :: Netmonitor, defect)

56 Branch
defect
Not set
normal

Tracking

(firefox57 fixed)

RESOLVED FIXED
Firefox 57
Tracking Status
firefox57 --- fixed

People

(Reporter: sime.vidas, Assigned: vi.le)

Details

Attachments

(1 file)

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:56.0) Gecko/20100101 Firefox/56.0
Build ID: 20170718030207

Steps to reproduce:

1. Open http://jlwagner.net/tests/sprite-vs-nosprite/nosprite.html which loads 223 individual SVG assets
2. Check the 'transferred' size in the Network panel


Actual results:

The 'transferred' size should be ~108 KB, like in Chrome DevTools.


Expected results:

The 'transferred' size is 36.53 KB. The difference to Chrome is due to the HTTP headers -- about 320 B per asset -- which count toward the 'transferred' size in Chrome, but not in Firefox.

I think, Firefox should show the total transferred size, because that's what developers should care about -- they need to know how many KBs users on metered connections spend to load their site -- and also because 'transferred' meaning 'transferred content' is misleading.
Component: Untriaged → Developer Tools: Netmonitor
I think I've found where the bug is: https://dxr.mozilla.org/mozilla-central/source/devtools/server/actors/webconsole.js#2299

We use "content.transferredSize" which is the "Response body size on the wire" according to https://dxr.mozilla.org/mozilla-central/source/devtools/shared/webconsole/network-monitor.js#369. One possible fix to this issue is to take into account the header size in webconsole.js, for transferredSize and contentSize, but I'm not sure if this is the best way.

Vincent
Thanks for finding. I'm not familiar with the code, but I've found a "headersSize" property in webconsole.js. Could this be added to "transferredSize" to get the full size? Hopefully, that would match what Chrome DevTools reports in the Size column.

Please consider discussing this with a few colleges in the Developer Tools team. To re-iterate my point, I think web developers are most interested in how much KB of data a resource spends in total, since they want to optimize their sites for users on metered connections. The information in my original post (above) shows that HTTP headers can take up a significant amount of KB, especially in HTTP/2 scenarios where there are lots of smaller requests. This information should not be ignored by Firefox Developer Tools.
(In reply to Šime Vidas from comment #2)
> Thanks for finding. I'm not familiar with the code, but I've found a
> "headersSize" property in webconsole.js. Could this be added to
> "transferredSize" to get the full size? Hopefully, that would match what
> Chrome DevTools reports in the Size column.

Yes exactly. We can also add the header size to transferredSize in other places.
Doing so results in a total transferred size of 107.43KB.

> Please consider discussing this with a few colleges in the Developer Tools
> team. To re-iterate my point, I think web developers are most interested in
> how much KB of data a resource spends in total, since they want to optimize
> their sites for users on metered connections. The information in my original
> post (above) shows that HTTP headers can take up a significant amount of KB,
> especially in HTTP/2 scenarios where there are lots of smaller requests.
> This information should not be ignored by Firefox Developer Tools.

Yes I agree with you. Although after reading again my previous comment we should not do the same for contentSize.
Assignee: nobody → vi.le
Comment on attachment 8891733 [details]
Bug 1382025 - Include the headers size in the netmonitor 'Transferred' column;

https://reviewboard.mozilla.org/r/162792/#review168794

Thank you for the patch.  I agree this is a good thing to do.  Also I checked MDN just to make sure that "transferred" wasn't somehow documented as not including the header size (the language is sufficiently vague, so I think it doesn't really need updating for this patch).

I found one nit.  The patch is ok with this fixed.

I was wondering if the header sizes in the tests are stable across platforms, but a try run can decide that.  If you can't push to try (I don't know or know how to check), let me know and I will do it.

::: devtools/client/netmonitor/test/browser_net_sort-01.js:170
(Diff revision 1)
>            fuzzyUrl: true,
>            status: 101,
>            statusText: "Meh",
>            type: "1",
>            fullMimeType: "text/1",
> -          transferred: L10N.getStr("networkMenu.sizeUnavailable"),
> +          size: L10N.getFormatStrWithNumbers("networkMenu.sizeB", 198),

This hunk changes "transferred" to "size", resulting in "size" being mentioned twice.  I suspect it should stay as "transferred".
Attachment #8891733 - Flags: review?(ttromey) → review+
Comment on attachment 8891733 [details]
Bug 1382025 - Include the headers size in the netmonitor 'Transferred' column;

https://reviewboard.mozilla.org/r/162792/#review168924

::: devtools/client/netmonitor/test/browser_net_sort-01.js:170
(Diff revision 1)
>            fuzzyUrl: true,
>            status: 101,
>            statusText: "Meh",
>            type: "1",
>            fullMimeType: "text/1",
> -          transferred: L10N.getStr("networkMenu.sizeUnavailable"),
> +          size: L10N.getFormatStrWithNumbers("networkMenu.sizeB", 198),

Fixed, Thanks!
Comment on attachment 8891733 [details]
Bug 1382025 - Include the headers size in the netmonitor 'Transferred' column;

https://reviewboard.mozilla.org/r/162792/#review168794

I've tested with several user agents and the headers size seems to be consistent. I've never lauched a try run. Is it the right command?

    ./mach try -b do -p all -u mochitest-dt -t none
(In reply to Vincent Lequertier from comment #8)
> Comment on attachment 8891733 [details]
> Bug 1382025 - Include the headers size in the netmonitor 'Transferred'
> column;
> 
> https://reviewboard.mozilla.org/r/162792/#review168794
> 
> I've tested with several user agents and the headers size seems to be
> consistent. I've never lauched a try run. Is it the right command?
> 
>     ./mach try -b do -p all -u mochitest-dt -t none

Yes, that seems fine; but you can do it directly from reviewboard.
I see one lint error on the try push:

TEST-UNEXPECTED-ERROR | /home/worker/checkouts/gecko/devtools/client/netmonitor/test/browser_net_filter-flags.js:267:1 | More than 1 blank line not allowed. (no-multiple-empty-lines)

The other errors aren't related to my patch, right?
(In reply to Vincent Lequertier from comment #11)
> I see one lint error on the try push:
> 
> TEST-UNEXPECTED-ERROR |
> /home/worker/checkouts/gecko/devtools/client/netmonitor/test/
> browser_net_filter-flags.js:267:1 | More than 1 blank line not allowed.
> (no-multiple-empty-lines)
> 
> The other errors aren't related to my patch, right?

I don't believe they are related.

FWIW, to get an answer, it's best to use the "Need more information" button.
Otherwise the question tends to get lost.
I've fixed the issue with the unnecessary blank lines and pushed to try again : https://treeherder.mozilla.org/#/jobs?repo=try&revision=fe68679cc0a10908fa81fffaeda4a2b46a82a1f3
No linting errors now :-) Tromey, do you see any issues with this try push?
Flags: needinfo?(ttromey)
Looks good.  I went ahead and landed it.
Flags: needinfo?(ttromey)
Pushed by ttromey@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/448569fa56e5
Include the headers size in the netmonitor 'Transferred' column; r=tromey
Thank you for the help on this bug :-)
https://hg.mozilla.org/mozilla-central/rev/448569fa56e5
Status: UNCONFIRMED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.