Bug 99097

Summary: IndexedDB: Pass type of error causing abort to IDBTransaction::onAbort
Product: WebKit Reporter: Joshua Bell <jsbell>
Component: WebCore Misc.Assignee: Joshua Bell <jsbell>
Status: RESOLVED FIXED    
Severity: Normal CC: alecflett, dgrogan, tony, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch for landing none

Description Joshua Bell 2012-10-11 13:48:24 PDT
IDBTransaction.cpp has the lovely note:

FIXME: Propagate true cause from back end (e.g. QuotaError, UnknownError, etc.)

When we fail due to a ConstraintError (e.g. when indexing) this is not passed from the back end to the front end. 

It's one of the reasons we fail this test:

http://w3c-test.org/webapps/IndexedDB/tests/submissions/Opera/idbobjectstore_createIndex6-event_order.htm
Comment 1 Joshua Bell 2012-10-11 16:10:35 PDT
Created attachment 168307 [details]
Patch
Comment 2 Joshua Bell 2012-10-11 16:12:46 PDT
(In reply to comment #1)
> Created an attachment (id=168307) [details]
> Patch

This needs to be split up for landing:

* Add new Chromium public webkit API
* Add new Chromium plumbing
* This patch
* Remove old Chromium plumbing
* Remove old Chromium public webkit API
Comment 3 David Grogan 2012-10-11 18:56:18 PDT
Comment on attachment 168307 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=168307&action=review

LGTM

This gets us closer to being able to fix http://code.google.com/p/chromium/issues/detail?id=113118

> Source/WebCore/Modules/indexeddb/IDBTransactionBackendImpl.cpp:114
> +    abort(IDBDatabaseError::create(IDBDatabaseException::UNKNOWN_ERR, "Internal error."));

"Internal error" makes sense because a call to this method can only be originated from the backend. Accurate?

> LayoutTests/storage/indexeddb/lazy-index-population.html:102
> +	    shouldBe("trans.error.name", "'ConstraintError'");

the new lines have weird indents
Comment 4 Joshua Bell 2012-10-12 09:40:39 PDT
(In reply to comment #3)
> This gets us closer to being able to fix http://code.google.com/p/chromium/issues/detail?id=113118

Yep. The message still gets dropped on the floor when DOMError is created, though, but at least the plumbing is there.

> > Source/WebCore/Modules/indexeddb/IDBTransactionBackendImpl.cpp:114
> > +    abort(IDBDatabaseError::create(IDBDatabaseException::UNKNOWN_ERR, "Internal error."));
> 
> "Internal error" makes sense because a call to this method can only be originated from the backend. Accurate?
> 

Correct. FWIW, this is what the front end synthesizes today for all backend-triggered aborts. I left this as the default on the back end since it applies to most call sites. Places where an error is generated to fire at a request now use that error, and constraint failures from indexing get special handling.

> > LayoutTests/storage/indexeddb/lazy-index-population.html:102
> > +	    shouldBe("trans.error.name", "'ConstraintError'");
> 
> the new lines have weird indents

Will fix, thanks.
Comment 5 Joshua Bell 2012-10-12 11:38:14 PDT
Created attachment 168452 [details]
Patch
Comment 6 Joshua Bell 2012-10-15 14:00:13 PDT
tony@ - r?

Chromium side has rolled in, so this is unblocked.
Comment 7 WebKit Review Bot 2012-10-15 14:21:09 PDT
Comment on attachment 168452 [details]
Patch

Rejecting attachment 168452 [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:
/src/WebIDBTransactionCallbacksImpl.cpp
patching file Source/WebKit/chromium/src/WebIDBTransactionCallbacksImpl.h
patching file LayoutTests/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file LayoutTests/storage/indexeddb/lazy-index-population-expected.txt
patching file LayoutTests/storage/indexeddb/lazy-index-population.html

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force', u'--reviewer', u'Tony Chang']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue

Full output: http://queues.webkit.org/results/14378116
Comment 8 WebKit Review Bot 2012-10-15 14:30:34 PDT
Comment on attachment 168452 [details]
Patch

Rejecting attachment 168452 [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:
/src/WebIDBTransactionCallbacksImpl.cpp
patching file Source/WebKit/chromium/src/WebIDBTransactionCallbacksImpl.h
patching file LayoutTests/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file LayoutTests/storage/indexeddb/lazy-index-population-expected.txt
patching file LayoutTests/storage/indexeddb/lazy-index-population.html

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force', u'--reviewer', u'Tony Chang']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue

Full output: http://queues.webkit.org/results/14295738
Comment 9 Joshua Bell 2012-10-15 14:32:23 PDT
Created attachment 168786 [details]
Patch for landing
Comment 10 WebKit Review Bot 2012-10-15 15:19:10 PDT
Comment on attachment 168786 [details]
Patch for landing

Clearing flags on attachment: 168786

Committed r131371: <http://trac.webkit.org/changeset/131371>
Comment 11 WebKit Review Bot 2012-10-15 15:19:14 PDT
All reviewed patches have been landed.  Closing bug.