Bug 96097 – MediaStream API: add RTCPeerConnection::onnegotiationneeded

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Tommy Widenflycht]

Including test as usual.

[Tommy Widenflycht]

Created attachment 162745 [details]
Patch

[WebKit Review Bot]

Please wait for approval from [e-mail], [e-mail], [e-mail], [e-mail] or [e-mail] before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.

[Adam Barth]

Comment on attachment 162745 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=162745&action=review

> Source/Platform/chromium/public/WebRTCPeerConnectionHandlerClient.h:61
> +    virtual void doNegotiate() = 0;

What does the "do" prefix mean here?

[Adam Barth]

Is this something like requestNegotiation?  What sort of negotiation?

[Tommy Widenflycht]

Comment on attachment 162745 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=162745&action=review

>> Source/Platform/chromium/public/WebRTCPeerConnectionHandlerClient.h:61
>> +    virtual void doNegotiate() = 0;
> 
> What does the "do" prefix mean here?

Here I actually had a bit of information in the ChangeLog regarding this but here's a longer version:

The onnegotiationnedded is sent from the network stack in the UA when some state changes which requires renegotiation with the other side. The negotiation is done by the app calling createOffer and sending that SessionDescription to the other PeerConnection over the web link.

This is for example triggered by adding/removing tracks on the PeerConnection.

So in short this is a command that the app has to re-negotiate the connection.

[Tommy Widenflycht]

Would you be happier with a more descriptive name? doNegotiateLink or doNegotiateConnection maybe?

[Adam Barth]

The problem is just that "do" doesn't mean anything as a verb.

Perhaps just negotiationNeeded() to mirror the DOM event name?

[Tommy Widenflycht]

Created attachment 162825 [details]
Patch

[Tommy Widenflycht]

(In reply to comment #7)
> The problem is just that "do" doesn't mean anything as a verb.
> 
> Perhaps just negotiationNeeded() to mirror the DOM event name?

Done, that works just fine by me.

[Adam Barth]

Comment on attachment 162825 [details]
Patch

Thanks!

[WebKit Review Bot]

Comment on attachment 162825 [details]
Patch

Rejecting attachment 162825 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
mit-queue/Source/WebKit/chromium/third_party/skia/src --revision 5408 --non-interactive --force --accept theirs-conflict --ignore-externals' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
47>At revision 5408.

________ running '/usr/bin/python tools/clang/scripts/update.py --mac-only' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'

________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
Updating webkit projects from gyp files...

Full output: http://queues.webkit.org/results/13792989

[Tommy Widenflycht]

Created attachment 163302 [details]
Patch

[Tommy Widenflycht]

Rebased.

[WebKit Review Bot]

Comment on attachment 163302 [details]
Patch

Clearing flags on attachment: 163302

Committed r128166: <http://trac.webkit.org/changeset/128166>