Update API for Objective-C RTCIceCandidate.

Update API for Objective-C RTCIceCandidate.

BUG=
R=tkchin@webrtc.org

Committed: https://crrev.com/29d5e570b5bacdd524535f58d2b367b8f5cdd052
Patch from Jon Hjelle <hjon@andyet.net>;.

Cr-Commit-Position: refs/heads/master@{#11156}

[hjon, 2015-12-12 22:48:43]

Description was changed from

==========
Update API for Objective-C RTCIceCandidate.

BUG=
==========

to

==========
Update API for Objective-C RTCIceCandidate.

BUG=
==========

[hjon, 2015-12-12 22:48:43]

hjon@andyet.net changed reviewers:
+ tkchin@webrtc.org

[tkchin_webrtc, 2015-12-15 01:55:44]

https://codereview.webrtc.org/1517253005/diff/1/webrtc/api/objc/RTCIceCandida...
File webrtc/api/objc/RTCIceCandidate+Private.h (right):

https://codereview.webrtc.org/1517253005/diff/1/webrtc/api/objc/RTCIceCandida...
webrtc/api/objc/RTCIceCandidate+Private.h:25:
rtc::scoped_ptr<webrtc::IceCandidateInterface> candidate;
nit: let's give the C++ a "native" prefix
so call this nativeCandidate

https://codereview.webrtc.org/1517253005/diff/1/webrtc/api/objc/RTCIceCandida...
webrtc/api/objc/RTCIceCandidate+Private.h:28: * Initialize an RTCIceCandidate
from a native IceCandidateInterface.
add that no ownership is taken

https://codereview.webrtc.org/1517253005/diff/1/webrtc/api/objc/RTCIceCandida...
File webrtc/api/objc/RTCIceCandidate.h (right):

https://codereview.webrtc.org/1517253005/diff/1/webrtc/api/objc/RTCIceCandida...
webrtc/api/objc/RTCIceCandidate.h:37: - (instancetype)initWithSdpMid:(nullable
NSString *)sdpMid
nit: put the nullable arg last

https://codereview.webrtc.org/1517253005/diff/1/webrtc/api/objc/RTCIceCandida...
File webrtc/api/objc/RTCIceCandidate.mm (right):

https://codereview.webrtc.org/1517253005/diff/1/webrtc/api/objc/RTCIceCandida...
webrtc/api/objc/RTCIceCandidate.mm:44: std::string sdp;
assert on candidate

https://codereview.webrtc.org/1517253005/diff/1/webrtc/api/objc/RTCIceCandida...
webrtc/api/objc/RTCIceCandidate.mm:59: if (candidate == NULL) {
if (!candidate) {

https://codereview.webrtc.org/1517253005/diff/1/webrtc/api/objc/RTCIceCandida...
webrtc/api/objc/RTCIceCandidate.mm:60: NSLog(@"Failed to create ICE candidate:
%s\nline: %s",
RTCLog

#import "webrtc/base/RTCLogging.h"

https://codereview.webrtc.org/1517253005/diff/1/webrtc/api/objc/RTCIceCandida...
webrtc/api/objc/RTCIceCandidate.mm:65:
rtc::scoped_ptr<webrtc::IceCandidateInterface> iceCandidate(candidate);
I don't think you need this line, it *should* invoke ctor (my C++ is a bit
rusty). Try it and see?

Otherwise you can:
return rtc::scoped_ptr<webrtc::IceCandidateInterface>(candidate);

https://codereview.webrtc.org/1517253005/diff/1/webrtc/api/objctests/RTCIceCa...
File webrtc/api/objctests/RTCIceCandidateTest.mm (right):

https://codereview.webrtc.org/1517253005/diff/1/webrtc/api/objctests/RTCIceCa...
webrtc/api/objctests/RTCIceCandidateTest.mm:27: NSString *sdp =
@"candidate:4025901590 1 udp 2122265343 fdff:2642:12a6:fe38:c001:beda:fcf9:51aa
59052 typ host generation 0";
80 chars. You can split it like so:

@"foo"
  "bar"
  "baz"

[hjon, 2015-12-15 17:35:45]

https://codereview.webrtc.org/1517253005/diff/1/webrtc/api/objc/RTCIceCandida...
File webrtc/api/objc/RTCIceCandidate+Private.h (right):

https://codereview.webrtc.org/1517253005/diff/1/webrtc/api/objc/RTCIceCandida...
webrtc/api/objc/RTCIceCandidate+Private.h:25:
rtc::scoped_ptr<webrtc::IceCandidateInterface> candidate;
On 2015/12/15 01:55:44, tkchin_webrtc wrote:
> nit: let's give the C++ a "native" prefix
> so call this nativeCandidate
> 

Done.

https://codereview.webrtc.org/1517253005/diff/1/webrtc/api/objc/RTCIceCandida...
webrtc/api/objc/RTCIceCandidate+Private.h:28: * Initialize an RTCIceCandidate
from a native IceCandidateInterface.
On 2015/12/15 01:55:44, tkchin_webrtc wrote:
> add that no ownership is taken

Done.

https://codereview.webrtc.org/1517253005/diff/1/webrtc/api/objc/RTCIceCandida...
File webrtc/api/objc/RTCIceCandidate.h (right):

https://codereview.webrtc.org/1517253005/diff/1/webrtc/api/objc/RTCIceCandida...
webrtc/api/objc/RTCIceCandidate.h:37: - (instancetype)initWithSdpMid:(nullable
NSString *)sdpMid
On 2015/12/15 01:55:44, tkchin_webrtc wrote:
> nit: put the nullable arg last

Done.

https://codereview.webrtc.org/1517253005/diff/1/webrtc/api/objc/RTCIceCandida...
File webrtc/api/objc/RTCIceCandidate.mm (right):

https://codereview.webrtc.org/1517253005/diff/1/webrtc/api/objc/RTCIceCandida...
webrtc/api/objc/RTCIceCandidate.mm:44: std::string sdp;
On 2015/12/15 01:55:44, tkchin_webrtc wrote:
> assert on candidate 

Done.

https://codereview.webrtc.org/1517253005/diff/1/webrtc/api/objc/RTCIceCandida...
webrtc/api/objc/RTCIceCandidate.mm:59: if (candidate == NULL) {
On 2015/12/15 01:55:44, tkchin_webrtc wrote:
> if (!candidate) {

Done.

https://codereview.webrtc.org/1517253005/diff/1/webrtc/api/objc/RTCIceCandida...
webrtc/api/objc/RTCIceCandidate.mm:60: NSLog(@"Failed to create ICE candidate:
%s\nline: %s",
On 2015/12/15 01:55:44, tkchin_webrtc wrote:
> RTCLog
> 
> #import "webrtc/base/RTCLogging.h"

Done.

https://codereview.webrtc.org/1517253005/diff/1/webrtc/api/objc/RTCIceCandida...
webrtc/api/objc/RTCIceCandidate.mm:65:
rtc::scoped_ptr<webrtc::IceCandidateInterface> iceCandidate(candidate);
On 2015/12/15 01:55:44, tkchin_webrtc wrote:
> I don't think you need this line, it *should* invoke ctor (my C++ is a bit
> rusty). Try it and see?
> 
> Otherwise you can:
> return rtc::scoped_ptr<webrtc::IceCandidateInterface>(candidate);

I might have misunderstood this, but simply putting:
return candidate;

didn't seem to be sufficient. So I replaced it with:
return rtc::scoped_ptr<webrtc::IceCandidateInterface>(candidate);

as suggested.

https://codereview.webrtc.org/1517253005/diff/1/webrtc/api/objctests/RTCIceCa...
File webrtc/api/objctests/RTCIceCandidateTest.mm (right):

https://codereview.webrtc.org/1517253005/diff/1/webrtc/api/objctests/RTCIceCa...
webrtc/api/objctests/RTCIceCandidateTest.mm:27: NSString *sdp =
@"candidate:4025901590 1 udp 2122265343 fdff:2642:12a6:fe38:c001:beda:fcf9:51aa
59052 typ host generation 0";
On 2015/12/15 01:55:44, tkchin_webrtc wrote:
> 80 chars. You can split it like so:
> 
> @"foo"
>   "bar"
>   "baz"

Done.

[tkchin_webrtc, 2016-01-05 16:18:07]

lgtm

https://codereview.webrtc.org/1517253005/diff/40001/webrtc/api/objc/RTCIceCan...
File webrtc/api/objc/RTCIceCandidate+Private.h (right):

https://codereview.webrtc.org/1517253005/diff/40001/webrtc/api/objc/RTCIceCan...
webrtc/api/objc/RTCIceCandidate+Private.h:13: #include
"webrtc/base/scoped_ptr.h"
alphabetize

https://codereview.webrtc.org/1517253005/diff/40001/webrtc/api/objc/RTCIceCan...
webrtc/api/objc/RTCIceCandidate+Private.h:31: -
(instancetype)initWithCandidate:(webrtc::IceCandidateInterface *)candidate;
nit: maybe initWithNativeCandidate

https://codereview.webrtc.org/1517253005/diff/40001/webrtc/api/objc/RTCIceCan...
File webrtc/api/objc/RTCIceCandidate.mm (right):

https://codereview.webrtc.org/1517253005/diff/40001/webrtc/api/objc/RTCIceCan...
webrtc/api/objc/RTCIceCandidate.mm:25: sdpMid:(nullable NSString *)sdpMid {
I don't think you need the nullable specifiers in impl file.

https://codereview.webrtc.org/1517253005/diff/40001/webrtc/api/objctests/RTCI...
File webrtc/api/objctests/RTCIceCandidateTest.mm (right):

https://codereview.webrtc.org/1517253005/diff/40001/webrtc/api/objctests/RTCI...
webrtc/api/objctests/RTCIceCandidateTest.mm:28:
"fdff:2642:12a6:fe38:c001:beda:fcf9:51aa "
align "

https://codereview.webrtc.org/1517253005/diff/40001/webrtc/api/objctests/RTCI...
webrtc/api/objctests/RTCIceCandidateTest.mm:46: std::string sdp =
"candidate:4025901590 1 udp 2122265343 "
nit: std::string sdp("candidate...");

https://codereview.webrtc.org/1517253005/diff/40001/webrtc/api/objctests/RTCI...
webrtc/api/objctests/RTCIceCandidateTest.mm:51: nativeCandidate =
webrtc::CreateIceCandidate("audio", 0, sdp, nullptr);
combine 49 and 51

https://codereview.webrtc.org/1517253005/diff/40001/webrtc/api/objctests/RTCI...
webrtc/api/objctests/RTCIceCandidateTest.mm:53: RTCIceCandidate *iceCandidate =
[[RTCIceCandidate alloc] initWithCandidate:
break after = instead

[hjon, 2016-01-05 18:34:17]

https://codereview.webrtc.org/1517253005/diff/40001/webrtc/api/objc/RTCIceCan...
File webrtc/api/objc/RTCIceCandidate+Private.h (right):

https://codereview.webrtc.org/1517253005/diff/40001/webrtc/api/objc/RTCIceCan...
webrtc/api/objc/RTCIceCandidate+Private.h:13: #include
"webrtc/base/scoped_ptr.h"
On 2016/01/05 16:18:07, tkchin_webrtc wrote:
> alphabetize

Done.

https://codereview.webrtc.org/1517253005/diff/40001/webrtc/api/objc/RTCIceCan...
webrtc/api/objc/RTCIceCandidate+Private.h:31: -
(instancetype)initWithCandidate:(webrtc::IceCandidateInterface *)candidate;
On 2016/01/05 16:18:07, tkchin_webrtc wrote:
> nit: maybe initWithNativeCandidate

Done.

https://codereview.webrtc.org/1517253005/diff/40001/webrtc/api/objc/RTCIceCan...
File webrtc/api/objc/RTCIceCandidate.mm (right):

https://codereview.webrtc.org/1517253005/diff/40001/webrtc/api/objc/RTCIceCan...
webrtc/api/objc/RTCIceCandidate.mm:25: sdpMid:(nullable NSString *)sdpMid {
On 2016/01/05 16:18:07, tkchin_webrtc wrote:
> I don't think you need the nullable specifiers in impl file.

Done.

https://codereview.webrtc.org/1517253005/diff/40001/webrtc/api/objctests/RTCI...
File webrtc/api/objctests/RTCIceCandidateTest.mm (right):

https://codereview.webrtc.org/1517253005/diff/40001/webrtc/api/objctests/RTCI...
webrtc/api/objctests/RTCIceCandidateTest.mm:28:
"fdff:2642:12a6:fe38:c001:beda:fcf9:51aa "
On 2016/01/05 16:18:07, tkchin_webrtc wrote:
> align "

Done.

https://codereview.webrtc.org/1517253005/diff/40001/webrtc/api/objctests/RTCI...
webrtc/api/objctests/RTCIceCandidateTest.mm:46: std::string sdp =
"candidate:4025901590 1 udp 2122265343 "
On 2016/01/05 16:18:07, tkchin_webrtc wrote:
> nit: std::string sdp("candidate...");

Done.

https://codereview.webrtc.org/1517253005/diff/40001/webrtc/api/objctests/RTCI...
webrtc/api/objctests/RTCIceCandidateTest.mm:51: nativeCandidate =
webrtc::CreateIceCandidate("audio", 0, sdp, nullptr);
On 2016/01/05 16:18:07, tkchin_webrtc wrote:
> combine 49 and 51

Done.

https://codereview.webrtc.org/1517253005/diff/40001/webrtc/api/objctests/RTCI...
webrtc/api/objctests/RTCIceCandidateTest.mm:53: RTCIceCandidate *iceCandidate =
[[RTCIceCandidate alloc] initWithCandidate:
On 2016/01/05 16:18:07, tkchin_webrtc wrote:
> break after = instead

Done.

[tkchin_webrtc, 2016-01-05 21:26:37]

https://codereview.webrtc.org/1517253005/diff/60001/webrtc/api/objc/RTCIceCan...
File webrtc/api/objc/RTCIceCandidate.mm (right):

https://codereview.webrtc.org/1517253005/diff/60001/webrtc/api/objc/RTCIceCan...
webrtc/api/objc/RTCIceCandidate.mm:59: candidate = webrtc::CreateIceCandidate(
combine 56 + 59

[hjon, 2016-01-06 00:46:33]

https://codereview.webrtc.org/1517253005/diff/60001/webrtc/api/objc/RTCIceCan...
File webrtc/api/objc/RTCIceCandidate.mm (right):

https://codereview.webrtc.org/1517253005/diff/60001/webrtc/api/objc/RTCIceCan...
webrtc/api/objc/RTCIceCandidate.mm:59: candidate = webrtc::CreateIceCandidate(
On 2016/01/05 21:26:37, tkchin_webrtc wrote:
> combine 56 + 59

Done.

[tkchin_webrtc, 2016-01-06 19:04:57]

The CQ bit was checked by tkchin@webrtc.org

[tkchin_webrtc, 2016-01-06 19:04:57]

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/1517253005/#ps100001 (title:
"Remove copy")

[commit-bot: I haz the power, 2016-01-06 19:05:00]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1517253005/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1517253005/100001

[commit-bot: I haz the power, 2016-01-06 19:05:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-01-06 19:05:41]

Try jobs failed on following builders:
  mac_compile_x64_dbg on tryserver.webrtc (JOB_FAILED, no build URL)

[tkchin_webrtc, 2016-01-06 19:38:30]

The CQ bit was checked by tkchin@webrtc.org

[commit-bot: I haz the power, 2016-01-06 19:38:42]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1517253005/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1517253005/100001

[commit-bot: I haz the power, 2016-01-06 19:39:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-01-06 19:39:10]

Try jobs failed on following builders:
  mac_compile_x64_dbg on tryserver.webrtc (JOB_FAILED, no build URL)

[tkchin_webrtc, 2016-01-06 19:49:23]

Description was changed from

==========
Update API for Objective-C RTCIceCandidate.

BUG=
==========

to

==========
Update API for Objective-C RTCIceCandidate.

BUG=
R=tkchin@webrtc.org

Committed:
https://chromium.googlesource.com/external/webrtc/+/29d5e570b5bacdd524535f58d...
==========

[tkchin_webrtc, 2016-01-06 19:49:24]

Committed patchset #6 (id:100001) manually as
29d5e570b5bacdd524535f58d2b367b8f5cdd052 (presubmit successful).

[commit-bot: I haz the power, 2016-01-06 19:49:26]

Description was changed from

==========
Update API for Objective-C RTCIceCandidate.

BUG=
R=tkchin@webrtc.org

Committed:
https://chromium.googlesource.com/external/webrtc/+/29d5e570b5bacdd524535f58d...
==========

to

==========
Update API for Objective-C RTCIceCandidate.

BUG=
R=tkchin@webrtc.org

Committed: https://crrev.com/29d5e570b5bacdd524535f58d2b367b8f5cdd052
Patch from Jon Hjelle <hjon@andyet.net>.

Cr-Commit-Position: refs/heads/master@{#11156}
==========

[commit-bot: I haz the power, 2016-01-06 19:49:27]

Patchset 6 (id:??) landed as
https://crrev.com/29d5e570b5bacdd524535f58d2b367b8f5cdd052
Cr-Commit-Position: refs/heads/master@{#11156}

[nicholss, 2016-02-05 21:45:59]

nicholss@chromium.org changed reviewers:
+ nicholss@chromium.org

[nicholss, 2016-02-05 21:46:00]

I am having issues with the following line in the chromium build:

'../../libjingle/source/talk/libjingle.gyp:libjingle_peerconnection',

in file webrtc/api/api.gyp

Could someone confirm this should be working?

https://codereview.webrtc.org/1517253005/diff/100001/webrtc/api/BUILD.gn
File webrtc/api/BUILD.gn (right):

https://codereview.webrtc.org/1517253005/diff/100001/webrtc/api/BUILD.gn#newc...
webrtc/api/BUILD.gn:15: #"//talk/libjingle:libjingle_peerconnection",
But it is commented out here?

Looks like all changes for .gn are commented out?

https://codereview.webrtc.org/1517253005/diff/100001/webrtc/api/api.gyp
File webrtc/api/api.gyp (right):

https://codereview.webrtc.org/1517253005/diff/100001/webrtc/api/api.gyp#newco...
webrtc/api/api.gyp:19: '../../talk/libjingle.gyp:libjingle_peerconnection',
Hello, this addition is failing on iOS builds in chromium. It looks to be
commented out in BUILD.gn and added here? Is there more details on this change?