[webkit-reviews] review granted: [Bug 56344] Leaks Viewer should show how many bytes were leaked, not just how many allocations : [Attachment 86139] Show the number of leaked bytes, not just leaked allocations, in Leaks Viewer

[bugzilla-daemon at webkit.org]

Joseph Pecoraro <joepeck at webkit.org> has granted Adam Roben (:aroben)
<aroben at apple.com>'s request for review:
Bug 56344: Leaks Viewer should show how many bytes were leaked, not just how
many allocations
https://bugs.webkit.org/show_bug.cgi?id=56344

Attachment 86139: Show the number of leaked bytes, not just leaked allocations,
in Leaks Viewer
https://bugs.webkit.org/attachment.cgi?id=86139&action=review

------- Additional Comments from Joseph Pecoraro <joepeck at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=86139&action=review

r=me, Nice!

>
Tools/BuildSlaveSupport/build.webkit.org-config/public_html/LeaksViewer/LeaksPa
rserWorker.js:39
> +	       var match = /Leak:.*\ssize=(\d+)\s/.exec(line);

Nit: It might be worth adding a start anchor? /^Leak.../
I don't think you'd ever get a false positive with the '=' string, but it might
help matching against long lines.

>
Tools/BuildSlaveSupport/build.webkit.org-config/public_html/LeaksViewer/LeaksPa
rserWorker.js:88
> +		       childNode.selfTime += leak.size;
> +		   childNode.totalTime += leak.size;
> +		   ++childNode.numberOfCalls;

These property names no longer make sense, and could be renamed
in the Worker code and converted later. But I understand your concerns
about performance. So I'm okay with these.