Before setting a session description RTCPeerConnection must check if it is in valid state for that SDP type.
Created attachment 219120 [details] Patch
Comment on attachment 219120 [details] Patch 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.
(In reply to comment #2) > (From update of attachment 219120 [details]) > 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. OK. > > > 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"? The "set" is because the action to set the SDP, this method is called when setLocalDescription is executed. But I will rename it to validadeLocalDescriptionState :) > > > 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.
Created attachment 219284 [details] Patch
I renamed the methods to checkStateForLocal/RemoteDescription, which I found more meaningful.
Comment on attachment 219284 [details] Patch Rejecting attachment 219284 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-01', 'apply-attachment', '--no-update', '--non-interactive', 219284, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 500 characters of output: pranswer.html patching file LayoutTests/fast/mediastream/RTCPeerConnection-remoteDescription-expected.txt patching file LayoutTests/fast/mediastream/RTCPeerConnection-remoteDescription.html patching file LayoutTests/fast/mediastream/RTCPeerConnection-stable-expected.txt patching file LayoutTests/fast/mediastream/RTCPeerConnection-stable.html Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Eric Carlson']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit Full output: http://webkit-queues.appspot.com/results/49908112
Created attachment 219382 [details] Patch for landing
Comment on attachment 219382 [details] Patch for landing Clearing flags on attachment: 219382 Committed r160693: <http://trac.webkit.org/changeset/160693>