Bug 96952 - IndexedDB: Pending call cleanup
Summary: IndexedDB: Pending call cleanup
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Joshua Bell
URL:
Keywords:
Depends on: 96444
Blocks:
  Show dependency tree / graph
 
Reported: 2012-09-17 13:56 PDT by Joshua Bell
Modified: 2012-09-19 17:12 PDT (History)
4 users (show)

See Also:

Attachments
Patch (14.04 KB, patch)
2012-09-17 13:57 PDT, Joshua Bell 		no flags Details | Formatted Diff | Diff
Patch (13.99 KB, patch)
2012-09-17 15:04 PDT, Joshua Bell 		no flags Details | Formatted Diff | Diff
Patch for landing (14.02 KB, patch)
2012-09-19 16:35 PDT, Joshua Bell 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Joshua Bell 2012-09-17 13:56:00 PDT 
IndexedDB: Pending call cleanup
Comment 1 Joshua Bell 2012-09-17 13:57:26 PDT 
Created attachment 164452 [details]
Patch
Comment 2 Joshua Bell 2012-09-17 13:58:39 PDT 
dgrogan@ - this may be redundant with changes you have in progress. Is it worth landing now?
Comment 3 David Grogan 2012-09-17 14:43:02 PDT 
Comment on attachment 164452 [details]
Patch

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

LGTM

I haven't done this locally yet, no. This looks great. I'd expect that this would make the already-asserting layout test from bug 96947 hit ASSERT_WITH_MESSAGE(!m_pendingSecondHalfOpenWithVersion, "m_pendingSecondHalfOpenWithVersion not null").  Have you run it?

> Source/WebCore/Modules/indexeddb/IDBDatabaseBackendImpl.cpp:489
> +    ASSERT_WITH_MESSAGE(!m_pendingSecondHalfOpenWithVersion, "m_pendingSecondHalfOpenWithVersion not null");

I'd say change this to plain ASSERT.
Comment 4 David Grogan 2012-09-17 14:47:14 PDT 
(In reply to comment #3)
> (From update of attachment 164452 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=164452&action=review
> 
> LGTM
> 
> I haven't done this locally yet, no. This looks great. I'd expect that this would make the already-asserting layout test from bug 96947 hit ASSERT_WITH_MESSAGE(!m_pendingSecondHalfOpenWithVersion, "m_pendingSecondHalfOpenWithVersion not null").  Have you run it?

After further thought, I do not think that would happen. This patch should be a no-behavior change.

> > Source/WebCore/Modules/indexeddb/IDBDatabaseBackendImpl.cpp:489
> > +    ASSERT_WITH_MESSAGE(!m_pendingSecondHalfOpenWithVersion, "m_pendingSecondHalfOpenWithVersion not null");
> 
> I'd say change this to plain ASSERT.
Comment 5 Joshua Bell 2012-09-17 14:52:20 PDT 
(In reply to comment #4)
> (In reply to comment #3)
> > (From update of attachment 164452 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=164452&action=review
> > 
> > LGTM
> > 
> > I haven't done this locally yet, no. This looks great. I'd expect that this would make the already-asserting layout test from bug 96947 hit ASSERT_WITH_MESSAGE(!m_pendingSecondHalfOpenWithVersion, "m_pendingSecondHalfOpenWithVersion not null").  Have you run it?
> 
> After further thought, I do not think that would happen. This patch should be a no-behavior change.

I did run it and didn't any behavior changes in webkit or chromium tests.

> > > Source/WebCore/Modules/indexeddb/IDBDatabaseBackendImpl.cpp:489
> > > +    ASSERT_WITH_MESSAGE(!m_pendingSecondHalfOpenWithVersion, "m_pendingSecondHalfOpenWithVersion not null");
> > 
> > I'd say change this to plain ASSERT.

Will do.
Comment 6 Joshua Bell 2012-09-17 15:04:04 PDT 
Created attachment 164458 [details]
Patch
Comment 7 Joshua Bell 2012-09-17 15:04:27 PDT 
tony@ - r?
Comment 8 Joshua Bell 2012-09-19 16:35:24 PDT 
Created attachment 164801 [details]
Patch for landing
Comment 9 WebKit Review Bot 2012-09-19 17:12:28 PDT 
Comment on attachment 164801 [details]
Patch for landing

Clearing flags on attachment: 164801

Committed r129076: <http://trac.webkit.org/changeset/129076>
Comment 10 WebKit Review Bot 2012-09-19 17:12:31 PDT 
All reviewed patches have been landed.  Closing bug.