Bug 109266 – Migrate ExceptionCode ASSERTs in IDB to ASSERT_NO_EXCEPTION.

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Mike West]

I attempted to land some seemingly straightforward IDB changes as part of https://bugs.webkit.org/show_bug.cgi?id=109044; they caused crashes.

I'll figure out why and fix those files in a more focused patch.

[Mike West]

Created attachment 187280 [details]
Patch

[Mike West]

ASSERT_NO_EXCEPTIONS ensures, among other things, that methods which care about the value of 'ec' initialize it to 0. IDBCursor::continueFunction didn't, but ASSERTED(!ec) anyway. It doesn't in the current patch.

Moar review, Jochen?

[Mike West]

Created attachment 187287 [details]
Patch

Comment on attachment 187287 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=187287&action=review

ok

> Source/WebCore/Modules/indexeddb/IDBCursor.cpp:220
> +    ec = 0;

why not initialize this first thing in the method?

[Mike West]

(In reply to comment #4)
> (From update of attachment 187287 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=187287&action=review
> 
> ok
> 
> > Source/WebCore/Modules/indexeddb/IDBCursor.cpp:220
> > +    ec = 0;
> 
> why not initialize this first thing in the method?

Premature optimization. I figured we only need to initialize it in the case where the above block of 'if' statements doesn't set the value and return.

[Mike West]

Created attachment 187289 [details]
Patch

[Mike West]

(In reply to comment #5)
> (In reply to comment #4)
> > (From update of attachment 187287 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=187287&action=review
> > 
> > ok
> > 
> > > Source/WebCore/Modules/indexeddb/IDBCursor.cpp:220
> > > +    ec = 0;
> > 
> > why not initialize this first thing in the method?
> 
> Premature optimization. I figured we only need to initialize it in the case where the above block of 'if' statements doesn't set the value and return.

I moved the value up to the top of the method, and noticed two other methods where the same thing was happening. I don't think it's worth a rereview, as the change is exactly what you've already reviewed, just in two additional methods.

[WebKit Review Bot]

Comment on attachment 187289 [details]
Patch

Clearing flags on attachment: 187289

Committed r142262: <http://trac.webkit.org/changeset/142262>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.