Bug 71313 – IndexedDB: Object stores not persisting between sessions

• Component: New Bugs
• Status: RESOLVED FIXED

[Joshua Bell]

IndexedDB: Object stores not persisting between sessions

[Joshua Bell]

Created attachment 113207 [details]
Patch

LGTM, thanks

[Tony Chang]

Comment on attachment 113207 [details]
Patch

Is it possible to write a layout test for this?

[David Grogan]

Comment on attachment 113207 [details]
Patch

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

> Source/WebCore/storage/IDBDatabaseBackendImpl.cpp:112
> +    ASSERT(success == (m_id != InvalidId));

Would the following work?  The only advantage is that it skips the call to loadObjectStores that I just asked you about IRL.

if (success) {
  loadObjectStores();
  return;
}
if (!m_backingStore->createIDB.....)
  ASSERT_NOT_REACHED();

[Joshua Bell]

Created attachment 113227 [details]
Patch

[Joshua Bell]

Created attachment 113235 [details]
persistence layout test (doesn't detect bug)

Logically, the attached layout test should work, but it doesn't detect the bug. The IDBDatabaseBackendImpl is apparently persisting in the IDBFactoryBackendImpl's m_databaseBackendMap across the page load. If there's a way to clear out the script execution context, we can try that. Suggestions welcome.

We should investigate why the impl isn't being torn down as promptly as expected.

(In reply to comment #6)
> Created an attachment (id=113235) [details]
> persistence layout test (doesn't detect bug)
> 
> Logically, the attached layout test should work, but it doesn't detect the bug. The IDBDatabaseBackendImpl is apparently persisting in the IDBFactoryBackendImpl's m_databaseBackendMap across the page load. If there's a way to clear out the script execution context, we can try that. Suggestions welcome.

You could redirect through another security origin, but that would require an http test (localhost -> 127.0.0.01 -> localhost)

> 
> We should investigate why the impl isn't being torn down as promptly as expected.

I also wonder whether it's possible to invoke open() or createObjectStore() after calling close on the IDBDatabase

[Joshua Bell]

(In reply to comment #3)
> (From update of attachment 113207 [details])
> Is it possible to write a layout test for this?

tony@ - I believe the answer is "no" without further work, which we should do, but IMHO without holding up this fix. The issue was caught by a Chromium pyauto test (see http://crbug.com/102537), which is how it was detected (but missed on the webkit side).

r?/cq?

[Tony Chang]

Comment on attachment 113227 [details]
Patch

OK, but some things to investigate.

(In reply to comment #7)
> (In reply to comment #6)
> > Created an attachment (id=113235) [details] [details]
> > persistence layout test (doesn't detect bug)
> > 
> > Logically, the attached layout test should work, but it doesn't detect the bug. The IDBDatabaseBackendImpl is apparently persisting in the IDBFactoryBackendImpl's m_databaseBackendMap across the page load. If there's a way to clear out the script execution context, we can try that. Suggestions welcome.
> 
> You could redirect through another security origin, but that would require an http test (localhost -> 127.0.0.01 -> localhost)

Does it work to navigate to a different file:/// URL and back?

> > We should investigate why the impl isn't being torn down as promptly as expected.
> 
> I also wonder whether it's possible to invoke open() or createObjectStore() after calling close on the IDBDatabase

You should also feel free to add methods to layoutTestController to enable more testing.

[WebKit Review Bot]

Comment on attachment 113227 [details]
Patch

Clearing flags on attachment: 113227

Committed r99218: <http://trac.webkit.org/changeset/99218>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Joshua Bell]

(In reply to comment #6)
> Created an attachment (id=113235) [details]
> persistence layout test (doesn't detect bug)
> 
> Logically, the attached layout test should work, but it doesn't detect the bug. The IDBDatabaseBackendImpl is apparently persisting in the IDBFactoryBackendImpl's m_databaseBackendMap across the page load. If there's a way to clear out the script execution context, we can try that. Suggestions welcome.
> 
> We should investigate why the impl isn't being torn down as promptly as expected.

FYI, this specific issue should be fixed with: https://bugs.webkit.org/show_bug.cgi?id=72066