WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
59465
Port Mozilla's IndexedDB tests: readonly and read/write transactions
https://bugs.webkit.org/show_bug.cgi?id=59465
Summary
Port Mozilla's IndexedDB tests: readonly and read/write transactions
Mark Pilgrim (Google)
Reported
2011-04-26 09:47:19 PDT
Original test:
http://mxr.mozilla.org/mozilla2.0/source/dom/indexedDB/test/test_readonly_transactions.html?force=1
This is a port of Mozilla's IndexedDB test that tests both readonly and read/write transaction modes. WebKit passes these tests.
Attachments
patch with changelog and layouttest
(13.80 KB, patch)
2011-04-26 09:49 PDT
,
Mark Pilgrim (Google)
tony
: review+
Details
Formatted Diff
Diff
nits addressed
(14.16 KB, patch)
2011-04-26 18:30 PDT
,
Mark Pilgrim (Google)
no flags
Details
Formatted Diff
Diff
rebase webkit directory
(13.52 KB, patch)
2011-04-26 19:38 PDT
,
Mark Pilgrim (Google)
no flags
Details
Formatted Diff
Diff
nits addressed once again (regression in previous patch)
(13.52 KB, patch)
2011-04-27 08:33 PDT
,
Mark Pilgrim (Google)
no flags
Details
Formatted Diff
Diff
Show Obsolete
(3)
View All
Add attachment
proposed patch, testcase, etc.
Mark Pilgrim (Google)
Comment 1
2011-04-26 09:49:59 PDT
Created
attachment 91120
[details]
patch with changelog and layouttest
David Grogan
Comment 2
2011-04-26 16:16:24 PDT
Comment on
attachment 91120
[details]
patch with changelog and layouttest View in context:
https://bugs.webkit.org/attachment.cgi?id=91120&action=review
Substance is good, just style stuff
> third_party/WebKit/LayoutTests/storage/indexeddb/mozilla/readonly-transactions.html:47 > +function cleanDatabase()
Make more accurate/descriptive. You can probably ignore the deleteAllObjectStores calls and instead describe the other stuff they do.
> third_party/WebKit/LayoutTests/storage/indexeddb/mozilla/readonly-transactions.html:51 > + osName = "test store";
I think the Webkit Way is to abbreviate as little as possible. rename osName -> objectStoreName
> third_party/WebKit/LayoutTests/storage/indexeddb/mozilla/readwrite-transactions.html:26 > + indexedDB = evalAndLog("indexedDB = window.indexedDB || window.webkitIndexedDB || window.mozIndexedDB;");
at some point we should probably refactor some or all of this into a function that lives in resources/shared.js
> third_party/WebKit/LayoutTests/storage/indexeddb/mozilla/readwrite-transactions.html:49 > +function cleanDatabase()
same comment as usual about cleanDatabase
> third_party/WebKit/LayoutTests/storage/indexeddb/mozilla/readwrite-transactions.html:63 > + key1 = evalAndLog("key1 = event.target.result;");
move this to postAdd2, right before key1 is first used
> third_party/WebKit/LayoutTests/storage/indexeddb/mozilla/readwrite-transactions.html:72 > + key2 = evalAndLog("key2 = event.target.result;");
move this
Tony Chang
Comment 3
2011-04-26 16:19:28 PDT
Comment on
attachment 91120
[details]
patch with changelog and layouttest View in context:
https://bugs.webkit.org/attachment.cgi?id=91120&action=review
> third_party/WebKit/LayoutTests/storage/indexeddb/mozilla/readwrite-transactions.html:121 > + done();
It would be nice if on failure we still called done() so the test doesn't timeout. One way to do this is to have the onerror callback call done() before calling unexpectedErrorCallback().
Tony Chang
Comment 4
2011-04-26 16:21:00 PDT
Sorry, I didn't see David's comments. Feel free to fix those and upload a new patch.
David Grogan
Comment 5
2011-04-26 16:26:48 PDT
(In reply to
comment #3
)
> (From update of
attachment 91120
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=91120&action=review
> > > third_party/WebKit/LayoutTests/storage/indexeddb/mozilla/readwrite-transactions.html:121 > > + done(); > > It would be nice if on failure we still called done() so the test doesn't timeout. One way to do this is to have the onerror callback call done() before calling unexpectedErrorCallback().
We could have unexpectedErrorCallback call done(). Mark, I think that wouldn't change any expected results and would solve this across all of the IndexedDB tests.
Mark Pilgrim (Google)
Comment 6
2011-04-26 17:15:20 PDT
(In reply to
comment #2
)
> (From update of
attachment 91120
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=91120&action=review
> > > third_party/WebKit/LayoutTests/storage/indexeddb/mozilla/readwrite-transactions.html:63 > > + key1 = evalAndLog("key1 = event.target.result;"); > > move this to postAdd2, right before key1 is first used > > > third_party/WebKit/LayoutTests/storage/indexeddb/mozilla/readwrite-transactions.html:72 > > + key2 = evalAndLog("key2 = event.target.result;"); > > move this
I can't move these any later because they are gathering values that are dependent on the result of the previous call.
Mark Pilgrim (Google)
Comment 7
2011-04-26 17:17:04 PDT
(In reply to
comment #5
)
> (In reply to
comment #3
) > > It would be nice if on failure we still called done() so the test doesn't timeout. One way to do this is to have the onerror callback call done() before calling unexpectedErrorCallback(). > > We could have unexpectedErrorCallback call done(). Mark, I think that wouldn't change any expected results and would solve this across all of the IndexedDB tests.
I can add this to this bug's next patch.
David Grogan
Comment 8
2011-04-26 17:20:26 PDT
Comment on
attachment 91120
[details]
patch with changelog and layouttest View in context:
https://bugs.webkit.org/attachment.cgi?id=91120&action=review
>>> third_party/WebKit/LayoutTests/storage/indexeddb/mozilla/readwrite-transactions.html:63 >>> + key1 = evalAndLog("key1 = event.target.result;"); >> >> move this to postAdd2, right before key1 is first used > > I can't move these any later because they are gathering values that are dependent on the result of the previous call.
Oops, I missed that. Move requests withdrawn.
David Grogan
Comment 9
2011-04-26 17:22:54 PDT
(In reply to
comment #7
)
> (In reply to
comment #5
) > > (In reply to
comment #3
) > > > It would be nice if on failure we still called done() so the test doesn't timeout. One way to do this is to have the onerror callback call done() before calling unexpectedErrorCallback(). > > > > We could have unexpectedErrorCallback call done(). Mark, I think that wouldn't change any expected results and would solve this across all of the IndexedDB tests. > > I can add this to this bug's next patch.
Could you file a new bug instead? Don't feel obligated to make the change.
Mark Pilgrim (Google)
Comment 10
2011-04-26 18:30:30 PDT
Created
attachment 91205
[details]
nits addressed As requested, this does not include a patch to call done() from the error callback.
Mark Pilgrim (Google)
Comment 11
2011-04-26 18:47:08 PDT
(In reply to
comment #9
)
> (In reply to
comment #7
) > > (In reply to
comment #5
) > > > (In reply to
comment #3
) > > > > It would be nice if on failure we still called done() so the test doesn't timeout. One way to do this is to have the onerror callback call done() before calling unexpectedErrorCallback(). > > > > > > We could have unexpectedErrorCallback call done(). Mark, I think that wouldn't change any expected results and would solve this across all of the IndexedDB tests. > > > > I can add this to this bug's next patch. > > Could you file a new bug instead? Don't feel obligated to make the change.
Bug 59569
Mark Pilgrim (Google)
Comment 12
2011-04-26 19:38:36 PDT
Created
attachment 91214
[details]
rebase webkit directory
Mark Pilgrim (Google)
Comment 13
2011-04-27 08:33:09 PDT
Created
attachment 91287
[details]
nits addressed once again (regression in previous patch)
WebKit Commit Bot
Comment 14
2011-04-28 05:57:30 PDT
The commit-queue encountered the following flaky tests while processing
attachment 91287
[details]
: http/tests/misc/favicon-loads-with-icon-loading-override.html
bug 58412
(author:
alice.liu@apple.com
) The commit-queue is continuing to process your patch.
WebKit Commit Bot
Comment 15
2011-04-28 05:58:36 PDT
Comment on
attachment 91287
[details]
nits addressed once again (regression in previous patch) Clearing flags on attachment: 91287 Committed
r85178
: <
http://trac.webkit.org/changeset/85178
>
WebKit Commit Bot
Comment 16
2011-04-28 05:58:42 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug