Bug 85557 – IndexedDB: Remove all index metadata records when deleting an index

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Joshua Bell]

IndexedDB: Remove all index metadata records when deleting an index

[Joshua Bell]

Created attachment 140129 [details]
Patch

[Joshua Bell]

Without this patch, loading storage/indexeddb/delete-index-crash.html in Chromium, quitting (or loading an interstitial page), then reloading storage/indexeddb/delete-index-crash.html will hit an ASSERT:

ASSERTION FAILED: !metaDataKey.metaDataType()
../../third_party/WebKit/Source/WebCore/Modules/indexeddb/IDBLevelDBBackingStore.cpp(723) : virtual void WebCore::IDBLevelDBBackingStore::getIndexes(int64_t, int64_t, WTF::Vector<long int, 0ul>&, WTF::Vector<WTF::String, 0ul>&, WTF::Vector<WTF::String, 0ul>&, WTF::Vector<bool, 0ul>&, WTF::Vector<bool, 0ul>&)

In theory, this should repro in DumpRenderTree - it would require dropping all references to the database and forcing GC to run so the database is dropped from the backing store map. I'm not seeing that happen with a LayoutTest when using window.location to navigate to an interstitial page, although it works in Chromium.

We may be hitting something related to http://webkit.org/b/83074 or something DRT specific that's causing handles to be retained.

I can land this then land a Chromium-side test, or hold off on landing until I can determine why DRT isn't releasing handles.

[David Grogan]

Comment on attachment 140129 [details]
Patch

LGTM

Maybe we should try to get this in before the m20 freeze?  That way, in the off chance that this IS causing docs problems, they'll get the fix a release early without us having to do a merge.

[Tony Chang]

Comment on attachment 140129 [details]
Patch

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

> Source/WebCore/ChangeLog:8
> +        Reviewed by NOBODY (OOPS!).
> +
> +        No new tests. Issues does not repro in DumpRenderTree.

Can you add a sentence or two explaining why you are making the change (hitting an assert).  Please land a chromium side test after this gets rolled into chromium.  You could maybe mention that here.

> Source/WebCore/Modules/indexeddb/IDBLevelDBCoding.cpp:1093
> +    return encode(databaseId, objectStoreId, indexId, 255);

What is 255?  Should we make it a const somewhere?

[Joshua Bell]

Created attachment 140308 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 140308 [details]
Patch for landing

Rejecting attachment 140308 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1

ERROR: /mnt/git/webkit-commit-queue/Source/WebCore/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: http://queues.webkit.org/results/12565240

[Joshua Bell]

Created attachment 140310 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 140310 [details]
Patch for landing

Clearing flags on attachment: 140310

Committed r116170: <http://trac.webkit.org/changeset/116170>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.