Bug 125574

Summary: Adding RTCPeerConnectionErrorCallback
Product: WebKit Reporter: Thiago de Barros Lacerda <thiago.lacerda>
Component: WebCore Misc.Assignee: Thiago de Barros Lacerda <thiago.lacerda>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, cdumez, commit-queue, eric.carlson, esprehn+autocc, glenn, gyuyoung.kim, hta, jer.noble, kondapallykalyan, pnormand, rakuco, rego+ews, rniwa, tommyw, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 125568    
Bug Blocks: 124288, 125655    
Attachments:
Description Flags
Patch
none
Patch
pnormand: review-, buildbot: commit-queue-
Patch
none
Patch
none
Patch
none
Patch none

Thiago de Barros Lacerda
Reported 2013-12-11 07:19:27 PST
According to the spec there should be a RTCPeerConnectionErrorCallback function type for createOffer/Answer, setLocal/RemoteDescription and updateIce function calls. This callback must handle a DOMError object.
Attachments
Patch (44.16 KB, patch)
2013-12-11 07:49 PST, Thiago de Barros Lacerda 		no flags
DetailsFormatted DiffDiff
Patch (44.29 KB, patch)
2013-12-11 08:45 PST, Thiago de Barros Lacerda 		pnormand: review-
buildbot: commit-queue-
DetailsFormatted DiffDiff
Patch (52.74 KB, patch)
2013-12-11 09:20 PST, Philippe Normand 		no flags
DetailsFormatted DiffDiff
Patch (51.29 KB, patch)
2013-12-11 11:02 PST, Philippe Normand 		no flags
DetailsFormatted DiffDiff
Patch (56.62 KB, patch)
2013-12-12 03:39 PST, Philippe Normand 		no flags
DetailsFormatted DiffDiff
Patch (56.79 KB, patch)
2013-12-12 13:23 PST, Thiago de Barros Lacerda 		no flags
DetailsFormatted DiffDiff
Show Obsolete (5) View All Add attachment proposed patch, testcase, etc.
Thiago de Barros Lacerda
Comment 1 2013-12-11 07:49:21 PST
Created attachment 218967 [details] Patch
Philippe Normand
Comment 2 2013-12-11 07:54:46 PST
Comment on attachment 218967 [details] Patch Seems like this is not rebased on top of ToT ?
Philippe Normand
Comment 3 2013-12-11 07:56:40 PST
Looks good otherwise
Thiago de Barros Lacerda
Comment 4 2013-12-11 08:03:01 PST
(In reply to comment #3) > Looks good otherwise Hey Philippe, that's because it depends on https://bugs.webkit.org/show_bug.cgi?id=125568. Could you take a look on it as well?
Thiago de Barros Lacerda
Comment 5 2013-12-11 08:45:14 PST
Created attachment 218968 [details] Patch
Build Bot
Comment 6 2013-12-11 09:20:18 PST
Comment on attachment 218968 [details] Patch Attachment 218968 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/47308534
Philippe Normand
Comment 7 2013-12-11 09:20:37 PST
Created attachment 218972 [details] Patch with Xcode project update
Philippe Normand
Comment 8 2013-12-11 09:25:25 PST
Comment on attachment 218968 [details] Patch Didn't pass mac EWS
Build Bot
Comment 9 2013-12-11 09:55:54 PST
Comment on attachment 218972 [details] Patch Attachment 218972 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/47348348
Build Bot
Comment 10 2013-12-11 10:04:58 PST
Comment on attachment 218972 [details] Patch Attachment 218972 [details] did not pass win-ews (win): Output: http://webkit-queues.appspot.com/results/47208575
Build Bot
Comment 11 2013-12-11 10:21:10 PST
Comment on attachment 218972 [details] Patch Attachment 218972 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/45888560
Philippe Normand
Comment 12 2013-12-11 11:02:35 PST
Created attachment 218978 [details] Patch
Build Bot
Comment 13 2013-12-11 11:40:19 PST
Comment on attachment 218978 [details] Patch Attachment 218978 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/47438176
Build Bot
Comment 14 2013-12-11 12:24:42 PST
Comment on attachment 218978 [details] Patch Attachment 218978 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/45658027
Build Bot
Comment 15 2013-12-11 13:07:24 PST
Comment on attachment 218978 [details] Patch Attachment 218978 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/47008146
Eric Carlson
Comment 16 2013-12-11 13:14:10 PST
Comment on attachment 218978 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=218978&action=review > LayoutTests/fast/mediastream/RTCPeerConnection-createOffer.html:26 > errorReason = reason; > - shouldBe('errorReason', '"TEST_ERROR"'); > + shouldBe('errorReason.name', '"IncompatibleConstraintsError"'); Is it possible to create tests for the other new errors?
Philippe Normand
Comment 17 2013-12-12 03:39:11 PST
Created attachment 219069 [details] Patch with Xcode project update
Thiago de Barros Lacerda
Comment 18 2013-12-12 06:12:41 PST
(In reply to comment #16) > (From update of attachment 218978 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=218978&action=review > > > LayoutTests/fast/mediastream/RTCPeerConnection-createOffer.html:26 > > errorReason = reason; > > - shouldBe('errorReason', '"TEST_ERROR"'); > > + shouldBe('errorReason.name', '"IncompatibleConstraintsError"'); > > Is it possible to create tests for the other new errors? Yes Eric, I'm now working on a patch for them. I didn't not put on this because it would increase the size of the patch too much. Are you OK with another patch?
Eric Carlson
Comment 19 2013-12-12 07:24:41 PST
(In reply to comment #18) > (In reply to comment #16) > > (From update of attachment 218978 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=218978&action=review > > > > > LayoutTests/fast/mediastream/RTCPeerConnection-createOffer.html:26 > > > errorReason = reason; > > > - shouldBe('errorReason', '"TEST_ERROR"'); > > > + shouldBe('errorReason.name', '"IncompatibleConstraintsError"'); > > > > Is it possible to create tests for the other new errors? > > Yes Eric, I'm now working on a patch for them. I didn't not put on this because it would increase the size of the patch too much. Are you OK with another patch? That seems fine. Please include a FIXME: with a bug number so someone reading the patch knows what the plan is.
Thiago de Barros Lacerda
Comment 20 2013-12-12 13:23:48 PST
Created attachment 219113 [details] Patch
Philippe Normand
Comment 21 2013-12-12 23:51:10 PST
Seems like a FIXME is missing or we assume it's the bug blocked by this one?
Thiago de Barros Lacerda
Comment 22 2013-12-13 01:49:49 PST
(In reply to comment #21) > Seems like a FIXME is missing or we assume it's the bug blocked by this one? It's the bug blocked by this one
WebKit Commit Bot
Comment 23 2013-12-13 12:01:33 PST
Comment on attachment 219113 [details] Patch Clearing flags on attachment: 219113 Committed r160553: <http://trac.webkit.org/changeset/160553>
WebKit Commit Bot
Comment 24 2013-12-13 12:01:39 PST
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.