Preparation for new Promise-based RTCPeerConnection.getStats.

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 86 matching lines @@
 
     // 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.
     [CallWith=ScriptState] Promise<void> createOffer(RTCSessionDescriptionCallback successCallback, RTCPeerConnectionErrorCallback failureCallback, optional Dictionary rtcOfferOptions);
     // TODO(guidou): There should be no mediaConstraints argument.
     [CallWith=ScriptState] Promise<void> createAnswer(RTCSessionDescriptionCallback successCallback, RTCPeerConnectionErrorCallback failureCallback, optional Dictionary mediaConstraints);
     [CallWith=ScriptState] Promise<void> setLocalDescription(RTCSessionDescriptionInit description, VoidCallback successCallback, [Default=Undefined] optional RTCPeerConnectionErrorCallback failureCallback);
     // TODO(guidou): The failureCallback argument should be non-optional.
     [CallWith=ScriptState] Promise<void> setRemoteDescription(RTCSessionDescriptionInit description, VoidCallback successCallback, [Default=Undefined] optional RTCPeerConnectionErrorCallback failureCallback);
     [CallWith=ScriptState, MeasureAs=RTCPeerConnectionAddIceCandidateLegacy] Promise<void> addIceCandidate((RTCIceCandidateInit or RTCIceCandidate) candidate, VoidCallback successCallback, RTCPeerConnectionErrorCallback failureCallback);
+
     // TODO(guidou): The selector argument should the first (nullable,
     // non-optional) argument, and there should be a third failureCallback
     // argument.
-    [CallWith=ExecutionContext, LegacyInterfaceTypeChecking] void getStats(RTCStatsCallback successCallback, [Default=Undefined] optional MediaStreamTrack selector);
+    // TODO(hbos): WebIDL is not able to distinguish between
+    // |getStats(RTCStatsCallback)| and |getStats(MediaStreamTrack)| because
+    // |RTCStatsCallback| is a callback interface and |MediaStreamTrack| is an
+    // interface, see https://heycam.github.io/webidl/#idl-overloading. In order
+    // to support both signatures, |getStats(optional any)| is used. In C++ we
+    // dispatch to the correct |getStats| function based on the value type.

[hta - Chromium, 2016/07/19 09:56:48]

I read heycam to say that an interface is distinguishable from another interface
if "The two identified interfaces are not the same, it is not possible for a
single platform object to implement both interfaces, and it is not the case that
both are callback interfaces."

Does the WebIDL compiler give you an error when you try to make them separate?

[hbos_chromium, 2016/07/19 13:59:35]

I might have misinterpreted something. I'm not sure if they *should* be
distinguishable. (Could an object have a handleEvent function and still be
considered a MediaStreamTrack?)

I was able to get it to compile with both getStats, but due to the generated V8
code the wrong getStats function wound up being called, it didn't distinguish.
(In line with spec or V8 gen bug?)

I've spent a lot of time on this already. One idea was to change
RTCStatsCallback to a callback function but this isn't fully supported
(crbug.com/569301) and we would have to write a lot of V8 code manually instead.
This seemed less painful.

[hbos_chromium, 2016/07/20 09:29:48]

I've confirmed a second time (crbug.com/629068#c5) that it is unable to
distinguish. We can discuss whether this is a bug or by design there, but in the
meantime, is this an acceptable workaround?

+    //
+    // This is a workaround for the desired WebIDL which is:
+    //  [CallWith=ScriptState] Promise<void> getStats(RTCStatsCallback successCallback, optional MediaStreamTrack? selector);
+    //  [CallWith=ScriptState] Promise<RTCStatsReport> getStats(optional MediaStreamTrack? selector);
+    //
+    // The plan is to have the promise-based version replace the callback-based
+    // version and remove it, then we can use the desired WebIDL and not have to
+    // evaluate the value type ourselves in C++. crbug.com/629068, 627816.

[hta - Chromium, 2016/07/19 09:56:48]

Can you add UMA counters here while you're at it?

[hbos_chromium, 2016/07/19 13:59:35]

Good idea. I'd rather add and have that reviewed in a separate follow-up CL if
that's ok. Added a TODO comment (in RTCPeerConnection.h).

[hbos_chromium, 2016/07/20 11:28:44]

I looked into it and the change is just a couple of lines so I did it in the
same CL. It requires more owners though.

+    [CallWith=ScriptState] Promise getStats(optional any value);
+    [CallWith=ScriptState] Promise<void> getStats(RTCStatsCallback successCallback, MediaStreamTrack? selector);
 
     // https://w3c.github.io/webrtc-pc/#peer-to-peer-data-api
     // TODO(guidou): The label argument should have [TreatNullAs=EmptyString]
     // and be non-nullable.
     [RaisesException] RTCDataChannel createDataChannel([TreatUndefinedAs=NullString] DOMString? label, optional Dictionary options);
     attribute EventHandler ondatachannel;
 
     // Non-standard or removed from the spec:
     [CallWith=ExecutionContext, RaisesException] void updateIce(optional Dictionary configuration, optional Dictionary mediaConstraints);
     sequence<MediaStream> getLocalStreams();
     sequence<MediaStream> getRemoteStreams();
     MediaStream getStreamById(DOMString streamId);
     [CallWith=ExecutionContext, RaisesException] void addStream(MediaStream? stream, optional Dictionary mediaConstraints);
     [RaisesException] void removeStream(MediaStream? stream);
     [RaisesException] RTCDTMFSender createDTMFSender(MediaStreamTrack track);
     attribute EventHandler onaddstream;
     attribute EventHandler onremovestream;
 
     // Certificate management
     // http://w3c.github.io/webrtc-pc/#sec.cert-mgmt
     [RaisesException, CallWith=ScriptState] static Promise<RTCCertificate> generateCertificate(AlgorithmIdentifier keygenAlgorithm);
 };