Bug 58365 – IndexedDB createIndex should fail if name arg is null

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Mark Pilgrim (Google)]

Created attachment 89248 [details]
test case

According to http://dvcs.w3.org/hg/IndexedDB/raw-file/tip/Overview.html#widl-IDBObjectStore-createIndex , the name argument of createIndex must not be null, and objectStore.createIndex(null, null) must throw a CONSTRAINT_ERR. WebKit does not throw an exception. Layout test attached.

[Mark Pilgrim (Google)]

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().

[Mark Pilgrim (Google)]

Created attachment 91404 [details]
Patch

[David Grogan]

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.

[Mark Pilgrim (Google)]

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.

[David Grogan]

Comment on attachment 91415 [details]
Throw NON_TRANSIENT_ERR instead

r+

[WebKit Commit Bot]

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

[Mark Pilgrim (Google)]

What just happened?

[Tony Chang]

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.

[WebKit Commit Bot]

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

[Tony Chang]

Created attachment 91521 [details]
Patch for landing

[Tony Chang]

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.

[Tony Chang]

Comment on attachment 91521 [details]
Patch for landing

Crap, forgot to add the .html file.

[Tony Chang]

Created attachment 91523 [details]
Patch for landing

[Mark Pilgrim (Google)]

No idea how that happened. Thanks, Tony.

[Tony Chang]

Comment on attachment 91523 [details]
Patch for landing

hmm, the expected file is wrong.

[Mark Pilgrim (Google)]

OK, let me start over. I'll upload a new patch ASAP.

[Tony Chang]

(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.

[Mark Pilgrim (Google)]

No, I have nothing yet. I was finishing up another bug's patch. If you'd like to fix it, please do.

[Tony Chang]

Created attachment 91529 [details]
Patch for landing

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 91529 [details]:

animations/suspend-resume-animation.html bug 48161 (authors: [e-mail] and [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 91529 [details]
Patch for landing

Clearing flags on attachment: 91529

Committed r85238: <http://trac.webkit.org/changeset/85238>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 91529 [details]:

animations/suspend-resume-animation.html bug 48161 (authors: [e-mail] and [e-mail])
The commit-queue is continuing to process your patch.