Summary: | IndexedDB createIndex should fail if name arg is null | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Mark Pilgrim (Google) <pilgrim> | ||||||||||||||
Component: | Tools / Tests | Assignee: | Nobody <webkit-unassigned> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | commit-queue, dgrogan, fishd, hans, pilgrim, tony | ||||||||||||||
Priority: | P3 | ||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||
Hardware: | All | ||||||||||||||||
OS: | All | ||||||||||||||||
Attachments: |
|
Description
Mark Pilgrim (Google)
2011-04-12 12:46:01 PDT
Root problem was in the IDL file, we need to ensure that a null DOMString is treated as null (otherwise it gets serialized as the 4-character string "null"). After that, a basic isNull() check works as expected in ::createIndex(). Created attachment 91404 [details]
Patch
Comment on attachment 91404 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=91404&action=review r+ > Source/WebCore/storage/IDBObjectStore.idl:31 > + readonly attribute [ConvertNullStringTo=Null] DOMString name; I wonder why this was left off initially. It seems to be ok though. Created attachment 91415 [details] Throw NON_TRANSIENT_ERR instead Updated patch similar to bug 58465 -- Firefox throws NON_TRANSIENT_ERR, not CONSTRAINT_ERR, when name arg is null. Comment on attachment 91415 [details]
Throw NON_TRANSIENT_ERR instead
r+
Comment on attachment 91415 [details] Throw NON_TRANSIENT_ERR instead Rejecting attachment 91415 [details] from commit-queue. Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-8', 'apply-..." exit_code: 2 Last 500 characters of output: eate-index-null-name.html patch unexpectedly ends in middle of line patch: **** malformed patch at line 69: fatal: pathspec 'LayoutTests/storage/indexeddb/mozilla/create-index-null-name.html' did not match any files Failed to git add LayoutTests/storage/indexeddb/mozilla/create-index-null-name.html. at /mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply line 438. Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--reviewer', u'Tony Chang', u'--force']" exit_code: 2 Full output: http://queues.webkit.org/results/8519429 What just happened? Comment on attachment 91415 [details]
Throw NON_TRANSIENT_ERR instead
Looks like the patch failed to apply, although I'm not sure why (it says it's malformed, but it looks fine to me).
Let's try again.
Comment on attachment 91415 [details] Throw NON_TRANSIENT_ERR instead Rejecting attachment 91415 [details] from commit-queue. Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-8', 'apply-..." exit_code: 2 Last 500 characters of output: eate-index-null-name.html patch unexpectedly ends in middle of line patch: **** malformed patch at line 69: fatal: pathspec 'LayoutTests/storage/indexeddb/mozilla/create-index-null-name.html' did not match any files Failed to git add LayoutTests/storage/indexeddb/mozilla/create-index-null-name.html. at /mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply line 438. Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--reviewer', u'Tony Chang', u'--force']" exit_code: 2 Full output: http://queues.webkit.org/results/8514702 Created attachment 91521 [details]
Patch for landing
Turns out the diff was missing an newline at the end of the file. This confused patch and prevented the diff from being applied. I've uploaded a fixed diff for landing. Comment on attachment 91521 [details]
Patch for landing
Crap, forgot to add the .html file.
Created attachment 91523 [details]
Patch for landing
No idea how that happened. Thanks, Tony. Comment on attachment 91523 [details]
Patch for landing
hmm, the expected file is wrong.
OK, let me start over. I'll upload a new patch ASAP. (In reply to comment #16) > OK, let me start over. I'll upload a new patch ASAP. Sorry, I just uploaded an incorrect expected file (I ran patch twice so the new file was duplicated). If you have a fixed diff, feel free to upload. No, I have nothing yet. I was finishing up another bug's patch. If you'd like to fix it, please do. Created attachment 91529 [details]
Patch for landing
The commit-queue encountered the following flaky tests while processing attachment 91529 [details]: animations/suspend-resume-animation.html bug 48161 (authors: cmarrin@apple.com and simon.fraser@apple.com) The commit-queue is continuing to process your patch. Comment on attachment 91529 [details] Patch for landing Clearing flags on attachment: 91529 Committed r85238: <http://trac.webkit.org/changeset/85238> All reviewed patches have been landed. Closing bug. The commit-queue encountered the following flaky tests while processing attachment 91529 [details]: animations/suspend-resume-animation.html bug 48161 (authors: cmarrin@apple.com and simon.fraser@apple.com) The commit-queue is continuing to process your patch. |