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

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

 /*
  * Copyright (C) 2012 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  *
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
@@ skipping 66 matching lines @@
     [CallWith=ScriptState] Promise<void> setRemoteDescription(RTCSessionDescriptionInit description);
     readonly attribute RTCSessionDescription? remoteDescription;
     // readonly attribute RTCSessionDescription? currentRemoteDescription;
     // readonly attribute RTCSessionDescription? pendingRemoteDescription;
     [CallWith=ScriptState, MeasureAs=RTCPeerConnectionAddIceCandidatePromise] Promise<void> addIceCandidate ((RTCIceCandidateInit or RTCIceCandidate) candidate);
     readonly attribute RTCSignalingState signalingState;
     readonly attribute RTCIceGatheringState iceGatheringState;
     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;
     attribute EventHandler onicecandidate;
     attribute EventHandler onsignalingstatechange;
     attribute EventHandler oniceconnectionstatechange;
     // attribute EventHandler onicegatheringstatechange;
 
     // https://w3c.github.io/webrtc-pc/#legacy-interface-extensions
     // These methods return Promise<void> because having Promise-based versions requires that all overloads return Promises.
@@ skipping 15 matching lines @@
     // TODO(guidou): The label argument should have [TreatNullAs=EmptyString]
     // and be non-nullable.
     [CallWith=ExecutionContext, RaisesException] RTCDataChannel createDataChannel(DOMString? label, optional Dictionary options);
     attribute EventHandler ondatachannel;
 
     // Certificate management
     // https://w3c.github.io/webrtc-pc/#sec.cert-mgmt
     [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);
     [Measure, CallWith=ExecutionContext, RaisesException] void addStream(MediaStream? stream, optional Dictionary mediaConstraints);
     [Measure, RaisesException] void removeStream(MediaStream? stream);
     [Measure, RaisesException] RTCDTMFSender createDTMFSender(MediaStreamTrack track);
     attribute EventHandler onaddstream;
     attribute EventHandler onremovestream;
 };