Bug 93117

Summary: MediaStream API: Introduce RTCIceCandidate
Product: WebKit Reporter: Tommy Widenflycht <tommyw>
Component: WebCore Misc.Assignee: Tommy Widenflycht <tommyw>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, eric.carlson, feature-media-reviews, gustavo, gyuyoung.kim, ojan, philn, rakuco, webkit.review.bot, xan.lopez
Priority: P2 Keywords: WebExposed
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 80589    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Tommy Widenflycht 2012-08-03 07:20:23 PDT 
Introduce RTCIceCandidate together with its corresponding WebCore/platform representation.
Comment 1 Tommy Widenflycht 2012-08-03 07:26:25 PDT 
Created attachment 156373 [details]
Patch
Comment 2 Tommy Widenflycht 2012-08-03 07:47:56 PDT 
Created attachment 156381 [details]
Patch
Comment 3 Adam Barth 2012-08-03 10:01:43 PDT 
Comment on attachment 156381 [details]
Patch

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

> Source/WebCore/Modules/mediastream/DOMWindowMediaStream.idl:38
>          attribute IceCandidateConstructor IceCandidate;
> +        attribute RTCIceCandidateConstructor RTCIceCandidate;

It's kind of strange that we're rebuilding the entire feature with an RTC prefix.  I guess we'll remove the non-RTC prefixed versions when the time is right.

> Source/WebCore/Modules/mediastream/RTCIceCandidate.h:57
> +    String toString();

This function seems to be missing an implementation.

> Source/WebCore/Modules/mediastream/RTCIceCandidate.h:62
> +    RTCIceCandidate(PassRefPtr<RTCIceCandidateDescriptor>);

explicit
Comment 4 Tommy Widenflycht 2012-08-22 05:14:48 PDT 
Comment on attachment 156381 [details]
Patch

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

>> Source/WebCore/Modules/mediastream/DOMWindowMediaStream.idl:38
>> +        attribute RTCIceCandidateConstructor RTCIceCandidate;
> 
> It's kind of strange that we're rebuilding the entire feature with an RTC prefix.  I guess we'll remove the non-RTC prefixed versions when the time is right.

Yes, and the non-RTC prefixed version will be removed when PeerConnection00 disappears.

>> Source/WebCore/Modules/mediastream/RTCIceCandidate.h:57
>> +    String toString();
> 
> This function seems to be missing an implementation.

Removed for now. Will add this functionality later on.

>> Source/WebCore/Modules/mediastream/RTCIceCandidate.h:62
>> +    RTCIceCandidate(PassRefPtr<RTCIceCandidateDescriptor>);
> 
> explicit

Fixed.
Comment 5 Tommy Widenflycht 2012-08-22 05:20:05 PDT 
Created attachment 159907 [details]
Patch
Comment 6 Adam Barth 2012-08-22 12:05:22 PDT 
Comment on attachment 159907 [details]
Patch

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

> Source/WebCore/Modules/mediastream/RTCIceCandidate.h:57
> +    RTCIceCandidateDescriptor* descriptor();

Commonly we'd just declare this function line because it's a simple getter.
Comment 7 WebKit Review Bot 2012-08-22 12:18:12 PDT 
Comment on attachment 159907 [details]
Patch

Clearing flags on attachment: 159907

Committed r126328: <http://trac.webkit.org/changeset/126328>
Comment 8 WebKit Review Bot 2012-08-22 12:18:16 PDT 
All reviewed patches have been landed.  Closing bug.