Remove candidates when doing continual gathering

webrtc/api/jsep.h

Index: webrtc/api/jsep.h
diff --git a/webrtc/api/jsep.h b/webrtc/api/jsep.h
index 0673ce128e053098bbc87c7ee264ce88d6a01ccf..bb8b2e3cf0d96a9e48c65a18a8a38747a98c9556 100644
--- a/webrtc/api/jsep.h
+++ b/webrtc/api/jsep.h
@@ -18,6 +18,7 @@
 
 #include "webrtc/base/basictypes.h"
 #include "webrtc/base/refcount.h"
+#include "webrtc/base/scoped_ref_ptr.h"
 
 namespace cricket {
 class SessionDescription;
@@ -37,7 +38,7 @@ struct SdpParseError {
 // Class representation of an ICE candidate.
 // An instance of this interface is supposed to be owned by one class at
 // a time and is therefore not expected to be thread safe.
-class IceCandidateInterface {
+class IceCandidateInterface : public rtc::RefCountInterface {
  public:
   virtual ~IceCandidateInterface() {}
   /// If present, this contains the identierfier of the "media stream
@@ -52,6 +53,8 @@ class IceCandidateInterface {
   virtual bool ToString(std::string* out) const = 0;
 };
 
+typedef rtc::scoped_refptr<IceCandidateInterface> IceCandidateInterfaceRefPtr;
+
 // Creates a IceCandidateInterface based on SDP string.
 // Returns NULL if the sdp string can't be parsed.
 // |error| can be NULL if doesn't care about the failure reason.
@@ -95,6 +98,11 @@ class SessionDescriptionInterface {
   // Returns false if the session description does not have a media section that
   // corresponds to the |candidate| label.
   virtual bool AddCandidate(const IceCandidateInterface* candidate) = 0;
+  // Removes the candidates that have a matching address and protocol.
+  // Returns the number of candidates removed.
+  virtual int RemoveCandidates(
+      const std::vector<IceCandidateInterfaceRefPtr>& candidates) = 0;

[pthatcher1, 2016/03/01 23:51:53]

Why do we have to use a scoped_refptr here?  Why can't we just use a const
std::vector<IceCandidateInterface>&?  It seems like all the implementations of
this just read the values, so I don't see why we need to do refcounting.

And even if we worried about ownership, we could just copy the candidates and
pass ownership of the copy, since they are cheap to copy.

[honghaiz3, 2016/03/02 19:06:40]

We cannot do vector<IceCandidateInterface>. If so, we have to provide vector of
IceCandidateInterface, not vector of its subclasses. 
By the way, peerconnection_interface does use vector of Ref-counted pointers a
few times. 

[pthatcher1, 2016/03/02 20:37:58]

If this is just because we want an IceCandidateInteface so that we can have a
MockIceCandidate, then I think we should look into removing
IceCandidateInterface and just make it IceCandidate and handle mocking some
other way.  Introducing ref counting into a simple struct that's passed around
seems like too much complexity for too little gain (the MockIceCandidate).

[honghaiz3, 2016/03/03 01:17:40]

It is going to be a lot more complicated to try to remove IceCandidateInterface
because it is used by Chrome and by chromoting. All the RefCount object adds is
just a counter. We could create a vector of IceCandidate objects and create
another vector of pointers pointing to the objects in the object vector,
although that would appear not as safe as the ref-counted pointers.

+
   // Returns the number of m- lines in the session description.
   virtual size_t number_of_mediasections() const = 0;
   // Returns a collection of all candidates that belong to a certain m-line