Bug 96097

Summary: MediaStream API: add RTCPeerConnection::onnegotiationneeded
Product: WebKit Reporter: Tommy Widenflycht <tommyw>
Component: WebCore Misc.Assignee: Tommy Widenflycht <tommyw>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, dglazkov, eric.carlson, feature-media-reviews, fishd, hta, jamesr, ojan, tkent+wkapi, webkit.review.bot
Priority: P2 Keywords: WebExposed
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 80589    
Attachments:
Description Flags
Patch
none
Patch
abarth: review+, webkit.review.bot: commit-queue-
Patch none

Tommy Widenflycht
Reported 2012-09-07 04:41:02 PDT
Including test as usual.
Attachments
Patch (12.50 KB, patch)
2012-09-07 04:45 PDT, Tommy Widenflycht 		no flags
DetailsFormatted DiffDiff
Patch (12.56 KB, patch)
2012-09-07 11:14 PDT, Tommy Widenflycht 		abarth: review+
webkit.review.bot: commit-queue-
DetailsFormatted DiffDiff
Patch (12.49 KB, patch)
2012-09-11 01:43 PDT, Tommy Widenflycht 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Tommy Widenflycht
Comment 1 2012-09-07 04:45:01 PDT
Created attachment 162745 [details] Patch
WebKit Review Bot
Comment 2 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.
Adam Barth
Comment 3 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?
Adam Barth
Comment 4 2012-09-07 10:34:47 PDT
Is this something like requestNegotiation? What sort of negotiation?
Tommy Widenflycht
Comment 5 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.
Tommy Widenflycht
Comment 6 2012-09-07 10:51:26 PDT
Would you be happier with a more descriptive name? doNegotiateLink or doNegotiateConnection maybe?
Adam Barth
Comment 7 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?
Tommy Widenflycht
Comment 8 2012-09-07 11:14:10 PDT
Created attachment 162825 [details] Patch
Tommy Widenflycht
Comment 9 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.
Adam Barth
Comment 10 2012-09-10 08:40:50 PDT
Comment on attachment 162825 [details] Patch Thanks!
WebKit Review Bot
Comment 11 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
Tommy Widenflycht
Comment 12 2012-09-11 01:43:46 PDT
Created attachment 163302 [details] Patch
Tommy Widenflycht
Comment 13 2012-09-11 01:44:36 PDT
Rebased.
WebKit Review Bot
Comment 14 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>
Note You need to log in before you can comment on or make changes to this bug.