[webkit-reviews] review granted: [Bug 109044] Replace ExceptionCode assertions with ASSERT_NO_EXCEPTION macro. : [Attachment 186862] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Mike West <mkwst at chromium.org>'s
request for review:
Bug 109044: Replace ExceptionCode assertions with ASSERT_NO_EXCEPTION macro.
https://bugs.webkit.org/show_bug.cgi?id=109044

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

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


I like that this cuts down on the "ec" name, and code that knows that it’s an
integer.

It’s interesting how much of this is basic DOM tree and Range operations. Makes
me think we should have internal functions for these instead of going through
the “real DOM” functions. Likely an even better solution to this problem area
for many call sites that could even possibly lead to some refactoring
opportunities or a performance boost. At some point we should use grep to come
up with stats about which functions are used most often with
ASSERT_NO_EXCEPTION and IGNORE_EXCEPTION.

> Source/WebCore/Modules/indexeddb/IDBCursor.cpp:99
> +    const AtomicString& direction = directionToString(m_direction,
ASSERT_NO_EXCEPTION);
>      return direction;

No need for the local variable any more here.

> Source/WebCore/Modules/indexeddb/IDBTransaction.cpp:125
> +    const AtomicString& mode = modeToString(m_mode, ASSERT_NO_EXCEPTION);
>      return mode;

No need for the local variable any more here.