Update API for Objective-C RTCSessionDescription.

Update API for Objective-C RTCSessionDescription.

BUG=
R=tkchin@webrtc.org

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

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

[hjon, 2015-12-20 23:25:21]

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

[hjon, 2015-12-20 23:25:21]

Like https://codereview.webrtc.org/1527143002/, I'm not sure I like the naming
for the enum conversion methods. I'm also a little undecided about whether to
use an enum, string, or possibly a string constant for the description `type`.
Because the underlying implementation is a string, I'm leaning towards leaving
it as a string or defining some string constants.

[tkchin_webrtc, 2016-01-05 16:29:19]

https://codereview.webrtc.org/1524303006/diff/20001/webrtc/api/objc/RTCSessio...
File webrtc/api/objc/RTCSessionDescription+Private.h (right):

https://codereview.webrtc.org/1524303006/diff/20001/webrtc/api/objc/RTCSessio...
webrtc/api/objc/RTCSessionDescription+Private.h:32: -
(instancetype)initWithDescription:(webrtc::SessionDescriptionInterface *)
ditto break after :

https://codereview.webrtc.org/1524303006/diff/20001/webrtc/api/objc/RTCSessio...
File webrtc/api/objc/RTCSessionDescription.h (right):

https://codereview.webrtc.org/1524303006/diff/20001/webrtc/api/objc/RTCSessio...
webrtc/api/objc/RTCSessionDescription.h:18: @property(nonatomic, copy, readonly)
NSString *type;
copy is technically for setters only. Since we're not changing this to readwrite
in a private interface we can just omit it.

https://developer.apple.com/library/mac/documentation/Cocoa/Conceptual/Progra...

https://codereview.webrtc.org/1524303006/diff/20001/webrtc/api/objc/RTCSessio...
File webrtc/api/objc/RTCSessionDescription.mm (right):

https://codereview.webrtc.org/1524303006/diff/20001/webrtc/api/objc/RTCSessio...
webrtc/api/objc/RTCSessionDescription.mm:15: #import
"webrtc/base/objc/RTCLogging.h"
remove space

https://codereview.webrtc.org/1524303006/diff/20001/webrtc/api/objc/RTCSessio...
webrtc/api/objc/RTCSessionDescription.mm:26: _type = [type copy];
indents

https://codereview.webrtc.org/1524303006/diff/20001/webrtc/api/objc/RTCSessio...
webrtc/api/objc/RTCSessionDescription.mm:44: description =
webrtc::CreateSessionDescription(_type.stdString,
combine 41 and 44?

https://codereview.webrtc.org/1524303006/diff/20001/webrtc/api/objc/RTCSessio...
webrtc/api/objc/RTCSessionDescription.mm:57: -
(instancetype)initWithDescription:(webrtc::SessionDescriptionInterface *)
break after : instead of after *)

