1240959 - Netmonitor total size gets very confused by maps.google.com

Status: RESOLVED FIXED

(DevTools :: Netmonitor, defect)

Description

[Brian Grinstead [:bgrins]]

Attachment: maps.google-size-netmonitor.png

STR: 
Open netmonitor
Load https://www.google.com/maps/@37.3565066,-122.0322134,13z

Expected:
Something like 1MB is shown as transferred

Actual:
While requests are streaming in you can see the size jump around from 0 to 1000kb to 0 to 4000kb etc.  Once it's done the number is way too small (see screenshot)

Comment 1

[Brian Grinstead [:bgrins]]

Attachment: netmon-maps.google.gif

Also, the 'size' value jumps around randomly when I sort by size in the table

Comment 2

[Brian Grinstead [:bgrins]]

This is pretty bad.  I've suspected for a while that the 'size' column isn't always telling me the truth and this is the most clear example I've been able to find.

Honza, can you reproduce this and help get it prioritized if so?

Comment 3

[James Long (:jlongster)]

I actually noticed this the other day on a complex site.

Comment 4

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Brian Grinstead [:bgrins] from comment #2)
> This is pretty bad.  I've suspected for a while that the 'size' column isn't
> always telling me the truth and this is the most clear example I've been
> able to find.
> 
> Honza, can you reproduce this and help get it prioritized if so?
Yes, I'll investigate it.

Brian, you are talking about the total size displayed
at the bottom/right of the panel, right?

Honza

Comment 5

[Brian Grinstead [:bgrins]]

(In reply to Jan Honza Odvarko [:Honza] from comment #4)
> (In reply to Brian Grinstead [:bgrins] from comment #2)
> > This is pretty bad.  I've suspected for a while that the 'size' column isn't
> > always telling me the truth and this is the most clear example I've been
> > able to find.
> > 
> > Honza, can you reproduce this and help get it prioritized if so?
> Yes, I'll investigate it.
> 
> Brian, you are talking about the total size displayed
> at the bottom/right of the panel, right?

Yes, exactly.  That whole 'summary' section is weird since all of the fields continually bounce between 0 and some other value as requests stream in, but this bug is specifically about that total size being wrong.

Comment 6

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Attachment: bug1240959.patch

This patch should fix the problem
- Summary calculation check the contentSize field before adding.
- Convert response.text to avoid an exception

Honza

Comment 7

[Jan Honza Odvarko [:Honza] (always need-info? me)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c8e18d593524

Comment 8

[Victor Porof [:vporof][:vp]]

Comment on attachment 8722444 [details] [diff] [review]
bug1240959.patch

Review of attachment 8722444 [details] [diff] [review]:
-----------------------------------------------------------------

r+ with comment addressed

::: devtools/client/netmonitor/netmonitor-view.js
@@ +2226,5 @@
> +
> +    let result = 0;
> +    aItemsArray.forEach(item => {
> +      let size = item.attachment.contentSize;
> +      result += (typeof size == "number") ? size : 0;

What does this change exactly? The forEach and iterative addition works exactly as before. However, the typeof conditional suggests something wrong with `contentSize`. When is that not a number? And if it isn't a number, why?

Comment 9

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Victor Porof [:vporof][:vp] from comment #8)
> What does this change exactly? The forEach and iterative addition works
> exactly as before. However, the typeof conditional suggests something wrong
> with `contentSize`.
Yes

> When is that not a number? And if it isn't a number, why?
It's undefined since it isn't set yet.

Honza

Comment 10

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 8722444 [details] [diff] [review]
bug1240959.patch

Comment addressed so, r+

Honza

Comment 11

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Attachment: bug1240959.patch

Yet updating the commit message.

Honza

Comment 12

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

There's some netmonitor failures in the try push.

Comment 13

[Jan Honza Odvarko [:Honza] (always need-info? me)]

New try push:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=83b8afb934f0

Honza

Comment 14

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/e0cc363043ad

Comment 15

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out for failures in browser_net_content-type.js (dt tests).

Backout: https://hg.mozilla.org/integration/fx-team/rev/53864eca4327

Push with failure (newer pushes also show failures in Tier 1 jobs): https://treeherder.mozilla.org/#/jobs?repo=fx-team&revision=e0cc363043ad
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=7516494&repo=fx-team

16:48:25     INFO -  134 INFO TEST-UNEXPECTED-FAIL | devtools/client/netmonitor/test/browser_net_content-type.js | Test timed out -

Comment 16

[William Lachance (:wlach)]

This generated some performance regressions before it was backed out:

https://treeherder.allizom.org/perf.html#/alerts?id=267

Comment 17

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Attachment: bug1240959.patch

Patch updated. I put back the original response.text conversion (using btoa), but kept the 'catch' clause for an exception.

Try push:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a51625ade054

Honza

Comment 18

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 8723513 [details] [diff] [review]
bug1240959.patch

Victor, the tests are passing now. I reverted the response decoding (it breaks images), but kept the try-catch block. Does that sound ok to you?
Size summary works well for me.

Honza

Comment 19

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/37ca88b45a72

Comment 20

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/37ca88b45a72

Comment 21

[Saheda Reza [:Antora]]

I have reproduced this bug according to (2016-1-19)

It's fixed on Latest Developer Edition -- Build ID (20160408004012), User Agent: Mozilla/5.0 (Windows NT 6.3; rv:47.0) Gecko/20100101 Firefox/47.0

Tested OS-- Windows8.1 32bit