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

Description Thiago de Barros Lacerda 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.
Comment 1 Thiago de Barros Lacerda 2013-12-11 07:49:21 PST
Created attachment 218967 [details]
Patch
Comment 2 Philippe Normand 2013-12-11 07:54:46 PST
Comment on attachment 218967 [details]
Patch

Seems like this is not rebased on top of ToT ?
Comment 3 Philippe Normand 2013-12-11 07:56:40 PST
Looks good otherwise
Comment 4 Thiago de Barros Lacerda 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?
Comment 5 Thiago de Barros Lacerda 2013-12-11 08:45:14 PST
Created attachment 218968 [details]
Patch
Comment 6 Build Bot 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
Comment 7 Philippe Normand 2013-12-11 09:20:37 PST
Created attachment 218972 [details]
Patch

with Xcode project update
Comment 8 Philippe Normand 2013-12-11 09:25:25 PST
Comment on attachment 218968 [details]
Patch

Didn't pass mac EWS
Comment 9 Build Bot 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
Comment 10 Build Bot 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
Comment 11 Build Bot 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
Comment 12 Philippe Normand 2013-12-11 11:02:35 PST
Created attachment 218978 [details]
Patch
Comment 13 Build Bot 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
Comment 14 Build Bot 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
Comment 15 Build Bot 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
Comment 16 Eric Carlson 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?
Comment 17 Philippe Normand 2013-12-12 03:39:11 PST
Created attachment 219069 [details]
Patch

with Xcode project update
Comment 18 Thiago de Barros Lacerda 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?
Comment 19 Eric Carlson 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.
Comment 20 Thiago de Barros Lacerda 2013-12-12 13:23:48 PST
Created attachment 219113 [details]
Patch
Comment 21 Philippe Normand 2013-12-12 23:51:10 PST
Seems like  a FIXME is missing or we assume it's the bug blocked by this one?
Comment 22 Thiago de Barros Lacerda 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
Comment 23 WebKit Commit Bot 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>
Comment 24 WebKit Commit Bot 2013-12-13 12:01:39 PST
All reviewed patches have been landed.  Closing bug.