For memory usage we can estimate the canvas memory usage as 4*width*height for each canvas. They should dwarf the memory usage so it's a good first approximation.

Comment on attachment 440954 [details] [diff] [review] Implement nsIMemoryReporter Requesting feedback from Vlad to ensure that I have given the right path. Also, I forgot to mention that we now ensure that our canvases are created with a 0x0 size - apparently that is not the default!

0x0 is the same as the default for canvas, use 1x1 if you need a small one until you figure out a real size. However, its size doesn't matter until the first time getContext is called on it, so don't worry about it if you set the right size beforehand.

(In reply to comment #3) > 0x0 is the same as the default for canvas, use 1x1 if you need a small one > until you figure out a real size. However, its size doesn't matter until the > first time getContext is called on it, so don't worry about it if you set the > right size beforehand. I think the problem is that the width and height properties aren't actually set when you make a canvas via document.createElement (or otherwise omit the width and height). nsHTMLCanvasElement returns 300x150 when those aren't set so that's what was causing me to incorrectly calculate the cache size.

Just noticed something while I was looking at this patch. I think you missed a semi-colon at the end of this line: + let canvas = this.win.win.document.createElementNS("http://www.w3.org/1999/xhtml", "canvas") While not essential, I think you probably meant to include one here anyway.

Comment on attachment 440954 [details] [diff] [review] Implement nsIMemoryReporter I can review this once it gets feedback+

Comment on attachment 440954 [details] [diff] [review] Implement nsIMemoryReporter Review of attachment 440954 [details] [diff] [review]: Seems fine with the minor changes below made. ::: browser/components/wintaskbar/WindowsPreviewPerTab.jsm @@ +718,5 @@ + memused += 4*canvas.width*canvas.height; + }); + return memused; + } + }, I'm about (in the next hour or so) to land bug 633653 which will require some changes here. First, the path will need to be "heap-used/aeropeek/cache". Well, that's assuming the memory is allocated with one of malloc/calloc/realloc/new/new[]; if not we'll need to discuss it more. Also, the description needs to be tweaked to match the wording of other descriptions -- basically it needs to start with "Memory" and end with a period. This would be fine: "Memory used to cached images of the tab contents for quick drawing of Aero Peek previews and thumbnails." How much memory do these caches take up, roughly speaking?

(In reply to comment #7) > > ::: browser/components/wintaskbar/WindowsPreviewPerTab.jsm > @@ +718,5 @@ > + memused += 4*canvas.width*canvas.height; > + }); > + return memused; > + } > + }, > > I'm about (in the next hour or so) to land bug 633653 which will require some > changes here. You can probably tell I'm still learning how Splinter inserts comments into the patch :) The comments I made are relevant to the lines a few higher up than the ones quoted.

I wonder if these might be counted elsewhere too (like in images/chrome)

(In reply to comment #9) > I wonder if these might be counted elsewhere too (like in images/chrome) Good question, we definitely don't want to double-count any memory.