Update API for Objective-C RTCConfiguration.

Update API for Objective-C RTCConfiguration.

BUG=

Committed: https://crrev.com/6d49a8ed171c76fefb80f31629001c4c42f08996
Cr-Commit-Position: refs/heads/master@{#11386}

[hjon_webrtc, 2016-01-22 17:47:18]

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

[tkchin_webrtc, 2016-01-22 21:17:04]

https://codereview.webrtc.org/1616303002/diff/20001/webrtc/api/objc/RTCConfig...
File webrtc/api/objc/RTCConfiguration.h (right):

https://codereview.webrtc.org/1616303002/diff/20001/webrtc/api/objc/RTCConfig...
webrtc/api/objc/RTCConfiguration.h:50: @property(nonatomic, readonly, nullable)
NSArray<RTCIceServer *> *iceServers;
nullable or returns empty array?

https://codereview.webrtc.org/1616303002/diff/20001/webrtc/api/objc/RTCConfig...
File webrtc/api/objc/RTCConfiguration.mm (right):

https://codereview.webrtc.org/1616303002/diff/20001/webrtc/api/objc/RTCConfig...
webrtc/api/objc/RTCConfiguration.mm:146:
nativeTransportsTypeForTransportPolicy:(RTCIceTransportPolicy)policy {
is this because it's called policy in the spec?

https://codereview.webrtc.org/1616303002/diff/20001/webrtc/api/objc/RTCConfig...
webrtc/api/objc/RTCConfiguration.mm:176: return @"None";
personally I think all these are best all caps
e.g. RELAY, NO_HOST

https://codereview.webrtc.org/1616303002/diff/20001/webrtc/api/objc/RTCIceSer...
File webrtc/api/objc/RTCIceServer.mm (right):

https://codereview.webrtc.org/1616303002/diff/20001/webrtc/api/objc/RTCIceSer...
webrtc/api/objc/RTCIceServer.mm:67: [NSMutableArray
arrayWithCapacity:nativeServer.urls.size()];
indent 4

https://codereview.webrtc.org/1616303002/diff/20001/webrtc/api/objctests/RTCC...
File webrtc/api/objctests/RTCConfigurationTest.mm (right):

https://codereview.webrtc.org/1616303002/diff/20001/webrtc/api/objctests/RTCC...
webrtc/api/objctests/RTCConfigurationTest.mm:30: RTCIceServer *server =
[[RTCIceServer alloc] initWithURLStrings:@[
separate out the array in its own var

https://codereview.webrtc.org/1616303002/diff/20001/webrtc/api/objctests/RTCC...
webrtc/api/objctests/RTCConfigurationTest.mm:34: [[RTCConfiguration alloc]
initWithIceServers:@[server]
@[ server ]

https://codereview.webrtc.org/1616303002/diff/20001/webrtc/api/objctests/RTCC...
webrtc/api/objctests/RTCConfigurationTest.mm:45: EXPECT_EQ((size_t)1,
nativeConfig.servers.size());
1u?

[hjon_webrtc, 2016-01-22 21:35:23]

https://codereview.webrtc.org/1616303002/diff/20001/webrtc/api/objc/RTCConfig...
File webrtc/api/objc/RTCConfiguration.h (right):

https://codereview.webrtc.org/1616303002/diff/20001/webrtc/api/objc/RTCConfig...
webrtc/api/objc/RTCConfiguration.h:50: @property(nonatomic, readonly, nullable)
NSArray<RTCIceServer *> *iceServers;
On 2016/01/22 21:17:04, tkchin_webrtc wrote:
> nullable or returns empty array?

Done.

https://codereview.webrtc.org/1616303002/diff/20001/webrtc/api/objc/RTCConfig...
File webrtc/api/objc/RTCConfiguration.mm (right):

https://codereview.webrtc.org/1616303002/diff/20001/webrtc/api/objc/RTCConfig...
webrtc/api/objc/RTCConfiguration.mm:146:
nativeTransportsTypeForTransportPolicy:(RTCIceTransportPolicy)policy {
On 2016/01/22 21:17:04, tkchin_webrtc wrote:
> is this because it's called policy in the spec?

Correct. Added note in the header file calling this out.

https://codereview.webrtc.org/1616303002/diff/20001/webrtc/api/objc/RTCConfig...
webrtc/api/objc/RTCConfiguration.mm:176: return @"None";
On 2016/01/22 21:17:04, tkchin_webrtc wrote:
> personally I think all these are best all caps
> e.g. RELAY, NO_HOST

Done.

https://codereview.webrtc.org/1616303002/diff/20001/webrtc/api/objc/RTCIceSer...
File webrtc/api/objc/RTCIceServer.mm (right):

https://codereview.webrtc.org/1616303002/diff/20001/webrtc/api/objc/RTCIceSer...
webrtc/api/objc/RTCIceServer.mm:67: [NSMutableArray
arrayWithCapacity:nativeServer.urls.size()];
On 2016/01/22 21:17:04, tkchin_webrtc wrote:
> indent 4

Done.

https://codereview.webrtc.org/1616303002/diff/20001/webrtc/api/objctests/RTCC...
File webrtc/api/objctests/RTCConfigurationTest.mm (right):

https://codereview.webrtc.org/1616303002/diff/20001/webrtc/api/objctests/RTCC...
webrtc/api/objctests/RTCConfigurationTest.mm:30: RTCIceServer *server =
[[RTCIceServer alloc] initWithURLStrings:@[
On 2016/01/22 21:17:04, tkchin_webrtc wrote:
> separate out the array in its own var

Done.

https://codereview.webrtc.org/1616303002/diff/20001/webrtc/api/objctests/RTCC...
webrtc/api/objctests/RTCConfigurationTest.mm:34: [[RTCConfiguration alloc]
initWithIceServers:@[server]
On 2016/01/22 21:17:04, tkchin_webrtc wrote:
> @[ server ]

Done.

https://codereview.webrtc.org/1616303002/diff/20001/webrtc/api/objctests/RTCC...
webrtc/api/objctests/RTCConfigurationTest.mm:45: EXPECT_EQ((size_t)1,
nativeConfig.servers.size());
On 2016/01/22 21:17:04, tkchin_webrtc wrote:
> 1u?

Done.

[tkchin_webrtc, 2016-01-22 22:29:14]

https://codereview.webrtc.org/1616303002/diff/40001/webrtc/api/objc/RTCConfig...
File webrtc/api/objc/RTCConfiguration.mm (right):

https://codereview.webrtc.org/1616303002/diff/40001/webrtc/api/objc/RTCConfig...
webrtc/api/objc/RTCConfiguration.mm:56:
iceBackupCandidatePairPingInterval:(int)iceBackupCandidatePairPingInterval {
indent 4 and align : with the line that was indented by 4

https://codereview.webrtc.org/1616303002/diff/40001/webrtc/api/objc/RTCConfig...
webrtc/api/objc/RTCConfiguration.mm:74: "%d\n%d\n%d\n",
I'm okay with an indent 4 here

return [NSString...
    _iceServers,
    [[self class]....
];

https://codereview.webrtc.org/1616303002/diff/40001/webrtc/api/objc/RTCConfig...
webrtc/api/objc/RTCConfiguration.mm:214: return @"Balanced";
ditto caps here and elsewhere

[hjon_webrtc, 2016-01-22 22:37:41]

https://codereview.webrtc.org/1616303002/diff/40001/webrtc/api/objc/RTCConfig...
File webrtc/api/objc/RTCConfiguration.mm (right):

https://codereview.webrtc.org/1616303002/diff/40001/webrtc/api/objc/RTCConfig...
webrtc/api/objc/RTCConfiguration.mm:56:
iceBackupCandidatePairPingInterval:(int)iceBackupCandidatePairPingInterval {
On 2016/01/22 22:29:14, tkchin_webrtc wrote:
> indent 4 and align : with the line that was indented by 4

Done.

https://codereview.webrtc.org/1616303002/diff/40001/webrtc/api/objc/RTCConfig...
webrtc/api/objc/RTCConfiguration.mm:74: "%d\n%d\n%d\n",
On 2016/01/22 22:29:14, tkchin_webrtc wrote:
> I'm okay with an indent 4 here
> 
> return [NSString...
>     _iceServers,
>     [[self class]....
> ];

Done.

https://codereview.webrtc.org/1616303002/diff/40001/webrtc/api/objc/RTCConfig...
webrtc/api/objc/RTCConfiguration.mm:214: return @"Balanced";
On 2016/01/22 22:29:14, tkchin_webrtc wrote:
> ditto caps here and elsewhere

Done.

[tkchin_webrtc, 2016-01-22 22:49:57]

nits.

lgtm

https://codereview.webrtc.org/1616303002/diff/80001/webrtc/api/BUILD.gn
File webrtc/api/BUILD.gn (right):

https://codereview.webrtc.org/1616303002/diff/80001/webrtc/api/BUILD.gn#newco...
webrtc/api/BUILD.gn:47: #"objc/RTCMediaSource+Private.h",
? some duplication going on here

https://codereview.webrtc.org/1616303002/diff/80001/webrtc/api/objc/RTCConfig...
File webrtc/api/objc/RTCConfiguration.mm (right):

https://codereview.webrtc.org/1616303002/diff/80001/webrtc/api/objc/RTCConfig...
webrtc/api/objc/RTCConfiguration.mm:74: @"RTCConfiguration:
{\n%@\n%@\n%@\n%@\n%@\n%d\n%d\n%d\n}\n",
indent 4 not 2

https://codereview.webrtc.org/1616303002/diff/80001/webrtc/api/objc/RTCConfig...
webrtc/api/objc/RTCConfiguration.mm:270: return @"ENABLED";
suggest TCP_ENABLED/DISABLED

https://codereview.webrtc.org/1616303002/diff/80001/webrtc/api/objctests/RTCC...
File webrtc/api/objctests/RTCConfigurationTest.mm (right):

https://codereview.webrtc.org/1616303002/diff/80001/webrtc/api/objctests/RTCC...
webrtc/api/objctests/RTCConfigurationTest.mm:89: EXPECT_EQ((size_t)1,
config.iceServers.count);
update these to 1u too

https://codereview.webrtc.org/1616303002/diff/80001/webrtc/api/objctests/RTCI...
File webrtc/api/objctests/RTCIceServerTest.mm (right):

https://codereview.webrtc.org/1616303002/diff/80001/webrtc/api/objctests/RTCI...
webrtc/api/objctests/RTCIceServerTest.mm:73: EXPECT_EQ((size_t)1,
iceServer.urlStrings.count);
ditto

[hjon_webrtc, 2016-01-22 23:07:33]

The CQ bit was checked by hjon@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2016-01-22 23:07:40]

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

[commit-bot: I haz the power, 2016-01-23 00:08:26]

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

[commit-bot: I haz the power, 2016-01-23 00:08:26]

Dry run: This issue passed the CQ dry run.

[hjon_webrtc, 2016-01-25 19:25:03]

tkchin_webrtc@ I should be good to land this via CQ, right?

https://codereview.webrtc.org/1616303002/diff/80001/webrtc/api/BUILD.gn
File webrtc/api/BUILD.gn (right):

https://codereview.webrtc.org/1616303002/diff/80001/webrtc/api/BUILD.gn#newco...
webrtc/api/BUILD.gn:47: #"objc/RTCMediaSource+Private.h",
On 2016/01/22 22:49:56, tkchin_webrtc wrote:
> ? some duplication going on here

Done.

https://codereview.webrtc.org/1616303002/diff/80001/webrtc/api/objc/RTCConfig...
File webrtc/api/objc/RTCConfiguration.mm (right):

https://codereview.webrtc.org/1616303002/diff/80001/webrtc/api/objc/RTCConfig...
webrtc/api/objc/RTCConfiguration.mm:74: @"RTCConfiguration:
{\n%@\n%@\n%@\n%@\n%@\n%d\n%d\n%d\n}\n",
On 2016/01/22 22:49:56, tkchin_webrtc wrote:
> indent 4 not 2

Done.

https://codereview.webrtc.org/1616303002/diff/80001/webrtc/api/objc/RTCConfig...
webrtc/api/objc/RTCConfiguration.mm:270: return @"ENABLED";
On 2016/01/22 22:49:56, tkchin_webrtc wrote:
> suggest TCP_ENABLED/DISABLED

Done.

https://codereview.webrtc.org/1616303002/diff/80001/webrtc/api/objctests/RTCC...
File webrtc/api/objctests/RTCConfigurationTest.mm (right):

https://codereview.webrtc.org/1616303002/diff/80001/webrtc/api/objctests/RTCC...
webrtc/api/objctests/RTCConfigurationTest.mm:89: EXPECT_EQ((size_t)1,
config.iceServers.count);
On 2016/01/22 22:49:57, tkchin_webrtc wrote:
> update these to 1u too

Done.

https://codereview.webrtc.org/1616303002/diff/80001/webrtc/api/objctests/RTCI...
File webrtc/api/objctests/RTCIceServerTest.mm (right):

https://codereview.webrtc.org/1616303002/diff/80001/webrtc/api/objctests/RTCI...
webrtc/api/objctests/RTCIceServerTest.mm:73: EXPECT_EQ((size_t)1,
iceServer.urlStrings.count);
On 2016/01/22 22:49:57, tkchin_webrtc wrote:
> ditto

Done.

[tkchin_webrtc, 2016-01-26 20:06:53]

The CQ bit was checked by tkchin@webrtc.org

[tkchin_webrtc, 2016-01-26 20:06:53]

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/1616303002/#ps100001 (title:
"Changes based on feedback")

[commit-bot: I haz the power, 2016-01-26 20:06:56]

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

[commit-bot: I haz the power, 2016-01-26 21:06:45]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-01-26 21:06:55]

Description was changed from

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

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

to

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

BUG=

Committed: https://crrev.com/6d49a8ed171c76fefb80f31629001c4c42f08996
Cr-Commit-Position: refs/heads/master@{#11386}
==========

[commit-bot: I haz the power, 2016-01-26 21:06:56]

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