[webkit-reviews] review granted: [Bug 109295] Use IGNORE_EXCEPTION for initialized, but unused, ExceptionCodes. : [Attachment 187322] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Feb 8 09:32:47 PST 2013


Darin Adler <darin at apple.com> has granted Mike West <mkwst at chromium.org>'s
request for review:
Bug 109295: Use IGNORE_EXCEPTION for initialized, but unused, ExceptionCodes.
https://bugs.webkit.org/show_bug.cgi?id=109295

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

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=187322&action=review


I think the vast majority of these should be ASSERT_NO_EXCEPTION instead. It’s
kind of random where we are using IGNORE_EXCEPTION and where
ASSERT_NO_EXCEPTION, but using the macros makes this no worse. Adding hundreds
of assertions all at once would risk creating lots of new problems for people
doing development with debug builds, so we probably could want to do it only a
few at a time.

review+ as long as you fix the one obviously wrong one below

>> Source/WebCore/page/DOMSelection.cpp:412
>> +	    if (r->compareBoundaryPoints(Range::END_TO_START, range.get(),
IGNORE_EXCEPTION) < 1 && !IGNORE_EXCEPTION) {
> 
> it's not ignoring the exception here, right?

Absolutely right. This one is wrong!


More information about the webkit-reviews mailing list