Closed Bug 1219556 Opened 5 years ago Closed 4 years ago

Transferred / size numbers are obviously wrong

Categories

(DevTools :: Netmonitor, defect)

defect
Not set
normal

Tracking

(firefox44 affected, firefox52 fixed, firefox53 fixed)

RESOLVED FIXED
Firefox 53
Tracking Status
firefox44 --- affected
firefox52 --- fixed
firefox53 --- fixed

People

(Reporter: canuckistani, Assigned: tromey)

References

()

Details

Attachments

(3 files)

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.
See Also: → 937586
(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!
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.
This shows how the image is truncated.
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.
(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.
Assignee: nobody → ttromey
Status: NEW → ASSIGNED
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.
Attachment #8821579 - Flags: review?(jryans) → review+
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.
Pushed by ttromey@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0bb1f11160a4
report full body size from NetworkResponseListener; r=jryans
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
Attachment #8821579 - Flags: approval-mozilla-aurora?
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
Flags: needinfo?(ttromey)
> Maybe it is just that the linux debug build is slow.

Also see the discussion in bug 1317053.
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.
Pushed by ttromey@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1414b6a43029
report full body size from NetworkResponseListener; r=jryans
https://hg.mozilla.org/mozilla-central/rev/1414b6a43029
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
Comment on attachment 8821579 [details]
Bug 1219556 - report full body size from NetworkResponseListener;

devtools net monitor fix, aurora52+
Attachment #8821579 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Flags: needinfo?(ttromey)
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
QA Whiteboard: [bugday-20170111]
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.
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.
Flags: needinfo?(ttromey)
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.
Attachment #8826346 - Flags: review?(jryans) → review+
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.