Bug 59569 – Should call done() from unexpectedErrorCallback so failing IndexedDB tests don't time out

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Mark Pilgrim (Google)]

As discussed in bug 59465, especially https://bugs.webkit.org/show_bug.cgi?id=59465#c9

[Mark Pilgrim (Google)]

Presumably also from unexpectedSuccessCallback, unexpectedAbortCallback, unexpectedCompleteCallback, and unexpectedBlockedCallback.

These are all in LayoutTests/storage/indexeddb/resources/shared.js

[Mark Pilgrim (Google)]

Preliminary test run with these changes (calling done() in all the unexpected callbacks) confirms that this does not affect existing tests... except one. LayoutTests/storage/indexeddb/exception-in-event-aborts.html shows a slight difference. Attaching diff for discussion.

[Mark Pilgrim (Google)]

Created attachment 91207 [details]
diff of affected test

[David Grogan]

Comment on attachment 91207 [details]
diff of affected test

That test needs some fixing, but I'm not yet sure how much.  I think 

request = evalAndLog("store.add({x: 'value4', y: 'zzz4'}, 'key4')");
request.onsuccess = unexpectedSuccessCallback;
request.onerror = throwAndCatch;

should probably be changed to 

request.onsuccess = function(){ testPassed("something") }
request.onerror = unexpectedErrorCallback

but I'm not yet positive that Jeremy made a mistake as opposed to me not understanding his cleverness.

[Mark Pilgrim (Google)]

I'm going ahead with this change as well as your proposed fix for exception-in-event-aborts.html.

[Mark Pilgrim (Google)]

Created attachment 91356 [details]
Patch

[David Grogan]

Comment on attachment 91356 [details]
Patch

r+

Out of curiosity, and for my own sake, how are you updating the text expectation txt files?  I copy them manually from layout-test-results but I'd imagine there's a better way.

[Tony Chang]

BTW, normally when uploading a patch, you should set cq? if you want me to also cq+ the patch.  I've been adding cq+ since you guys don't have commit access right now, but cq? makes it more explicit that you want me to do this.

[Mark Pilgrim (Google)]

(In reply to comment #7)
> Out of curiosity, and for my own sake, how are you updating the text expectation txt files?  I copy them manually from layout-test-results but I'd imagine there's a better way.

That's what I'm doing too, plus re-running them to make sure they pass and I didn't mess up the whitespace or anything.

[WebKit Commit Bot]

Comment on attachment 91356 [details]
Patch

Clearing flags on attachment: 91356

Committed r85153: <http://trac.webkit.org/changeset/85153>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.