[webkit-reviews] review denied: [Bug 20949] Catch repeated messages in Inspector Controller to limit memory usage : [Attachment 23668] new patch with test

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Sep 22 14:37:08 PDT 2008


Timothy Hatcher <timothy at hatcher.name> has denied 's request for review:
Bug 20949: Catch repeated messages in Inspector Controller to limit memory
usage
https://bugs.webkit.org/show_bug.cgi?id=20949

Attachment 23668: new patch with test
https://bugs.webkit.org/attachment.cgi?id=23668&action=edit

------- Additional Comments from Timothy Hatcher <timothy at hatcher.name>
+	     var previousMessageElement = this.currentGroup.messagesElement;
+	    
previousMessageElement.removeChild(previousMessageElement.lastChild);
+	     previousMessageElement.appendChild(msg.toMessageElement());

previousMessageElement is not the right name for that variable, messagesElement
is a better name.

+	     msg.repeatCount -= this.previousMessage.repeatCount;

This line deserves a comment.

Even better would be to assign the difference of the repeat count to a new
property on the msg, so clients know they are getting the difference. Better
yet, just store it in a loval variable, and pass that number to
addMessageToResource. In addMessageToResource if the number is > 0 then do the
repeat path.

-	     msg.resource = WebInspector.resourceURLMap[msg.url];
+	     msg.resource = WebInspector.resourceURLMap[msg.url]; 

Don't land the extra space at the end of that line.

r- until some of these are fixed.


More information about the webkit-reviews mailing list