Bug 102457 - IndexedDB: remove setVersion from pending-version-change-on-exit.html
Summary: IndexedDB: remove setVersion from pending-version-change-on-exit.html
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: David Grogan
URL:
Keywords:
Depends on:
Blocks: 94972
  Show dependency treegraph
 
Reported: 2012-11-15 19:19 PST by David Grogan
Modified: 2012-11-20 16:46 PST (History)
4 users (show)

See Also:


Attachments
Patch (5.40 KB, patch)
2012-11-15 19:23 PST, David Grogan
no flags Details | Formatted Diff | Diff
Patch (5.40 KB, patch)
2012-11-16 10:16 PST, David Grogan
no flags Details | Formatted Diff | Diff
Patch (5.97 KB, patch)
2012-11-19 18:13 PST, David Grogan
no flags Details | Formatted Diff | Diff
Patch (5.99 KB, patch)
2012-11-20 12:29 PST, David Grogan
no flags Details | Formatted Diff | Diff
Patch for landing (5.98 KB, patch)
2012-11-20 15:57 PST, David Grogan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Grogan 2012-11-15 19:19:02 PST
IndexedDB: remove setVersion from pending-version-change-on-exit.html
Comment 1 David Grogan 2012-11-15 19:23:04 PST
Created attachment 174591 [details]
Patch
Comment 2 David Grogan 2012-11-15 19:25:20 PST
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)
Comment 3 David Grogan 2012-11-16 10:16:13 PST
Created attachment 174716 [details]
Patch
Comment 4 Joshua Bell 2012-11-19 17:14:14 PST
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?
Comment 5 David Grogan 2012-11-19 18:09:14 PST
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.
Comment 6 David Grogan 2012-11-19 18:13:55 PST
Created attachment 175104 [details]
Patch
Comment 7 David Grogan 2012-11-19 18:15:20 PST
Josh, could you give this another look before I ask Tony to review?
Comment 8 Joshua Bell 2012-11-20 09:44:16 PST
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.
Comment 9 David Grogan 2012-11-20 12:29:11 PST
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.
Comment 10 David Grogan 2012-11-20 12:29:52 PST
Created attachment 175265 [details]
Patch
Comment 11 David Grogan 2012-11-20 12:30:14 PST
Tony, could you review this?
Comment 12 WebKit Review Bot 2012-11-20 14:25:48 PST
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
Comment 13 David Grogan 2012-11-20 15:57:30 PST
Created attachment 175296 [details]
Patch for landing
Comment 14 WebKit Review Bot 2012-11-20 16:45:57 PST
Comment on attachment 175296 [details]
Patch for landing

Clearing flags on attachment: 175296

Committed r135333: <http://trac.webkit.org/changeset/135333>
Comment 15 WebKit Review Bot 2012-11-20 16:46:02 PST
All reviewed patches have been landed.  Closing bug.