Project: chromium Issues People Development process History Sign in
New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 4 users
Status: Fixed
Owner:
Closed: Mar 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: ----
Type: Feature

Blocking:
issue 581348



Sign in to add a comment
PeerConnection.setConfiguration
Project Member Reported by hta@chromium.org, Feb 17 2016 Back to list
Feature description: Support the setConfiguration call as specified.

Eng owner: pthatcher@chromium.org

WebRTC spec: http://w3c.github.io/webrtc-pc/#widl-RTCPeerConnection-getConfiguration-RTCConfiguration
 
Comment 1 by hta@chromium.org, Feb 17 2016
Blocking: chromium:581348
Comment 2 by hta@chromium.org, Feb 17 2016
Owner: deadbeef@chromium.org
Comment 3 by hbos@chromium.org, Apr 12 2016
Is this being worked on?
Comment 4 by hbos@chromium.org, Apr 12 2016
Cc: hbos@chromium.org
Comment 5 by deadbeef@webrtc.org, Apr 12 2016
Not actively at the moment, but it will probably be done next quarter.
Comment 6 by hbos@chromium.org, Apr 13 2016
Cc: hta@chromium.org
Components: Blink>WebRTC>Network
Labels: WebRTCTriaged
Status: Available
Project Member Comment 11 by bugdroid1@chromium.org, Dec 13 2016
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/08110857fc26f4a0a0725edef75ea658a9d4a4c8

commit 08110857fc26f4a0a0725edef75ea658a9d4a4c8
Author: deadbeef <deadbeef@chromium.org>
Date: Tue Dec 13 17:31:17 2016

Rename "updateICE" to "setConfiguration", everywhere except in Blink.

This CL prepares for later renaming updateIce to setConfiguration in
Blink, once it has an Intent to Ship.

updateIce was never functional in the first place (in the native webrtc
library), always throwing the same exception. SetConfiguration is
functional, and will allow an application to change the set of ICE
servers and ICE candidate policy. Soon, also the ICE candidate pool
size.

updateIce will no longer be tracked after this CL, which should be ok
since it has always been nonfunctional and stats show zero use.

BUG= chromium:587453 

