Bug 102457 – IndexedDB: remove setVersion from pending-version-change-on-exit.html

• Component: New Bugs
• Status: RESOLVED FIXED

[David Grogan]

IndexedDB: remove setVersion from pending-version-change-on-exit.html

[David Grogan]

Created attachment 174591 [details]
Patch

[David Grogan]

Josh, could you take a look at this?  There's a chromium bug about it:
http://code.google.com/p/chromium/issues/detail?id=161428
Let me know if you have any ideas.

This test still passes in content_browsertests, though it is flaky there for a different reason (http://code.google.com/p/chromium/issues/detail?id=159856)

[David Grogan]

Created attachment 174716 [details]
Patch

[Joshua Bell]

Comment on attachment 174716 [details]
Patch

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

> LayoutTests/ChangeLog:8
> +        This test hangs in single process mode for unknown reasons.

Related to wkbug.com/82776 ?

> LayoutTests/storage/indexeddb/pending-version-change-on-exit.html:15
> +else {

Place on same line as {

> LayoutTests/storage/indexeddb/pending-version-change-on-exit.html:16
> +  indexedDBTest(prepareDatabase, startTheWorker);

If the worker is going to hard-code the dbname, should the page should hard-code it as well so that changes to indexedDBTest() don't break the test in mysterious ways?

[David Grogan]

Comment on attachment 174716 [details]
Patch

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

>> LayoutTests/ChangeLog:8
>> +        This test hangs in single process mode for unknown reasons.
> 
> Related to wkbug.com/82776 ?

Oh sheesh, of course. Added more code that makes it clear this is what's happening.

>> LayoutTests/storage/indexeddb/pending-version-change-on-exit.html:15
>> +else {
> 
> Place on same line as {

done.

>> LayoutTests/storage/indexeddb/pending-version-change-on-exit.html:16
>> +  indexedDBTest(prepareDatabase, startTheWorker);
> 
> If the worker is going to hard-code the dbname, should the page should hard-code it as well so that changes to indexedDBTest() don't break the test in mysterious ways?

Good point, changed to pass the dbname to the worker.

[David Grogan]

Created attachment 175104 [details]
Patch

[David Grogan]

Josh, could you give this another look before I ask Tony to review?

[Joshua Bell]

Comment on attachment 175104 [details]
Patch

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

lgtm

> LayoutTests/storage/indexeddb/pending-version-change-on-exit.html:32
> +  var worker = startWorker("resources/pending-version-change-on-exit.js?" + encodeURI(dbname));

You should use encodeURIComponent/decodeURIComponent here (since what's being encoded is being used as only part of a URI and you do want URI-reserved characters encoded) although given the usage here it should work correctly either way.

[David Grogan]

Comment on attachment 175104 [details]
Patch

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

>> LayoutTests/storage/indexeddb/pending-version-change-on-exit.html:32
>> +  var worker = startWorker("resources/pending-version-change-on-exit.js?" + encodeURI(dbname));
> 
> You should use encodeURIComponent/decodeURIComponent here (since what's being encoded is being used as only part of a URI and you do want URI-reserved characters encoded) although given the usage here it should work correctly either way.

Changed, thanks for the tip.

[David Grogan]

Created attachment 175265 [details]
Patch

[David Grogan]

Tony, could you review this?

[WebKit Review Bot]

Comment on attachment 175265 [details]
Patch

Rejecting attachment 175265 [details] from commit-queue.

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

Last 500 characters of output:
WebKit/chromium/v8 --revision 12947 --non-interactive --force --accept theirs-conflict --ignore-externals' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
51>At revision 12947.

________ running '/usr/bin/python tools/clang/scripts/update.py --mac-only' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'

________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
Updating webkit projects from gyp files...
Total errors found: 0 in 1 files

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

[David Grogan]

Created attachment 175296 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 175296 [details]
Patch for landing

Clearing flags on attachment: 175296

Committed r135333: <http://trac.webkit.org/changeset/135333>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.