Comment on attachment 8645171 [details] [diff] [review] 1192335-bytesize.patch Review of attachment 8645171 [details] [diff] [review]: ----------------------------------------------------------------- Looks great! Thanks! ::: toolkit/devtools/server/tests/mochitest/test_memory_allocations_07.html @@ +35,5 @@ > + yield memory.stopRecordingAllocations(); > + > + ok(response.allocationSizes, "The response should have bytesizes."); > + is(response.allocationSizes.length, response.allocations.length, > + "There should be a bytesize for every allocation."); Let's additionally assert that the allocations length is >= 3. @@ +36,5 @@ > + > + ok(response.allocationSizes, "The response should have bytesizes."); > + is(response.allocationSizes.length, response.allocations.length, > + "There should be a bytesize for every allocation."); > + ok(response.allocationSizes.every(s => typeof s === "number"), "every bytesize is a number"); Lets assert that it is not only a number, but a positive, non-zero number. ::: toolkit/devtools/shared/memory.js @@ +289,5 @@ > > // Safe because SavedFrames are frozen/immutable. > let waived = Cu.waiveXrays(stack); > > // Ensure that we have a form, count, and index for new allocations Missed this in the last review, but "count" should be "size" now.