Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1972483002:
Pass around the candidate removals events in IOS clients
|
Created:
4 years, 7 months ago by honghaiz3 Modified:
4 years, 5 months ago Reviewers:
tkchin_webrtc CC:
webrtc-reviews_webrtc.org, tterriberry_mozilla.com, pthatcher1 Base URL:
https://chromium.googlesource.com/external/webrtc@master Target Ref:
refs/pending/heads/master Project:
webrtc Visibility:
Public. |
DescriptionPass around the candidate removals events in IOS clients
When local candidates are removed, signal to RTCPeerConnection
and eventually send to the remote client.
When a candidate-removal message is received, notify the native PeerConnection.
BUG=
R=tkchin@webrtc.org
Committed: https://crrev.com/da2ba4dcba8d0d56aac3a43a52a5c0821300fcea
Cr-Commit-Position: refs/heads/master@{#12852}
Patch Set 1 : #Patch Set 2 : Fix a type conversion in RTCPeerConnection.mm #
Total comments: 36
Patch Set 3 : #
Total comments: 2
Patch Set 4 : Address Zeke comments #
Total comments: 5
Patch Set 5 : Address comments #Patch Set 6 : merge with head #Patch Set 7 : Merge with head #Messages
Total messages: 41 (20 generated)
Patchset #1 (id:1) has been deleted
Patchset #1 (id:20001) has been deleted
Patchset #1 (id:40001) has been deleted
Description was changed from ========== Handle candidate removals in RTCPeerConnection BUG= ========== to ========== Pass around the candidate removals events in IOS clients When local candidates are removed, signal to RTCPeerConnection and eventually send to the remote client. When a candidate-removal message is received, notify the native PeerConnection. BUG= ==========
honghaiz@webrtc.org changed reviewers: + tkchin@webrtc.org
I tried to add tests for the RTCPeerConnection, but feel it is better to add it later after the basic framework for testing RTCPeerConnection is done.
On 2016/05/11 19:21:07, honghaiz3 wrote: > I tried to add tests for the RTCPeerConnection, but feel it is better to add it > later after the basic framework for testing RTCPeerConnection is done. Zeke, can you take a look at this CL?
https://codereview.webrtc.org/1972483002/diff/80001/webrtc/examples/objc/AppR... File webrtc/examples/objc/AppRTCDemo/ARDSignalingMessage.h (right): https://codereview.webrtc.org/1972483002/diff/80001/webrtc/examples/objc/AppR... webrtc/examples/objc/AppRTCDemo/ARDSignalingMessage.h:18: kARDSignalingMessageTypeCandidatesRemoval, nit: CandidateRemoval https://codereview.webrtc.org/1972483002/diff/80001/webrtc/examples/objc/AppR... webrtc/examples/objc/AppRTCDemo/ARDSignalingMessage.h:41: @interface ARDICECandidatesRemovalMessage : ARDSignalingMessage nit: ARDICECandidateRemovalMessage https://codereview.webrtc.org/1972483002/diff/80001/webrtc/examples/objc/AppR... File webrtc/examples/objc/AppRTCDemo/ARDSignalingMessage.m (right): https://codereview.webrtc.org/1972483002/diff/80001/webrtc/examples/objc/AppR... webrtc/examples/objc/AppRTCDemo/ARDSignalingMessage.m:55: [NSMutableArray arrayWithCapacity:[jsonCandidates count]]; dot syntax for properties. jsonCandidates.count https://codereview.webrtc.org/1972483002/diff/80001/webrtc/examples/objc/AppR... webrtc/examples/objc/AppRTCDemo/ARDSignalingMessage.m:62: initWithRemovedCandidates:candidates]; nit: Just move the entire thing to next line if it fits instead of doing odd break https://codereview.webrtc.org/1972483002/diff/80001/webrtc/examples/objc/AppR... webrtc/examples/objc/AppRTCDemo/ARDSignalingMessage.m:105: NSArray<RTCIceCandidate *> *)candidates { nit: indent 4 from leftmost margin (so remove two spaces on left here) https://codereview.webrtc.org/1972483002/diff/80001/webrtc/examples/objc/AppR... webrtc/examples/objc/AppRTCDemo/ARDSignalingMessage.m:106: if (self = [super initWithType:kARDSignalingMessageTypeCandidatesRemoval]) { May be worth tossing an assertion here. NSParameterAssert(candidates.count) https://codereview.webrtc.org/1972483002/diff/80001/webrtc/examples/objc/AppR... webrtc/examples/objc/AppRTCDemo/ARDSignalingMessage.m:114: [NSMutableArray arrayWithCapacity:[_candidates count]]; dot syntax for count https://codereview.webrtc.org/1972483002/diff/80001/webrtc/examples/objc/AppR... webrtc/examples/objc/AppRTCDemo/ARDSignalingMessage.m:115: for (const RTCIceCandidate *candidate in _candidates) { const qualifier not necessary https://codereview.webrtc.org/1972483002/diff/80001/webrtc/examples/objc/AppR... webrtc/examples/objc/AppRTCDemo/ARDSignalingMessage.m:119: NSDictionary * json = @{ *json https://codereview.webrtc.org/1972483002/diff/80001/webrtc/examples/objc/AppR... File webrtc/examples/objc/AppRTCDemo/RTCICECandidate+JSON.h (right): https://codereview.webrtc.org/1972483002/diff/80001/webrtc/examples/objc/AppR... webrtc/examples/objc/AppRTCDemo/RTCICECandidate+JSON.h:18: NSDictionary *) also nit: It is probably cleaner to make a method like + (NSData *)JSONDataForIceCandidates:(NSArray<RTCIceCandidate *> *)candidates; https://codereview.webrtc.org/1972483002/diff/80001/webrtc/sdk/objc/Framework... File webrtc/sdk/objc/Framework/Classes/RTCPeerConnection.mm (right): https://codereview.webrtc.org/1972483002/diff/80001/webrtc/sdk/objc/Framework... webrtc/sdk/objc/Framework/Classes/RTCPeerConnection.mm:188: NSMutableArray* iceCandidates = style in this file needs to be updated. Rule is C++ rules in C++ functions, and ObjC rules in ObjC methods. Just update new methods for now. So here in addition to * next to type, we also want underscores instead of camel casing. ice_candidates https://codereview.webrtc.org/1972483002/diff/80001/webrtc/sdk/objc/Framework... webrtc/sdk/objc/Framework/Classes/RTCPeerConnection.mm:191: std::unique_ptr<JsepIceCandidate> candidate_wrapper( ooc, why does new api return cricket::Candidate instead of JsepIceCandidate? https://codereview.webrtc.org/1972483002/diff/80001/webrtc/sdk/objc/Framework... webrtc/sdk/objc/Framework/Classes/RTCPeerConnection.mm:288: (unsigned long)[iceCandidates count]); .count https://codereview.webrtc.org/1972483002/diff/80001/webrtc/sdk/objc/Framework... webrtc/sdk/objc/Framework/Classes/RTCPeerConnection.mm:292: candidates.push_back(candidate->candidate()); probably add a if (candidate) check since nativeCandidate may be nullptr https://codereview.webrtc.org/1972483002/diff/80001/webrtc/sdk/objc/Framework... webrtc/sdk/objc/Framework/Classes/RTCPeerConnection.mm:296: _peerConnection->RemoveIceCandidates(candidates); ObjC name should match W3C name. Since this doesn't look to be in spec yet, let's match C++ name. removeIceCandidates
Thanks! PTAL. https://codereview.webrtc.org/1972483002/diff/80001/webrtc/examples/objc/AppR... File webrtc/examples/objc/AppRTCDemo/ARDSignalingMessage.h (right): https://codereview.webrtc.org/1972483002/diff/80001/webrtc/examples/objc/AppR... webrtc/examples/objc/AppRTCDemo/ARDSignalingMessage.h:18: kARDSignalingMessageTypeCandidatesRemoval, On 2016/05/16 20:48:58, tkchin_webrtc wrote: > nit: CandidateRemoval I understand this may appear grammatically incorrect, but I chose this to emphasize that this message is for removing a list of candidates instead of single candidate. https://codereview.webrtc.org/1972483002/diff/80001/webrtc/examples/objc/AppR... webrtc/examples/objc/AppRTCDemo/ARDSignalingMessage.h:41: @interface ARDICECandidatesRemovalMessage : ARDSignalingMessage On 2016/05/16 20:48:58, tkchin_webrtc wrote: > nit: ARDICECandidateRemovalMessage Same as above. https://codereview.webrtc.org/1972483002/diff/80001/webrtc/examples/objc/AppR... File webrtc/examples/objc/AppRTCDemo/ARDSignalingMessage.m (right): https://codereview.webrtc.org/1972483002/diff/80001/webrtc/examples/objc/AppR... webrtc/examples/objc/AppRTCDemo/ARDSignalingMessage.m:55: [NSMutableArray arrayWithCapacity:[jsonCandidates count]]; On 2016/05/16 20:48:58, tkchin_webrtc wrote: > dot syntax for properties. jsonCandidates.count Done. https://codereview.webrtc.org/1972483002/diff/80001/webrtc/examples/objc/AppR... webrtc/examples/objc/AppRTCDemo/ARDSignalingMessage.m:62: initWithRemovedCandidates:candidates]; On 2016/05/16 20:48:58, tkchin_webrtc wrote: > nit: Just move the entire thing to next line if it fits instead of doing odd > break It does not fit even if I move the entire thing to the next line. Hope this have used 100-column limit. https://codereview.webrtc.org/1972483002/diff/80001/webrtc/examples/objc/AppR... webrtc/examples/objc/AppRTCDemo/ARDSignalingMessage.m:105: NSArray<RTCIceCandidate *> *)candidates { On 2016/05/16 20:48:58, tkchin_webrtc wrote: > nit: indent 4 from leftmost margin (so remove two spaces on left here) Done. https://codereview.webrtc.org/1972483002/diff/80001/webrtc/examples/objc/AppR... webrtc/examples/objc/AppRTCDemo/ARDSignalingMessage.m:106: if (self = [super initWithType:kARDSignalingMessageTypeCandidatesRemoval]) { On 2016/05/16 20:48:58, tkchin_webrtc wrote: > May be worth tossing an assertion here. NSParameterAssert(candidates.count) Done. https://codereview.webrtc.org/1972483002/diff/80001/webrtc/examples/objc/AppR... webrtc/examples/objc/AppRTCDemo/ARDSignalingMessage.m:114: [NSMutableArray arrayWithCapacity:[_candidates count]]; On 2016/05/16 20:48:58, tkchin_webrtc wrote: > dot syntax for count Done. https://codereview.webrtc.org/1972483002/diff/80001/webrtc/examples/objc/AppR... webrtc/examples/objc/AppRTCDemo/ARDSignalingMessage.m:115: for (const RTCIceCandidate *candidate in _candidates) { On 2016/05/16 20:48:58, tkchin_webrtc wrote: > const qualifier not necessary Done. https://codereview.webrtc.org/1972483002/diff/80001/webrtc/examples/objc/AppR... webrtc/examples/objc/AppRTCDemo/ARDSignalingMessage.m:119: NSDictionary * json = @{ On 2016/05/16 20:48:58, tkchin_webrtc wrote: > *json Done. https://codereview.webrtc.org/1972483002/diff/80001/webrtc/examples/objc/AppR... File webrtc/examples/objc/AppRTCDemo/RTCICECandidate+JSON.h (right): https://codereview.webrtc.org/1972483002/diff/80001/webrtc/examples/objc/AppR... webrtc/examples/objc/AppRTCDemo/RTCICECandidate+JSON.h:18: On 2016/05/16 20:48:58, tkchin_webrtc wrote: > NSDictionary *) > > also nit: It is probably cleaner to make a method like > + (NSData *)JSONDataForIceCandidates:(NSArray<RTCIceCandidate *> *)candidates; Done for the first. I tried the second suggestion. It appears less cleaner to me. Candidate should only handle things inside a Candidate. Plus, if we do so, it will become inevitable to have "removed-candidates" const string (and perhaps "candidates") in both RTCIcECandidate+JSON.h and ARDSignalingMesssage.m file, which looks more error-prune in the future. https://codereview.webrtc.org/1972483002/diff/80001/webrtc/sdk/objc/Framework... File webrtc/sdk/objc/Framework/Classes/RTCPeerConnection.mm (right): https://codereview.webrtc.org/1972483002/diff/80001/webrtc/sdk/objc/Framework... webrtc/sdk/objc/Framework/Classes/RTCPeerConnection.mm:188: NSMutableArray* iceCandidates = On 2016/05/16 20:48:58, tkchin_webrtc wrote: > style in this file needs to be updated. Rule is C++ rules in C++ functions, and > ObjC rules in ObjC methods. Just update new methods for now. > > So here in addition to * next to type, we also want underscores instead of camel > casing. > ice_candidates Done. also replaced iceCandidate below. https://codereview.webrtc.org/1972483002/diff/80001/webrtc/sdk/objc/Framework... webrtc/sdk/objc/Framework/Classes/RTCPeerConnection.mm:191: std::unique_ptr<JsepIceCandidate> candidate_wrapper( On 2016/05/16 20:48:58, tkchin_webrtc wrote: > ooc, why does new api return cricket::Candidate instead of JsepIceCandidate? Good observation. We were doing that because we want to get rid of JsepIceCandidate and just use cricket::Candidate. We have only done that for the new code yet. https://codereview.webrtc.org/1972483002/diff/80001/webrtc/sdk/objc/Framework... webrtc/sdk/objc/Framework/Classes/RTCPeerConnection.mm:288: (unsigned long)[iceCandidates count]); On 2016/05/16 20:48:58, tkchin_webrtc wrote: > .count Done. https://codereview.webrtc.org/1972483002/diff/80001/webrtc/sdk/objc/Framework... webrtc/sdk/objc/Framework/Classes/RTCPeerConnection.mm:292: candidates.push_back(candidate->candidate()); On 2016/05/16 20:48:58, tkchin_webrtc wrote: > probably add a if (candidate) check since nativeCandidate may be nullptr Done. Thanks! https://codereview.webrtc.org/1972483002/diff/80001/webrtc/sdk/objc/Framework... webrtc/sdk/objc/Framework/Classes/RTCPeerConnection.mm:296: _peerConnection->RemoveIceCandidates(candidates); On 2016/05/16 20:48:58, tkchin_webrtc wrote: > ObjC name should match W3C name. Since this doesn't look to be in spec yet, > let's match C++ name. > > removeIceCandidates C++ name style is capitalizing all words. RemoveIceCandidates. And this matches the rest of methods there.
https://codereview.webrtc.org/1972483002/diff/80001/webrtc/examples/objc/AppR... File webrtc/examples/objc/AppRTCDemo/ARDSignalingMessage.h (right): https://codereview.webrtc.org/1972483002/diff/80001/webrtc/examples/objc/AppR... webrtc/examples/objc/AppRTCDemo/ARDSignalingMessage.h:18: kARDSignalingMessageTypeCandidatesRemoval, On 2016/05/17 00:46:52, honghaiz3 wrote: > On 2016/05/16 20:48:58, tkchin_webrtc wrote: > > nit: CandidateRemoval > > I understand this may appear grammatically incorrect, but I chose this to > emphasize that this message is for removing a list of candidates instead of > single candidate. I think it is ok to use CandidateRemoval because the ctor only takes an array, and the only exposed property is also an array, so it should be clear to any user. Makes it easier to read for me. But up to you. https://codereview.webrtc.org/1972483002/diff/80001/webrtc/sdk/objc/Framework... File webrtc/sdk/objc/Framework/Classes/RTCPeerConnection.mm (right): https://codereview.webrtc.org/1972483002/diff/80001/webrtc/sdk/objc/Framework... webrtc/sdk/objc/Framework/Classes/RTCPeerConnection.mm:287: RTCLogInfo(@"Remove %lu remote ice candidates", I don't think we should log this here. It should happen in C++ land, or in the code calling this. https://codereview.webrtc.org/1972483002/diff/80001/webrtc/sdk/objc/Framework... webrtc/sdk/objc/Framework/Classes/RTCPeerConnection.mm:296: _peerConnection->RemoveIceCandidates(candidates); On 2016/05/17 00:46:53, honghaiz3 wrote: > On 2016/05/16 20:48:58, tkchin_webrtc wrote: > > ObjC name should match W3C name. Since this doesn't look to be in spec yet, > > let's match C++ name. > > > > removeIceCandidates > > C++ name style is capitalizing all words. RemoveIceCandidates. > And this matches the rest of methods there. I was referring to the name of the method. s/removeRemoteIceCandidates/removeIceCandidates/g https://codereview.webrtc.org/1972483002/diff/100001/webrtc/examples/objc/App... File webrtc/examples/objc/AppRTCDemo/RTCICECandidate+JSON.m (right): https://codereview.webrtc.org/1972483002/diff/100001/webrtc/examples/objc/App... webrtc/examples/objc/AppRTCDemo/RTCICECandidate+JSON.m:54: kRTCICECandidateMLineIndexKey : @(self.sdpMLineIndex), the reason why I didn't like JSONDictionary is because this dictionary is not the same as the dictionary in JSONData. Maybe the better thing is to have JSONData call JSONDictionary and then add the type key somewhere else. but otherwise agree we don't want dup-strings
PTAL. https://codereview.webrtc.org/1972483002/diff/80001/webrtc/examples/objc/AppR... File webrtc/examples/objc/AppRTCDemo/ARDSignalingMessage.h (right): https://codereview.webrtc.org/1972483002/diff/80001/webrtc/examples/objc/AppR... webrtc/examples/objc/AppRTCDemo/ARDSignalingMessage.h:18: kARDSignalingMessageTypeCandidatesRemoval, On 2016/05/17 02:01:43, tkchin_webrtc wrote: > On 2016/05/17 00:46:52, honghaiz3 wrote: > > On 2016/05/16 20:48:58, tkchin_webrtc wrote: > > > nit: CandidateRemoval > > > > I understand this may appear grammatically incorrect, but I chose this to > > emphasize that this message is for removing a list of candidates instead of > > single candidate. > > I think it is ok to use CandidateRemoval because the ctor only takes an array, > and the only exposed property is also an array, so it should be clear to any > user. Makes it easier to read for me. But up to you. Done. https://codereview.webrtc.org/1972483002/diff/80001/webrtc/sdk/objc/Framework... File webrtc/sdk/objc/Framework/Classes/RTCPeerConnection.mm (right): https://codereview.webrtc.org/1972483002/diff/80001/webrtc/sdk/objc/Framework... webrtc/sdk/objc/Framework/Classes/RTCPeerConnection.mm:287: RTCLogInfo(@"Remove %lu remote ice candidates", On 2016/05/17 02:01:43, tkchin_webrtc wrote: > I don't think we should log this here. It should happen in C++ land, or in the > code calling this. Done. https://codereview.webrtc.org/1972483002/diff/80001/webrtc/sdk/objc/Framework... webrtc/sdk/objc/Framework/Classes/RTCPeerConnection.mm:296: _peerConnection->RemoveIceCandidates(candidates); On 2016/05/17 02:01:43, tkchin_webrtc wrote: > On 2016/05/17 00:46:53, honghaiz3 wrote: > > On 2016/05/16 20:48:58, tkchin_webrtc wrote: > > > ObjC name should match W3C name. Since this doesn't look to be in spec yet, > > > let's match C++ name. > > > > > > removeIceCandidates > > > > C++ name style is capitalizing all words. RemoveIceCandidates. > > And this matches the rest of methods there. > > I was referring to the name of the method. > s/removeRemoteIceCandidates/removeIceCandidates/g Ah. I got it. You meant the method name in this class, not the line here. Done. https://codereview.webrtc.org/1972483002/diff/100001/webrtc/examples/objc/App... File webrtc/examples/objc/AppRTCDemo/RTCICECandidate+JSON.m (right): https://codereview.webrtc.org/1972483002/diff/100001/webrtc/examples/objc/App... webrtc/examples/objc/AppRTCDemo/RTCICECandidate+JSON.m:54: kRTCICECandidateMLineIndexKey : @(self.sdpMLineIndex), On 2016/05/17 02:01:43, tkchin_webrtc wrote: > the reason why I didn't like JSONDictionary is because this dictionary is not > the same as the dictionary in JSONData. Maybe the better thing is to have > JSONData call JSONDictionary and then add the type key somewhere else. > > but otherwise agree we don't want dup-strings Done. Passed in the type key as an argument.
https://codereview.webrtc.org/1972483002/diff/120001/webrtc/examples/objc/App... File webrtc/examples/objc/AppRTCDemo/ARDSignalingMessage.m (right): https://codereview.webrtc.org/1972483002/diff/120001/webrtc/examples/objc/App... webrtc/examples/objc/AppRTCDemo/ARDSignalingMessage.m:20: static NSString * const kARDTypeValueRemoveCandidates = @"remove-candidates"; I hope to use a longer variable name like kARDSignalingMessageTyupeValueRemoveCandidates, but this makes the formatting (when it is used) extremely hard, so I shortened it a little.
lgtm https://codereview.webrtc.org/1972483002/diff/120001/webrtc/examples/objc/App... File webrtc/examples/objc/AppRTCDemo/ARDSignalingMessage.m (right): https://codereview.webrtc.org/1972483002/diff/120001/webrtc/examples/objc/App... webrtc/examples/objc/AppRTCDemo/ARDSignalingMessage.m:20: static NSString * const kARDTypeValueRemoveCandidates = @"remove-candidates"; On 2016/05/17 06:14:33, honghaiz3 wrote: > I hope to use a longer variable name like > kARDSignalingMessageTyupeValueRemoveCandidates, > but this makes the formatting (when it is used) extremely hard, so I shortened > it a little. That's fine. We really need to make 100 char happen for objc bits of our repo heh. https://codereview.webrtc.org/1972483002/diff/120001/webrtc/examples/objc/App... webrtc/examples/objc/AppRTCDemo/ARDSignalingMessage.m:109: [RTCIceCandidate JSONDataFromIceCandidates:_candidates nit JSONDatForIceCandidates
The CQ bit was checked by honghaiz@webrtc.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1972483002/140001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1972483002/140001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_arm64_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
The CQ bit was checked by honghaiz@webrtc.org
The patchset sent to the CQ was uploaded after l-g-t-m from tkchin@webrtc.org Link to the patchset: https://codereview.webrtc.org/1972483002/#ps160001 (title: "merge with head")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1972483002/160001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1972483002/160001
The CQ bit was unchecked by honghaiz@webrtc.org
The CQ bit was checked by honghaiz@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1972483002/160001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1972483002/160001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_arm64_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
The CQ bit was checked by honghaiz@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1972483002/160001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1972483002/160001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_arm64_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL) android_dbg on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
The CQ bit was checked by honghaiz@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1972483002/160001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1972483002/160001
The CQ bit was checked by honghaiz@webrtc.org
The patchset sent to the CQ was uploaded after l-g-t-m from tkchin@webrtc.org Link to the patchset: https://codereview.webrtc.org/1972483002/#ps180001 (title: "Merge with head")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1972483002/180001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1972483002/180001
Description was changed from ========== Pass around the candidate removals events in IOS clients When local candidates are removed, signal to RTCPeerConnection and eventually send to the remote client. When a candidate-removal message is received, notify the native PeerConnection. BUG= ========== to ========== Pass around the candidate removals events in IOS clients When local candidates are removed, signal to RTCPeerConnection and eventually send to the remote client. When a candidate-removal message is received, notify the native PeerConnection. BUG= R=tkchin@webrtc.org Committed: https://chromium.googlesource.com/external/webrtc/+/da2ba4dcba8d0d56aac3a43a5... ==========
Description was changed from ========== Pass around the candidate removals events in IOS clients When local candidates are removed, signal to RTCPeerConnection and eventually send to the remote client. When a candidate-removal message is received, notify the native PeerConnection. BUG= R=tkchin@webrtc.org Committed: https://chromium.googlesource.com/external/webrtc/+/da2ba4dcba8d0d56aac3a43a5... ========== to ========== Pass around the candidate removals events in IOS clients When local candidates are removed, signal to RTCPeerConnection and eventually send to the remote client. When a candidate-removal message is received, notify the native PeerConnection. BUG= R=tkchin@webrtc.org Committed: https://crrev.com/da2ba4dcba8d0d56aac3a43a52a5c0821300fcea Cr-Commit-Position: refs/heads/master@{#12852} ==========
Committed patchset #7 (id:180001) manually as da2ba4dcba8d0d56aac3a43a52a5c0821300fcea (presubmit successful).
Patchset 7 (id:??) landed as https://crrev.com/da2ba4dcba8d0d56aac3a43a52a5c0821300fcea Cr-Commit-Position: refs/heads/master@{#12852}
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_arm64_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL) android_dbg on tryserver.webrtc (JOB_TIMED_OUT, no build URL) android_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
Just respond to the comment so it will go away from my outgoing CL list. https://codereview.webrtc.org/1972483002/diff/120001/webrtc/examples/objc/App... File webrtc/examples/objc/AppRTCDemo/ARDSignalingMessage.m (right): https://codereview.webrtc.org/1972483002/diff/120001/webrtc/examples/objc/App... webrtc/examples/objc/AppRTCDemo/ARDSignalingMessage.m:20: static NSString * const kARDTypeValueRemoveCandidates = @"remove-candidates"; On 2016/05/17 20:53:02, tkchin_webrtc wrote: > On 2016/05/17 06:14:33, honghaiz3 wrote: > > I hope to use a longer variable name like > > kARDSignalingMessageTyupeValueRemoveCandidates, > > but this makes the formatting (when it is used) extremely hard, so I shortened > > it a little. > > That's fine. We really need to make 100 char happen for objc bits of our repo > heh. Acknowledged. https://codereview.webrtc.org/1972483002/diff/120001/webrtc/examples/objc/App... webrtc/examples/objc/AppRTCDemo/ARDSignalingMessage.m:109: [RTCIceCandidate JSONDataFromIceCandidates:_candidates On 2016/05/17 20:53:02, tkchin_webrtc wrote: > nit JSONDatForIceCandidates Done. |
