Bug 109266 - Migrate ExceptionCode ASSERTs in IDB to ASSERT_NO_EXCEPTION.
Summary: Migrate ExceptionCode ASSERTs in IDB to ASSERT_NO_EXCEPTION.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mike West
URL:
Keywords:
Depends on: 109044
Blocks:
  Show dependency treegraph
 
Reported: 2013-02-08 00:21 PST by Mike West
Modified: 2013-02-08 05:06 PST (History)
5 users (show)

See Also:


Attachments
Patch (4.85 KB, patch)
2013-02-08 03:39 PST, Mike West
no flags Details | Formatted Diff | Diff
Patch (4.67 KB, patch)
2013-02-08 04:10 PST, Mike West
no flags Details | Formatted Diff | Diff
Patch (5.21 KB, patch)
2013-02-08 04:23 PST, Mike West
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mike West 2013-02-08 00:21:37 PST
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.
Comment 1 Mike West 2013-02-08 03:39:07 PST
Created attachment 187280 [details]
Patch
Comment 2 Mike West 2013-02-08 03:40:37 PST
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?
Comment 3 Mike West 2013-02-08 04:10:20 PST
Created attachment 187287 [details]
Patch
Comment 4 jochen 2013-02-08 04:14:08 PST
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?
Comment 5 Mike West 2013-02-08 04:18:16 PST
(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.
Comment 6 Mike West 2013-02-08 04:23:42 PST
Created attachment 187289 [details]
Patch
Comment 7 Mike West 2013-02-08 04:25:15 PST
(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.
Comment 8 WebKit Review Bot 2013-02-08 05:06:08 PST
Comment on attachment 187289 [details]
Patch

Clearing flags on attachment: 187289

Committed r142262: <http://trac.webkit.org/changeset/142262>
Comment 9 WebKit Review Bot 2013-02-08 05:06:13 PST
All reviewed patches have been landed.  Closing bug.