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

third_party/WebKit/Source/modules/peerconnection/RTCPeerConnection.idl

Index: third_party/WebKit/Source/modules/peerconnection/RTCPeerConnection.idl
diff --git a/third_party/WebKit/Source/modules/peerconnection/RTCPeerConnection.idl b/third_party/WebKit/Source/modules/peerconnection/RTCPeerConnection.idl
index 2e48d3ee8ebb1883df9c06a7b7a5a0cc86617bbb..371966228ba30553f5274ef31394874f4c3b8606 100644
--- a/third_party/WebKit/Source/modules/peerconnection/RTCPeerConnection.idl
+++ b/third_party/WebKit/Source/modules/peerconnection/RTCPeerConnection.idl
@@ -84,7 +84,7 @@ enum RTCIceConnectionState {
     readonly attribute RTCIceConnectionState iceConnectionState;
     // readonly attribute boolean? canTrickleIceCandidates;
     // RTCConfiguration getConfiguration();
-    // void setConfiguration(RTCConfiguration configuration);
+    [CallWith=ExecutionContext, RaisesException] void setConfiguration (RTCConfiguration configuration);

[foolip, 2016/11/17 10:10:54]

This is great, if the implementation already does what
https://w3c.github.io/webrtc-pc/#dom-rtcpeerconnection-setconfiguration says
then we should totally ship it.

Because this changes the web-exposed API, it'll need an "Intent to Ship". And
removing updateIce below will need an "Intent to Deprecate and Remove". See
https://www.chromium.org/blink#launch-process

Tests for setConfiguration will be needed as well, things like checking that
InvalidStateError is thrown if the connection is closed, and the many
InvalidModificationError cases in
https://w3c.github.io/webrtc-pc/#set-pc-configuration

I would suggest, for this CL, to do everything except the IDL changes, so that
when you do the web-exposed changes, it's a minimal CL that can be reverted if
stuff breaks.

Nit: extra space before (.

[Taylor_Brandstetter, 2016/11/17 21:32:52]

It doesn't do everything yet. Things it doesn't yet do:

- Candidate pool size (implemented in WebRTC library, just not added to
blink::RTCConfiguration).
- Changing the ICE ufrag/password in the next call to createOffer, as described
by JSEP.
- Throwing the right DOMExceptions. This is a problem with all PeerConnection
APIs; we don't currently have a mechanism to get an error reason from the native
library.

I thought these things could be considered bugs and fixed over time. Do any of
these absolutely need to be fixed before shipping it? I guess that's what the
"Intent to Ship" would reveal?
I do have an old "Intent to Deprecate and Remove":
https://groups.google.com/a/chromium.org/forum/#!searchin/blink-dev/setconfig...
Is it ok if these tests fail at first? (see above)
Reverted IDL changes.

[foolip, 2016/11/18 09:16:05]

It's fine to ship with some known issues, if fixing them is itself
risky/non-trivial or an orthogonal concern of sorts.

It sounds like supporting iceCandidatePoolSize would be easy, but that affects
the RTCPeerConnection constructor also, so if it's not easy, or if it's not
obvious that supporting it is even a good idea, then leaving that is fine I
think.

Overall, the most important question is whether the implementation is
interoperable with other implementations, or, if it's the first, if it's in
close conformance with the spec and tests, so that the next implementation will
not have to reverse engineer.
Oh, I'd forgotten about that. If it's still true that it always throws, then I'd
add an entry to chromestatus.com and update that thread saying that you're now
going to remove updateIce without a deprecation period.

For setConfiguration, because it wasn't super clear, perhaps it's best to send a
new Intent to Ship. With that, if you want to make all of the changes in a
single CL like this, that seems fine. Getting them all in the same release would
be good at least.
Per above, yes, see e.g.
third_party/WebKit/LayoutTests/imported/wpt/webrtc/rtcpeerconnection/rtcpeerconnection-constructor-expected.txt
for some failing tests of the RTCPeerConnection constructor that I recently
unprefixed.

(Note that this test is from web-platform-tests, for Blink-only tests it's
preferred to test the actual behavior and not the spec'd behavior.)

     // TODO(guidou): close() should never throw an exception.
     [RaisesException] void close();
     attribute EventHandler onnegotiationneeded;
@@ -120,7 +120,6 @@ enum RTCIceConnectionState {
     [RaisesException, CallWith=ScriptState] static Promise<RTCCertificate> generateCertificate(AlgorithmIdentifier keygenAlgorithm);
 
     // Non-standard or removed from the spec:
-    [Measure, CallWith=ExecutionContext, RaisesException] void updateIce(optional RTCConfiguration configuration, optional Dictionary mediaConstraints);
     [Measure] sequence<MediaStream> getLocalStreams();
     [Measure] sequence<MediaStream> getRemoteStreams();
     [Measure] MediaStream getStreamById(DOMString streamId);