IndexedDB: refactor basics layout test so that it can be run on workers.
Created attachment 123247 [details] Patch
Comment on attachment 123247 [details] Patch Attachment 123247 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11298324 New failing tests: storage/indexeddb/basics-workers.html
Created attachment 123374 [details] Patch
Created attachment 123380 [details] Patch
Created attachment 123383 [details] Patch
Josh, could you take a look at this? https://chromiumcodereview.appspot.com/9269021 runs this as a ui test. I'm going to convert more (all of the?) layout tests to run on workers after taking a look at integrating idb-worker-common.js and fs-worker-test-util.js into js-test-pre.js.
Comment on attachment 123383 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=123383&action=review overall lgtm, with a few questions > LayoutTests/storage/indexeddb/basics-workers.html:11 > +<script src="../../fast/js/resources/js-test-post.js"></script> We were removing js-test-post.js from other IDB layout tests - is it still necessary here? > LayoutTests/storage/indexeddb/basics.html:10 > +<script src="../../fast/js/resources/js-test-post.js"></script> We were removing js-test-post.js from other IDB layout tests - is it still necessary here? > LayoutTests/storage/indexeddb/resources/basics.js:2 > + window = self; I assume this is necessary because some of the shared code (from fast/js?) refers to window? If so, perhaps add a FIXME (here) to change those references to "self" if they're truly agnostic between running in a window and in a worker. If it's only code in this test (e.g. "'webkitIndexedDB' in window") then I'd just change them to self (which should be safe either way...) > LayoutTests/storage/indexeddb/resources/basics.js:12 > + shouldBeTrue("'webkitIndexedDB' in window"); s/window/self/ per above? > LayoutTests/storage/indexeddb/resources/basics.js:15 > + shouldBeTrue("'webkitIDBCursor' in window"); s/window/self/ per above?
Comment on attachment 123383 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=123383&action=review >> LayoutTests/storage/indexeddb/basics-workers.html:11 >> +<script src="../../fast/js/resources/js-test-post.js"></script> > > We were removing js-test-post.js from other IDB layout tests - is it still necessary here? Yeah, it calls layoutTestController.waitUntilDone() when jsTestIsAsync == true. >> LayoutTests/storage/indexeddb/resources/basics.js:2 >> + window = self; > > I assume this is necessary because some of the shared code (from fast/js?) refers to window? If so, perhaps add a FIXME (here) to change those references to "self" if they're truly agnostic between running in a window and in a worker. > > If it's only code in this test (e.g. "'webkitIndexedDB' in window") then I'd just change them to self (which should be safe either way...) Good idea. FIXME added. >> LayoutTests/storage/indexeddb/resources/basics.js:12 >> + shouldBeTrue("'webkitIndexedDB' in window"); > > s/window/self/ per above? Done. >> LayoutTests/storage/indexeddb/resources/basics.js:15 >> + shouldBeTrue("'webkitIDBCursor' in window"); > > s/window/self/ per above? Done.
Created attachment 124414 [details] Patch
Created attachment 124422 [details] Patch
Tony, could you review this?
Comment on attachment 124422 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=124422&action=review > LayoutTests/storage/indexeddb/resources/basics.js:2 > +if (this.importScripts) { > + // FIXME: Change js-test-pre.js to use self in place of window where Should this file go in resources or script-tests? If this file is only used by basics.html, then it probably should go in script-tests. > LayoutTests/storage/indexeddb/resources/idb-worker-common.js:1 > +function debug(message) Is this file used or is it just used when run in Chrome?
Comment on attachment 124422 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=124422&action=review >> LayoutTests/storage/indexeddb/resources/basics.js:2 >> + // FIXME: Change js-test-pre.js to use self in place of window where > > Should this file go in resources or script-tests? If this file is only used by basics.html, then it probably should go in script-tests. Also in basics-workers.html. Should I still put it in script-tests? I thought script-tests/make-script-test-wrappers was obsolete for tests outside of fast/js. >> LayoutTests/storage/indexeddb/resources/idb-worker-common.js:1 >> +function debug(message) > > Is this file used or is it just used when run in Chrome? It's used in DRT as well. DRT started running dedicated workers sometime in December.
Comment on attachment 124422 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=124422&action=review LGTM >>> LayoutTests/storage/indexeddb/resources/basics.js:2 >>> + // FIXME: Change js-test-pre.js to use self in place of window where >> >> Should this file go in resources or script-tests? If this file is only used by basics.html, then it probably should go in script-tests. > > Also in basics-workers.html. Should I still put it in script-tests? I thought script-tests/make-script-test-wrappers was obsolete for tests outside of fast/js. Ah, you can keep it here since it's used by 2 files.
Comment on attachment 124422 [details] Patch Clearing flags on attachment: 124422 Committed r106392: <http://trac.webkit.org/changeset/106392>
All reviewed patches have been landed. Closing bug.