Bug 96097 - MediaStream API: add RTCPeerConnection::onnegotiationneeded
Summary: MediaStream API: add RTCPeerConnection::onnegotiationneeded
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Tommy Widenflycht
URL:
Keywords: WebExposed
Depends on:
Blocks: 80589
  Show dependency treegraph
 
Reported: 2012-09-07 04:41 PDT by Tommy Widenflycht
Modified: 2012-09-11 04:13 PDT (History)
10 users (show)

See Also:


Attachments
Patch (12.50 KB, patch)
2012-09-07 04:45 PDT, Tommy Widenflycht
no flags Details | Formatted Diff | Diff
Patch (12.56 KB, patch)
2012-09-07 11:14 PDT, Tommy Widenflycht
abarth: review+
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
Patch (12.49 KB, patch)
2012-09-11 01:43 PDT, Tommy Widenflycht
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tommy Widenflycht 2012-09-07 04:41:02 PDT
Including test as usual.
Comment 1 Tommy Widenflycht 2012-09-07 04:45:01 PDT
Created attachment 162745 [details]
Patch
Comment 2 WebKit Review Bot 2012-09-07 04:47:18 PDT
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
Comment 3 Adam Barth 2012-09-07 10:31:07 PDT
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?
Comment 4 Adam Barth 2012-09-07 10:34:47 PDT
Is this something like requestNegotiation?  What sort of negotiation?
Comment 5 Tommy Widenflycht 2012-09-07 10:40:23 PDT
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.
Comment 6 Tommy Widenflycht 2012-09-07 10:51:26 PDT
Would you be happier with a more descriptive name? doNegotiateLink or doNegotiateConnection maybe?
Comment 7 Adam Barth 2012-09-07 10:57:38 PDT
The problem is just that "do" doesn't mean anything as a verb.

Perhaps just negotiationNeeded() to mirror the DOM event name?
Comment 8 Tommy Widenflycht 2012-09-07 11:14:10 PDT
Created attachment 162825 [details]
Patch
Comment 9 Tommy Widenflycht 2012-09-07 11:15:40 PDT
(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.
Comment 10 Adam Barth 2012-09-10 08:40:50 PDT
Comment on attachment 162825 [details]
Patch

Thanks!
Comment 11 WebKit Review Bot 2012-09-10 09:02:52 PDT
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
Comment 12 Tommy Widenflycht 2012-09-11 01:43:46 PDT
Created attachment 163302 [details]
Patch
Comment 13 Tommy Widenflycht 2012-09-11 01:44:36 PDT
Rebased.
Comment 14 WebKit Review Bot 2012-09-11 03:29:01 PDT
Comment on attachment 163302 [details]
Patch

Clearing flags on attachment: 163302

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