Closed Bug 1192128 Opened 5 years ago Closed 5 years ago
64-bit Window IDs not handled properly in DOM memory reporters
Window IDs are uint64_t but the DOM window reporters treat them like they are int32_t. This leads to stuff like this: > │ │ ├──14.15 MB (02.63%) -- window(<anonymized--2147483584>) > > │ ├──119.47 MB (11.23%) -- top(https://mail.google.com/mail/u/0/#label/A-moz%2Fbugz, id=2147483663) > > │ ├───5.81 MB (01.08%) ++ top(<anonymized--2147483647>, id=40802189313) I haven't seen this before. I don't know if something changed recently to make it show up.
I can reproduce this in one m-c build from a few days ago. But I can't reproduce it in an m-i build from today. Hmm.
It's like there was a short-lived bug whereby windows were getting IDs that didn't fit in 32-bits, or something.
If those strange WindowID values show up again, this code should handle them properly.
Attachment #8644778 - Flags: review?(continuation)
Since WindowID has process bits and process specific window ID, would it make sense to show only the process specific window ID or something. Or show process ID and process specific window ID http://mxr.mozilla.org/mozilla-central/source/dom/ipc/ContentChild.cpp?rev=ae8604b64e92#2775
(In reply to Olli Pettay [:smaug] from comment #4) > Or show process ID and process specific window ID The entry for the window will already show up in a part of the page that has the PID, so I don't think it is important to show the PID. It would be nice to just show the window ID without the masked-in PID, but that is really a separate issue.
Comment on attachment 8644778 [details] [diff] [review] In DOM memory reporter, handle WindowID() being a uint64_t Review of attachment 8644778 [details] [diff] [review]: ----------------------------------------------------------------- As far as I can see, there's no way to extract the more human-friendly process-specific window ID, so it is fine to just do this. I'll file a followup about making the output a little nicer.
Attachment #8644778 - Flags: review?(continuation) → review+
I filed bug 1192298 for that.
You need to log in before you can comment on or make changes to this bug.