https://codereview.webrtc.org/1524303006/diff/20001/webrtc/api/objc/RTCSessio...
webrtc/api/objc/RTCSessionDescription.mm:63: return [self initWithType:[NSString
stringForStdString:description->type()]
seems like initWithType:sdp: can be a designated initializer then

https://codereview.webrtc.org/1524303006/diff/20001/webrtc/api/objctests/RTCS...
File webrtc/api/objctests/RTCSessionDescriptionTest.mm (right):

https://codereview.webrtc.org/1524303006/diff/20001/webrtc/api/objctests/RTCS...
webrtc/api/objctests/RTCSessionDescriptionTest.mm:26: -
(void)testSessionDescription {
use more descriptive method name or add comments for what the test is testing

[tkchin_webrtc, 2016-01-05 16:44:53]

On 2015/12/20 23:25:21, hjon wrote:
> Like https://codereview.webrtc.org/1527143002/, I'm not sure I like the naming
> for the enum conversion methods. I'm also a little undecided about whether to
> use an enum, string, or possibly a string constant for the description `type`.
> Because the underlying implementation is a string, I'm leaning towards leaving
> it as a string or defining some string constants.

I'd suggest an enum, which is translated under the hood to string constants to
be passed down to C++.

[hjon, 2016-01-05 22:16:20]

https://codereview.webrtc.org/1524303006/diff/20001/webrtc/api/objc/RTCSessio...
File webrtc/api/objc/RTCSessionDescription+Private.h (right):

https://codereview.webrtc.org/1524303006/diff/20001/webrtc/api/objc/RTCSessio...
webrtc/api/objc/RTCSessionDescription+Private.h:32: -
(instancetype)initWithDescription:(webrtc::SessionDescriptionInterface *)
On 2016/01/05 16:29:18, tkchin_webrtc wrote:
> ditto break after :

Done.

https://codereview.webrtc.org/1524303006/diff/20001/webrtc/api/objc/RTCSessio...
File webrtc/api/objc/RTCSessionDescription.h (right):

https://codereview.webrtc.org/1524303006/diff/20001/webrtc/api/objc/RTCSessio...
webrtc/api/objc/RTCSessionDescription.h:18: @property(nonatomic, copy, readonly)
NSString *type;
On 2016/01/05 16:29:18, tkchin_webrtc wrote:
> copy is technically for setters only. Since we're not changing this to
readwrite
> in a private interface we can just omit it.
> 
>
https://developer.apple.com/library/mac/documentation/Cocoa/Conceptual/Progra...

Done.

https://codereview.webrtc.org/1524303006/diff/20001/webrtc/api/objc/RTCSessio...
File webrtc/api/objc/RTCSessionDescription.mm (right):

https://codereview.webrtc.org/1524303006/diff/20001/webrtc/api/objc/RTCSessio...
webrtc/api/objc/RTCSessionDescription.mm:15: #import
"webrtc/base/objc/RTCLogging.h"
On 2016/01/05 16:29:18, tkchin_webrtc wrote:
> remove space

Done.

https://codereview.webrtc.org/1524303006/diff/20001/webrtc/api/objc/RTCSessio...
webrtc/api/objc/RTCSessionDescription.mm:26: _type = [type copy];
On 2016/01/05 16:29:19, tkchin_webrtc wrote:
> indents

Done.

https://codereview.webrtc.org/1524303006/diff/20001/webrtc/api/objc/RTCSessio...
webrtc/api/objc/RTCSessionDescription.mm:44: description =
webrtc::CreateSessionDescription(_type.stdString,
On 2016/01/05 16:29:18, tkchin_webrtc wrote:
> combine 41 and 44?

Done.

https://codereview.webrtc.org/1524303006/diff/20001/webrtc/api/objc/RTCSessio...
webrtc/api/objc/RTCSessionDescription.mm:57: -
(instancetype)initWithDescription:(webrtc::SessionDescriptionInterface *)
On 2016/01/05 16:29:19, tkchin_webrtc wrote:
> break after : instead of after *)

Done.

https://codereview.webrtc.org/1524303006/diff/20001/webrtc/api/objc/RTCSessio...
webrtc/api/objc/RTCSessionDescription.mm:63: return [self initWithType:[NSString
stringForStdString:description->type()]
On 2016/01/05 16:29:19, tkchin_webrtc wrote:
> seems like initWithType:sdp: can be a designated initializer then

Done.

https://codereview.webrtc.org/1524303006/diff/20001/webrtc/api/objctests/RTCS...
File webrtc/api/objctests/RTCSessionDescriptionTest.mm (right):

https://codereview.webrtc.org/1524303006/diff/20001/webrtc/api/objctests/RTCS...
webrtc/api/objctests/RTCSessionDescriptionTest.mm:26: -
(void)testSessionDescription {
On 2016/01/05 16:29:19, tkchin_webrtc wrote:
> use more descriptive method name or add comments for what the test is testing

Done.

https://codereview.webrtc.org/1524303006/diff/40001/webrtc/api/objc/RTCSessio...
File webrtc/api/objc/RTCSessionDescription.mm (right):

https://codereview.webrtc.org/1524303006/diff/40001/webrtc/api/objc/RTCSessio...
webrtc/api/objc/RTCSessionDescription.mm:88: NSAssert(false, @"|string| is not a
known SDP type.");
tkchin@ What do you think about this method? Conceptually, this is how I'd like
it to look, but of course the compiler warns about this final else clause simply
returning from a non-void function. I can't currently think of a way to
accomplish this that doesn't seem a little awkward.

[tkchin_webrtc, 2016-01-06 19:42:08]

lgtm

[tkchin_webrtc, 2016-01-06 19:50:17]

The CQ bit was checked by tkchin@webrtc.org

[tkchin_webrtc, 2016-01-06 19:50:17]

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/1524303006/#ps100001 (title:
"Change #import to #include")

[commit-bot: I haz the power, 2016-01-06 19:50:21]

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

[commit-bot: I haz the power, 2016-01-06 19:51:04]

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

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

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

[tkchin_webrtc, 2016-01-06 20:05:36]

Description was changed from

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

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

to

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

BUG=
R=tkchin@webrtc.org

Committed:
https://chromium.googlesource.com/external/webrtc/+/67e83d653968f1147f7cb8645...
==========

[tkchin_webrtc, 2016-01-06 20:05:37]

Committed patchset #6 (id:100001) manually as
67e83d653968f1147f7cb86451c4f4494751643a (presubmit successful).

[commit-bot: I haz the power, 2016-01-06 20:05:37]

Description was changed from

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

BUG=
R=tkchin@webrtc.org

Committed:
https://chromium.googlesource.com/external/webrtc/+/67e83d653968f1147f7cb8645...
==========

to

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

BUG=
R=tkchin@webrtc.org

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

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

[commit-bot: I haz the power, 2016-01-06 20:05:38]

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