[webkit-reviews] review denied: [Bug 118328] Optimize addStrackTraceIfNecessary to be faster in the case when it's not necessary : [Attachment 206553] patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Jul 12 11:51:11 PDT 2013


Geoffrey Garen <ggaren at apple.com> has denied Chris <chris_curtis at apple.com>'s
request for review:
Bug 118328: Optimize addStrackTraceIfNecessary to be faster in the case when
it's not necessary
https://bugs.webkit.org/show_bug.cgi?id=118328

Attachment 206553: patch
https://bugs.webkit.org/attachment.cgi?id=206553&action=review

------- Additional Comments from Geoffrey Garen <ggaren at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=206553&action=review


> LayoutTests/ChangeLog:9
> +	   These results also match Chrome and Firfox's resutls on this test.

Typo: resutls => results.

> LayoutTests/inspector/console/console-exception-stack-traces.html:101
> +		   if (errorStackTrace.length - traceStackTrace.length == 1 &&
(errorStackTrace[0].functionName == "decodeURI" ||
errorStackTrace[0].functionName == "eval"))
> +		       ;
> +		   else
> +		       hadStackTraceDifference = true;

Empty statements can be hard to read. You should reverse the direction of this
branch to avoid an empty statement:

if (errorStackTrace.length - traceStackTrace.length != 1 ||
(errorStackTrace[0].functionName != "decodeURI" &&
errorStackTrace[0].functionName != "eval")
    hadStackTraceDifference = true;


More information about the webkit-reviews mailing list