1219556 - Transferred / size numbers are obviously wrong

Status: RESOLVED FIXED

(DevTools :: Netmonitor, defect)

Description

[Jeff Griffiths (:canuckistani) (:⚡︎)]

See link, and this SO question:

http://stackoverflow.com/questions/31508885/why-transferred-bytes-are-bigger-than-size-bytes-in-firefox-dev-tools?noredirect=1#comment54390454_31508885

This is just obviously wrong, probably a platform problem?

Comment 1

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

We should clarify exactly which file types are involved.

The original SO question seems to be about JS, but then we've cited an image issue here in the bug.

There is a known issue with the sizes for images and fonts, which has seen some recent activity.

Comment 2

[Jeff Griffiths (:canuckistani) (:⚡︎)]

(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #1)
> We should clarify exactly which file types are involved.
> 
> The original SO question seems to be about JS, but then we've cited an image
> issue here in the bug.

Yeah, the last comment on SO supplied that example. 

> There is a known issue with the sizes for images and fonts, which has seen
> some recent activity.

Sweet!

Comment 3

[Tom Tromey :tromey]

I still have problems with the image linked here.  The network panel says that the transferred
size is 6.41MB, but that the image is 1MB.  However, hovering in the network panel shows that the
image appears to be truncated -- in the network panel only, as I can see the full image just fine
in the browser.  So, this appears to be different from bug 937586.
I'll attach a screenshot.

Comment 4

[Tom Tromey :tromey]

Attachment: Screenshot from 2016-12-20 08-24-43.png

This shows how the image is truncated.

Comment 5

[Tom Tromey :tromey]

The truncation appears to be intentional, there's a 1MB response limit:
https://dxr.mozilla.org/mozilla-central/source/devtools/shared/webconsole/network-monitor.js#414
It's still a bug that the size is reported as 1MB though.

Comment 6

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

(In reply to Tom Tromey :tromey from comment #5)
> The truncation appears to be intentional, there's a 1MB response limit:
> https://dxr.mozilla.org/mozilla-central/source/devtools/shared/webconsole/
> network-monitor.js#414
> It's still a bug that the size is reported as 1MB though.

Right, we want to report the correct size no matter what the response limit is set to.

Comment 7

[Tom Tromey :tromey]

Attachment: Bug 1219556 - report full body size from NetworkResponseListener;

Review commit: https://reviewboard.mozilla.org/r/100814/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/100814/

Comment 8

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8821579 [details]
Bug 1219556 - report full body size from NetworkResponseListener;

https://reviewboard.mozilla.org/r/100814/#review101732

Thanks for fixing this!  It's too bad we let it sit around for so long... :S

Consider uplifting the fix to reach users more quickly.

::: devtools/client/netmonitor/test/sjs_truncate-test-server.sjs:5
(Diff revision 1)
> +/* Any copyright is dedicated to the Public Domain.
> +   http://creativecommons.org/publicdomain/zero/1.0/ */
> +
> +// This value comes from network-monitor.js.
> +const RESPONSE_BODY_LIMIT = 1048576;

Can we have `network-monitor.js` export this value and import it here?  I'd like to keep the test correct even if we change the value in the future.

Comment 9

[Tom Tromey :tromey]

Comment on attachment 8821579 [details]
Bug 1219556 - report full body size from NetworkResponseListener;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/100814/diff/1-2/

Comment 10

[Tom Tromey :tromey]

Comment on attachment 8821579 [details]
Bug 1219556 - report full body size from NetworkResponseListener;

https://reviewboard.mozilla.org/r/100814/#review101732

> Can we have `network-monitor.js` export this value and import it here?  I'd like to keep the test correct even if we change the value in the future.

We can't import it directly into a .sjs (AFAIK) but I changed it to be a request parameter instead.

Comment 11

[Tom Tromey :tromey]

Rebasing made the try results much nicer.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=df755896e76d917c993403c1c8691274923b6970

Comment 12

[Tom Tromey :tromey]

Comment on attachment 8821579 [details]
Bug 1219556 - report full body size from NetworkResponseListener;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/100814/diff/2-3/

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0bb1f11160a4
report full body size from NetworkResponseListener; r=jryans

Comment 14

[Tom Tromey :tromey]

Comment on attachment 8821579 [details]
Bug 1219556 - report full body size from NetworkResponseListener;

Approval Request Comment
[Feature/Bug causing the regression]:  Unknown.
[User impact if declined]: In some cases, the network monitor will report
incorrect sizes.  This has been noticed a couple of times, including in
a StackOverflow question.
[Is this code covered by automated tests?]: Yes.
[Has the fix been verified in Nightly?]: Not yet.
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No.
[Why is the change risky/not risky?]: It changes a single line to use the
correct value for the size.
[String changes made/needed]: N/A

Comment 15

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

I had to back this out for failures like https://treeherder.mozilla.org/logviewer.html#?job_id=65959757&repo=autoland

https://hg.mozilla.org/integration/autoland/rev/46a1b53d651b

Comment 16

[Tom Tromey :tromey]

Maybe it is just that the linux debug build is slow.
The log seems to show the test completing properly:

 15:14:18     INFO - TEST-PASS | devtools/client/netmonitor/test/browser_net_truncate.js | Item shouldn't have 'odd' class. - 
 15:14:18     INFO - Destroying the specified network monitor.
 15:14:18     INFO - Removing tab.
 15:14:18     INFO - Waiting for event: 'TabClose' on [object XULElement].
 15:14:18     INFO - Buffered messages logged at 15:14:15
 15:14:18     INFO - Got event: 'TabClose' on [object XULElement].
 15:14:18     INFO - Tab removed and finished closing
 15:14:18     INFO - finish() was called, cleaning up...
 15:14:18     INFO - Buffered messages finished
 15:14:18     INFO - TEST-UNEXPECTED-FAIL | devtools/client/netmonitor/test/browser_net_truncate.js | This test exceeded the timeout threshold. It should be rewritten or split up. If that's not possible, use requestLongerTimeout(N), but only as a last resort. - 
 15:14:18     INFO - MEMORY STAT | vsize 932MB | residentFast 359MB | heapAllocated 149MB
15:14:18 INFO - TEST-OK | devtools/client/netmonitor/test/browser_net_truncate.js | took 59882ms

Comment 17

[Tom Tromey :tromey]

Trying with a longer timeout:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5bcd7c2ca180f74fa8918cf43fd70aaaf68cd896

Comment 18

[Tom Tromey :tromey]

> Maybe it is just that the linux debug build is slow.

Also see the discussion in bug 1317053.

Comment 19

[Tom Tromey :tromey]

Comment on attachment 8821579 [details]
Bug 1219556 - report full body size from NetworkResponseListener;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/100814/diff/3-4/

Comment 20

[Tom Tromey :tromey]

I re-ran the failing test many (12) times with a longer timeout and there
were no more failures.  So, I'm going to try landing this again.

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1414b6a43029
report full body size from NetworkResponseListener; r=jryans

Comment 22

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/1414b6a43029

Comment 23

[Julien Cristau [:jcristau]]

Comment on attachment 8821579 [details]
Bug 1219556 - report full body size from NetworkResponseListener;

devtools net monitor fix, aurora52+

Comment 24

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/59fd34b22715

Comment 25

[Phil Ringnalda (:philor)]

Backed out in https://hg.mozilla.org/releases/mozilla-aurora/rev/9e93163b43f8dd2b7a2a7191dac910ae1259c0e8 for browser_net_sort-01.js failures like https://treeherder.mozilla.org/logviewer.html#?job_id=67458709&repo=mozilla-aurora

Comment 26

[Saheda Reza [:Antora]]

I reproduced this on firefox nightly according to (2015-10-28)

Fixing bug is verified on Latest Nightly-- Build ID:( 20170110030221 ),User Agent: Mozilla/5.0 (Windows NT 10.0; rv:53.0) Gecko/20100101 Firefox/53.0

Tested OS -- Windows10 32bit

Comment 27

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

It looks like this problem has been around for quite a while...  So, it doesn't have to be uplifted to 52 if that's hard for some reason, it would just be nice to have.

Comment 28

[Tom Tromey :tromey]

Attachment: Bug 1219556 - report full body size from NetworkResponseListener;

Review commit: https://reviewboard.mozilla.org/r/104280/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/104280/

Comment 29

[Tom Tromey :tromey]

The problem seems to be just the one failing test, which differed between
aurora and autoland.  This patch changes the test; after that I think we should
be ok for uplift.

Comment 30

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8826346 [details]
Bug 1219556 - report full body size from NetworkResponseListener;

https://reviewboard.mozilla.org/r/104280/#review105378

It looks pretty much the same to me, but hopefully this is right version for Aurora.

You may need to land it manually on Aurora, since the approval was set on the other patch, and it seems silly to request another approval for a nearly identical thing.

Comment 31

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/bb809441355b