Review-Url: https://codereview.chromium.org/2511633002
Cr-Commit-Position: refs/heads/master@{#438206}

[modify] https://crrev.com/08110857fc26f4a0a0725edef75ea658a9d4a4c8/components/test_runner/mock_webrtc_peer_connection_handler.cc
[modify] https://crrev.com/08110857fc26f4a0a0725edef75ea658a9d4a4c8/components/test_runner/mock_webrtc_peer_connection_handler.h
[modify] https://crrev.com/08110857fc26f4a0a0725edef75ea658a9d4a4c8/content/renderer/media/mock_peer_connection_impl.cc
[modify] https://crrev.com/08110857fc26f4a0a0725edef75ea658a9d4a4c8/content/renderer/media/mock_peer_connection_impl.h
[modify] https://crrev.com/08110857fc26f4a0a0725edef75ea658a9d4a4c8/content/renderer/media/peer_connection_tracker.cc
[modify] https://crrev.com/08110857fc26f4a0a0725edef75ea658a9d4a4c8/content/renderer/media/peer_connection_tracker.h
[modify] https://crrev.com/08110857fc26f4a0a0725edef75ea658a9d4a4c8/content/renderer/media/rtc_peer_connection_handler.cc
[modify] https://crrev.com/08110857fc26f4a0a0725edef75ea658a9d4a4c8/content/renderer/media/rtc_peer_connection_handler.h
[modify] https://crrev.com/08110857fc26f4a0a0725edef75ea658a9d4a4c8/content/renderer/media/rtc_peer_connection_handler_unittest.cc
[modify] https://crrev.com/08110857fc26f4a0a0725edef75ea658a9d4a4c8/third_party/WebKit/Source/modules/peerconnection/RTCPeerConnection.cpp
[modify] https://crrev.com/08110857fc26f4a0a0725edef75ea658a9d4a4c8/third_party/WebKit/Source/modules/peerconnection/RTCPeerConnection.h
[modify] https://crrev.com/08110857fc26f4a0a0725edef75ea658a9d4a4c8/third_party/WebKit/public/platform/WebRTCPeerConnectionHandler.h

Labels: M-58
Question for more experienced chromium/Blink developers: To implement getConfiguration, should Blink cache the previously set configuration, or should  PeerConnection::GetConfiguration be called and the result converted to a blink::WebRTCConfiguration?

Also, does someone else plan to work on getConfiguration, or should I stay the owner? setConfiguration is already ready to be shipped, so it would be great if we could ship [get/set]Configuration in M58.
Comment 13 by hbos@chromium.org, Jan 23 2017
I think getConfiguration should do a call all the way down to the webrtc layer.
There are optional stuff in the configuration that makes sense to leave out if you do a setConfiguration call to update something specific, but if set at any time should be returned when you do getConfiguration - this doesn't work with caching (unless you partially modify the cached version?). You should also be able to do getConfiguration without ever having called setConfiguration. Also I don't know but what if there are other things than setConfiguration, now or in the future, that can change the config.

We've not discussed doing this this milestone. If setConfiguration is done, does that mean getConfiguration is just a simple call away from being done? If so we should prioritize it for M58. :) Feel free to own it unless you want us to help out.
Yes, assuming we do what you describe, getConfiguration just needs some code to convert between webrtc:: and blink:: structures.
Project Member Comment 15 by bugdroid1@chromium.org, Jan 27 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9853d6c5883e860ff52b5447b17f50907ffee434

commit 9853d6c5883e860ff52b5447b17f50907ffee434
Author: deadbeef <deadbeef@chromium.org>
Date: Fri Jan 27 01:05:46 2017

Rename RTCPeerConnection.updateIce to setConfiguration and make it work.

In addition to the renaming/basic wiring up, this CL adds a mechanism
for passing error information between webrtc/chromium and blink.

In this CL, it's only used for an InvalidModificationError.

Intent to Implement and Ship setConfiguration:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/wh74Pd37LUQ/45Dx81Y2BgAJ

Intent to Deprecate and Remove updateIce:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/9gq36Wl4Jwo/a8-rZOqCAgAJ

BUG= chromium:587453 

Review-Url: https://codereview.chromium.org/2631433002
Cr-Commit-Position: refs/heads/master@{#446512}

[modify] https://crrev.com/9853d6c5883e860ff52b5447b17f50907ffee434/components/test_runner/mock_webrtc_peer_connection_handler.cc
[modify] https://crrev.com/9853d6c5883e860ff52b5447b17f50907ffee434/components/test_runner/mock_webrtc_peer_connection_handler.h
[modify] https://crrev.com/9853d6c5883e860ff52b5447b17f50907ffee434/content/browser/webrtc/webrtc_browsertest.cc
[modify] https://crrev.com/9853d6c5883e860ff52b5447b17f50907ffee434/content/renderer/media/mock_peer_connection_impl.cc
[modify] https://crrev.com/9853d6c5883e860ff52b5447b17f50907ffee434/content/renderer/media/mock_peer_connection_impl.h
[modify] https://crrev.com/9853d6c5883e860ff52b5447b17f50907ffee434/content/renderer/media/rtc_peer_connection_handler.cc
[modify] https://crrev.com/9853d6c5883e860ff52b5447b17f50907ffee434/content/renderer/media/rtc_peer_connection_handler.h
[modify] https://crrev.com/9853d6c5883e860ff52b5447b17f50907ffee434/content/renderer/media/rtc_peer_connection_handler_unittest.cc
[add] https://crrev.com/9853d6c5883e860ff52b5447b17f50907ffee434/content/test/data/media/peerconnection-setConfiguration.html
[modify] https://crrev.com/9853d6c5883e860ff52b5447b17f50907ffee434/third_party/WebKit/LayoutTests/external/wpt/webrtc/rtcpeerconnection/rtcpeerconnection-idl-expected.txt
[modify] https://crrev.com/9853d6c5883e860ff52b5447b17f50907ffee434/third_party/WebKit/LayoutTests/platform/mac/virtual/stable/webexposed/global-interface-listing-expected.txt
[modify] https://crrev.com/9853d6c5883e860ff52b5447b17f50907ffee434/third_party/WebKit/LayoutTests/platform/win/virtual/stable/webexposed/global-interface-listing-expected.txt
[modify] https://crrev.com/9853d6c5883e860ff52b5447b17f50907ffee434/third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt
[modify] https://crrev.com/9853d6c5883e860ff52b5447b17f50907ffee434/third_party/WebKit/Source/core/frame/UseCounter.h
[modify] https://crrev.com/9853d6c5883e860ff52b5447b17f50907ffee434/third_party/WebKit/Source/modules/peerconnection/RTCPeerConnection.cpp
[modify] https://crrev.com/9853d6c5883e860ff52b5447b17f50907ffee434/third_party/WebKit/Source/modules/peerconnection/RTCPeerConnection.h
[modify] https://crrev.com/9853d6c5883e860ff52b5447b17f50907ffee434/third_party/WebKit/Source/modules/peerconnection/RTCPeerConnection.idl
[modify] https://crrev.com/9853d6c5883e860ff52b5447b17f50907ffee434/third_party/WebKit/public/BUILD.gn
[add] https://crrev.com/9853d6c5883e860ff52b5447b17f50907ffee434/third_party/WebKit/public/platform/WebRTCError.h
[modify] https://crrev.com/9853d6c5883e860ff52b5447b17f50907ffee434/third_party/WebKit/public/platform/WebRTCPeerConnectionHandler.h

Should this be bumped to M59?
Comment 17 by hta@chromium.org, Mar 22 2017
Cc: guidou@chromium.org
Guidou, do we need to do anything?
Status: Fixed
Summary: PeerConnection.setConfiguration (was: PeerConnection.getConfiguration and setConfiguration)
setConfiguration is implemented in M58, and zhihuang@ has been working on a CL for getConfiguration. I'll make a new bug for getConfiguration.
Description: Show this description
Description: Show this description
Sign in to add a comment