Bug 96952

Summary: IndexedDB: Pending call cleanup
Product: WebKit Reporter: Joshua Bell <jsbell>
Component: New BugsAssignee: Joshua Bell <jsbell>
Status: RESOLVED FIXED    
Severity: Normal CC: alecflett, dgrogan, tony, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 96444    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch for landing none

Joshua Bell
Reported 2012-09-17 13:56:00 PDT
IndexedDB: Pending call cleanup
Attachments
Patch (14.04 KB, patch)
2012-09-17 13:57 PDT, Joshua Bell 		no flags
DetailsFormatted DiffDiff
Patch (13.99 KB, patch)
2012-09-17 15:04 PDT, Joshua Bell 		no flags
DetailsFormatted DiffDiff
Patch for landing (14.02 KB, patch)
2012-09-19 16:35 PDT, Joshua Bell 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Joshua Bell
Comment 1 2012-09-17 13:57:26 PDT
Created attachment 164452 [details] Patch
Joshua Bell
Comment 2 2012-09-17 13:58:39 PDT
dgrogan@ - this may be redundant with changes you have in progress. Is it worth landing now?
David Grogan
Comment 3 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.
David Grogan
Comment 4 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.
Joshua Bell
Comment 5 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.
Joshua Bell
Comment 6 2012-09-17 15:04:04 PDT
Created attachment 164458 [details] Patch
Joshua Bell
Comment 7 2012-09-17 15:04:27 PDT
tony@ - r?
Joshua Bell
Comment 8 2012-09-19 16:35:24 PDT
Created attachment 164801 [details] Patch for landing
WebKit Review Bot
Comment 9 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>
WebKit Review Bot
Comment 10 2012-09-19 17:12:31 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.