[webkit-reviews] review granted: [Bug 125655] Checking RTCPeerConnection signalingState before setting local/remoteDescription : [Attachment 219120] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Dec 13 13:33:49 PST 2013


Eric Carlson <eric.carlson at apple.com> has granted Thiago de Barros Lacerda
<thiago.lacerda at openbossa.org>'s request for review:
Bug 125655: Checking RTCPeerConnection signalingState before setting
local/remoteDescription
https://bugs.webkit.org/show_bug.cgi?id=125655

Attachment 219120: Patch
https://bugs.webkit.org/attachment.cgi?id=219120&action=review

------- Additional Comments from Eric Carlson <eric.carlson at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=219120&action=review


> Source/WebCore/ChangeLog:21
> +	   * Modules/mediastream/RTCPeerConnection.cpp:
> +	   (WebCore::RTCPeerConnection::validateSetLocalDescription):
> +	   (WebCore::RTCPeerConnection::validateSetRemoteDescription):
> +	   (WebCore::RTCPeerConnection::setLocalDescription):
> +	   (WebCore::RTCPeerConnection::setRemoteDescription):

Nit: as before, I think it is helpful to include a brief description of what
changed for each method listed in the ChangeLog.

> Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp:215
> +bool RTCPeerConnection::validateSetLocalDescription(RTCSessionDescription*
localDescription)

Nit: I don't think "validateSetLocalDescription" is a great name, what is
"Set"? Maybe something like "validateLocalSessionDescription", or
"validateLocalDescriptionState"?

> Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp:227
> +bool RTCPeerConnection::validateSetRemoteDescription(RTCSessionDescription*
remoteDescription)

Ditto about "Set", it doesn't help me understand what the method does.


More information about the webkit-reviews mailing list