Summary: | IndexedDB: Pending call cleanup | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Joshua Bell <jsbell> | ||||||||
Component: | New Bugs | Assignee: | 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
Joshua Bell
2012-09-17 13:56:00 PDT
Created attachment 164452 [details]
Patch
dgrogan@ - this may be redundant with changes you have in progress. Is it worth landing now? 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. (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. (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. Created attachment 164458 [details]
Patch
tony@ - r? Created attachment 164801 [details]
Patch for landing
Comment on attachment 164801 [details] Patch for landing Clearing flags on attachment: 164801 Committed r129076: <http://trac.webkit.org/changeset/129076> All reviewed patches have been landed. Closing bug. |