WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
95565
MediaStream API: Add Ice-related functionality to RTCPeerConnection
https://bugs.webkit.org/show_bug.cgi?id=95565
Summary
MediaStream API: Add Ice-related functionality to RTCPeerConnection
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
Details
Formatted Diff
Diff
Patch
(54.01 KB, patch)
2012-09-03 03:57 PDT
,
Tommy Widenflycht
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Tommy Widenflycht
Comment 1
2012-08-31 06:40:38 PDT
Created
attachment 161688
[details]
Patch
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
Created
attachment 161894
[details]
Patch
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.
Top of Page
Format For Printing
XML
Clone This Bug