Bug 95565

Summary: MediaStream API: Add Ice-related functionality to RTCPeerConnection
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, gyuyoung.kim, jamesr, ojan, rakuco, tkent+wkapi, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 80589    
Attachments:
Description Flags
Patch
none
Patch none

Tommy Widenflycht
Reported 2012-08-31 06:29:12 PDT
Contains the entire slice of functionality.
Attachments
Patch (51.91 KB, patch)
2012-08-31 06:40 PDT, Tommy Widenflycht
no flags
Patch (54.01 KB, patch)
2012-09-03 03:57 PDT, Tommy Widenflycht
no flags
Tommy Widenflycht
Comment 1 2012-08-31 06:40:38 PDT
WebKit Review Bot
Comment 2 2012-08-31 06:43:16 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-01 00:42:53 PDT
Comment on attachment 161688 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=161688&action=review > Source/WebCore/Modules/mediastream/RTCIceCandidateEvent.idl:30 > + readonly attribute RTCIceCandidate candidate; Bad indent. I think we have this in a bunch of event interfaces. > Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp:149 > +void RTCPeerConnection::addIceCandidate(PassRefPtr<RTCIceCandidate> prpIceCandidate, ExceptionCode& ec) It's not clear to me why this function need to take a reference to the RTCIceCandidate > Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp:190 > + return "new"; ASCIILiteral
WebKit Review Bot
Comment 4 2012-09-01 01:05:24 PDT
Comment on attachment 161688 [details] Patch Rejecting attachment 161688 [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: Kit/chromium/third_party/yasm/source/patched-yasm --revision 134927 --non-interactive --force --accept theirs-conflict --ignore-externals' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' 50>At revision 134927. ________ 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/13713530
Adam Barth
Comment 5 2012-09-01 01:09:45 PDT
Hunk #1 FAILED at 44. 1 out of 1 hunk FAILED -- saving rejects to file Tools/DumpRenderTree/chromium/MockWebRTCPeerConnectionHandler.h.rej
Tommy Widenflycht
Comment 6 2012-09-03 03:53:07 PDT
Comment on attachment 161688 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=161688&action=review >> Source/WebCore/Modules/mediastream/RTCIceCandidateEvent.idl:30 >> + readonly attribute RTCIceCandidate candidate; > > Bad indent. I think we have this in a bunch of event interfaces. Fixed. >> Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp:149 >> +void RTCPeerConnection::addIceCandidate(PassRefPtr<RTCIceCandidate> prpIceCandidate, ExceptionCode& ec) > > It's not clear to me why this function need to take a reference to the RTCIceCandidate You are right. Fixed. >> Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp:190 >> + return "new"; > > ASCIILiteral Done.
Tommy Widenflycht
Comment 7 2012-09-03 03:57:49 PDT
WebKit Review Bot
Comment 8 2012-09-03 05:17:12 PDT
Comment on attachment 161894 [details] Patch Attachment 161894 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13753046 New failing tests: http/tests/cache/subresource-expiration-1.html http/tests/cache/stopped-revalidation.html
Tommy Widenflycht
Comment 9 2012-09-03 05:33:13 PDT
cr-linux LayoutTest failure is unrelated to my patch.
Adam Barth
Comment 10 2012-09-03 08:40:51 PDT
Comment on attachment 161894 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=161894&action=review > Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp:186 > - return ""; > + return ASCIILiteral(""); You don't actually need this here. The benefit of ASCIILiteral is that it avoid a strlen, a malloc, and a memcpy, none of which happen for the empty string. :)
WebKit Review Bot
Comment 11 2012-09-03 09:07:46 PDT
Comment on attachment 161894 [details] Patch Clearing flags on attachment: 161894 Committed r127425: <http://trac.webkit.org/changeset/127425>
WebKit Review Bot
Comment 12 2012-09-03 09:07:51 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.