Fix a case where empty candidate id is used

BUG=4161
R=juberti@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/35749004

git-svn-id: http://webrtc.googlecode.com/svn/trunk@8071 4adac7df-926f-26a2-2b94-8c16560cd09d
diff --git a/talk/app/webrtc/jsepsessiondescription_unittest.cc b/talk/app/webrtc/jsepsessiondescription_unittest.cc
index 1c24c1a..530c60b 100644
--- a/talk/app/webrtc/jsepsessiondescription_unittest.cc
+++ b/talk/app/webrtc/jsepsessiondescription_unittest.cc
@@ -100,8 +100,8 @@
   virtual void SetUp() {
     int port = 1234;
     rtc::SocketAddress address("127.0.0.1", port++);
-    cricket::Candidate candidate("rtp", cricket::ICE_CANDIDATE_COMPONENT_RTP,
-                                 "udp", address, 1, "", "", "local", 0, "1");
+    cricket::Candidate candidate(cricket::ICE_CANDIDATE_COMPONENT_RTP, "udp",
+                                 address, 1, "", "", "local", 0, "1");
     candidate_ = candidate;
     const std::string session_id =
         rtc::ToString(rtc::CreateRandomId64());
diff --git a/talk/app/webrtc/statscollector_unittest.cc b/talk/app/webrtc/statscollector_unittest.cc
index 6bac798..539bf23 100644
--- a/talk/app/webrtc/statscollector_unittest.cc
+++ b/talk/app/webrtc/statscollector_unittest.cc
@@ -1040,8 +1040,7 @@
   uint32 priority = 1000;
 
   cricket::Candidate c;
-  const std::string& local_id = rtc::CreateRandomString(8);
-  c.set_id(local_id);
+  ASSERT(c.id().length() > 0);
   c.set_type(cricket::LOCAL_PORT_TYPE);
   c.set_protocol(cricket::UDP_PROTOCOL_NAME);
   c.set_address(local_address);
@@ -1049,15 +1048,18 @@
   c.set_network_type(network_type);
   std::string report_id = AddCandidateReport(
       &stats, c, StatsReport::kStatsReportTypeIceLocalCandidate);
-  EXPECT_EQ("Cand-" + local_id, report_id);
+  EXPECT_EQ("Cand-" + c.id(), report_id);
 
-  const std::string& remote_id = rtc::CreateRandomString(8);
-  c.set_id(remote_id);
+  c = cricket::Candidate();
+  ASSERT(c.id().length() > 0);
   c.set_type(cricket::PRFLX_PORT_TYPE);
+  c.set_protocol(cricket::UDP_PROTOCOL_NAME);
   c.set_address(remote_address);
+  c.set_priority(priority);
+  c.set_network_type(network_type);
   report_id = AddCandidateReport(
       &stats, c, StatsReport::kStatsReportTypeIceRemoteCandidate);
-  EXPECT_EQ("Cand-" + remote_id, report_id);
+  EXPECT_EQ("Cand-" + c.id(), report_id);
 
   stats.GetStats(NULL, &reports);
 
diff --git a/talk/app/webrtc/webrtcsdp.cc b/talk/app/webrtc/webrtcsdp.cc
index 9bb619e..1fca0ae 100644
--- a/talk/app/webrtc/webrtcsdp.cc
+++ b/talk/app/webrtc/webrtcsdp.cc
@@ -1090,8 +1090,7 @@
     }
   }
 
-  const std::string id;
-  *candidate = Candidate(id, component_id, cricket::ProtoToString(protocol),
+  *candidate = Candidate(component_id, cricket::ProtoToString(protocol),
                          address, priority, username, password, candidate_type,
                          generation, foundation);
   candidate->set_related_address(related_address);
diff --git a/talk/app/webrtc/webrtcsdp_unittest.cc b/talk/app/webrtc/webrtcsdp_unittest.cc
index b33aab4..975b0ac 100644
--- a/talk/app/webrtc/webrtcsdp_unittest.cc
+++ b/talk/app/webrtc/webrtcsdp_unittest.cc
@@ -588,41 +588,41 @@
     // v4 host
     int port = 1234;
     rtc::SocketAddress address("192.168.1.5", port++);
-    Candidate candidate1("", ICE_CANDIDATE_COMPONENT_RTP, "udp", address,
+    Candidate candidate1(ICE_CANDIDATE_COMPONENT_RTP, "udp", address,
                          kCandidatePriority, "", "", LOCAL_PORT_TYPE,
                          kCandidateGeneration, kCandidateFoundation1);
     address.SetPort(port++);
-    Candidate candidate2("", ICE_CANDIDATE_COMPONENT_RTCP, "udp", address,
+    Candidate candidate2(ICE_CANDIDATE_COMPONENT_RTCP, "udp", address,
                          kCandidatePriority, "", "", LOCAL_PORT_TYPE,
                          kCandidateGeneration, kCandidateFoundation1);
     address.SetPort(port++);
-    Candidate candidate3("", ICE_CANDIDATE_COMPONENT_RTCP, "udp", address,
+    Candidate candidate3(ICE_CANDIDATE_COMPONENT_RTCP, "udp", address,
                          kCandidatePriority, "", "", LOCAL_PORT_TYPE,
                          kCandidateGeneration, kCandidateFoundation1);
     address.SetPort(port++);
-    Candidate candidate4("", ICE_CANDIDATE_COMPONENT_RTP, "udp", address,
+    Candidate candidate4(ICE_CANDIDATE_COMPONENT_RTP, "udp", address,
                          kCandidatePriority, "", "", LOCAL_PORT_TYPE,
                          kCandidateGeneration, kCandidateFoundation1);
 
     // v6 host
     rtc::SocketAddress v6_address("::1", port++);
-    cricket::Candidate candidate5("", cricket::ICE_CANDIDATE_COMPONENT_RTP,
-                                  "udp", v6_address, kCandidatePriority, "", "",
+    cricket::Candidate candidate5(cricket::ICE_CANDIDATE_COMPONENT_RTP, "udp",
+                                  v6_address, kCandidatePriority, "", "",
                                   cricket::LOCAL_PORT_TYPE,
                                   kCandidateGeneration, kCandidateFoundation2);
     v6_address.SetPort(port++);
-    cricket::Candidate candidate6("", cricket::ICE_CANDIDATE_COMPONENT_RTCP,
-                                  "udp", v6_address, kCandidatePriority, "", "",
+    cricket::Candidate candidate6(cricket::ICE_CANDIDATE_COMPONENT_RTCP, "udp",
+                                  v6_address, kCandidatePriority, "", "",
                                   cricket::LOCAL_PORT_TYPE,
                                   kCandidateGeneration, kCandidateFoundation2);
     v6_address.SetPort(port++);
-    cricket::Candidate candidate7("", cricket::ICE_CANDIDATE_COMPONENT_RTCP,
-                                  "udp", v6_address, kCandidatePriority, "", "",
+    cricket::Candidate candidate7(cricket::ICE_CANDIDATE_COMPONENT_RTCP, "udp",
+                                  v6_address, kCandidatePriority, "", "",
                                   cricket::LOCAL_PORT_TYPE,
                                   kCandidateGeneration, kCandidateFoundation2);
     v6_address.SetPort(port++);
-    cricket::Candidate candidate8("", cricket::ICE_CANDIDATE_COMPONENT_RTP,
-                                  "udp", v6_address, kCandidatePriority, "", "",
+    cricket::Candidate candidate8(cricket::ICE_CANDIDATE_COMPONENT_RTP, "udp",
+                                  v6_address, kCandidatePriority, "", "",
                                   cricket::LOCAL_PORT_TYPE,
                                   kCandidateGeneration, kCandidateFoundation2);
 
@@ -630,31 +630,31 @@
     int port_stun = 2345;
     rtc::SocketAddress address_stun("74.125.127.126", port_stun++);
     rtc::SocketAddress rel_address_stun("192.168.1.5", port_stun++);
-    cricket::Candidate candidate9("", cricket::ICE_CANDIDATE_COMPONENT_RTP,
-                                  "udp", address_stun, kCandidatePriority, "",
-                                  "", STUN_PORT_TYPE, kCandidateGeneration,
+    cricket::Candidate candidate9(cricket::ICE_CANDIDATE_COMPONENT_RTP, "udp",
+                                  address_stun, kCandidatePriority, "", "",
+                                  STUN_PORT_TYPE, kCandidateGeneration,
                                   kCandidateFoundation3);
     candidate9.set_related_address(rel_address_stun);
 
     address_stun.SetPort(port_stun++);
     rel_address_stun.SetPort(port_stun++);
-    cricket::Candidate candidate10("", cricket::ICE_CANDIDATE_COMPONENT_RTCP,
-                                   "udp", address_stun, kCandidatePriority, "",
-                                   "", STUN_PORT_TYPE, kCandidateGeneration,
+    cricket::Candidate candidate10(cricket::ICE_CANDIDATE_COMPONENT_RTCP, "udp",
+                                   address_stun, kCandidatePriority, "", "",
+                                   STUN_PORT_TYPE, kCandidateGeneration,
                                    kCandidateFoundation3);
     candidate10.set_related_address(rel_address_stun);
 
     // relay
     int port_relay = 3456;
     rtc::SocketAddress address_relay("74.125.224.39", port_relay++);
-    cricket::Candidate candidate11("", cricket::ICE_CANDIDATE_COMPONENT_RTCP,
-                                   "udp", address_relay, kCandidatePriority, "",
-                                   "", cricket::RELAY_PORT_TYPE,
+    cricket::Candidate candidate11(cricket::ICE_CANDIDATE_COMPONENT_RTCP, "udp",
+                                   address_relay, kCandidatePriority, "", "",
+                                   cricket::RELAY_PORT_TYPE,
                                    kCandidateGeneration, kCandidateFoundation4);
     address_relay.SetPort(port_relay++);
-    cricket::Candidate candidate12("", cricket::ICE_CANDIDATE_COMPONENT_RTP,
-                                   "udp", address_relay, kCandidatePriority, "",
-                                   "", RELAY_PORT_TYPE, kCandidateGeneration,
+    cricket::Candidate candidate12(cricket::ICE_CANDIDATE_COMPONENT_RTP, "udp",
+                                   address_relay, kCandidatePriority, "", "",
+                                   RELAY_PORT_TYPE, kCandidateGeneration,
                                    kCandidateFoundation4);
 
     // voice
@@ -1612,7 +1612,7 @@
 // TODO(mallinath) : Enable this test once WebRTCSdp capable of parsing
 // RFC 6544.
 TEST_F(WebRtcSdpTest, SerializeTcpCandidates) {
-  Candidate candidate("", ICE_CANDIDATE_COMPONENT_RTP, "tcp",
+  Candidate candidate(ICE_CANDIDATE_COMPONENT_RTP, "tcp",
                       rtc::SocketAddress("192.168.1.5", 9), kCandidatePriority,
                       "", "", LOCAL_PORT_TYPE, kCandidateGeneration,
                       kCandidateFoundation1);
@@ -1905,7 +1905,7 @@
   sdp = kSdpTcpActiveCandidate;
   EXPECT_TRUE(SdpDeserializeCandidate(sdp, &jcandidate));
   // Make a cricket::Candidate equivalent to kSdpTcpCandidate string.
-  Candidate candidate("", ICE_CANDIDATE_COMPONENT_RTP, "tcp",
+  Candidate candidate(ICE_CANDIDATE_COMPONENT_RTP, "tcp",
                       rtc::SocketAddress("192.168.1.5", 9), kCandidatePriority,
                       "", "", LOCAL_PORT_TYPE, kCandidateGeneration,
                       kCandidateFoundation1);
diff --git a/webrtc/p2p/base/candidate.h b/webrtc/p2p/base/candidate.h
index 43809fa..8a07760 100644
--- a/webrtc/p2p/base/candidate.h
+++ b/webrtc/p2p/base/candidate.h
@@ -20,6 +20,7 @@
 
 #include "webrtc/p2p/base/constants.h"
 #include "webrtc/base/basictypes.h"
+#include "webrtc/base/helpers.h"
 #include "webrtc/base/network.h"
 #include "webrtc/base/socketaddress.h"
 
@@ -32,13 +33,13 @@
   // TODO: Match the ordering and param list as per RFC 5245
   // candidate-attribute syntax. http://tools.ietf.org/html/rfc5245#section-15.1
   Candidate()
-      : component_(0),
+      : id_(rtc::CreateRandomString(8)),
+        component_(0),
         priority_(0),
         network_type_(rtc::ADAPTER_TYPE_UNKNOWN),
         generation_(0) {}
 
-  Candidate(const std::string& id,
-            int component,
+  Candidate(int component,
             const std::string& protocol,
             const rtc::SocketAddress& address,
             uint32 priority,
@@ -47,7 +48,7 @@
             const std::string& type,
             uint32 generation,
             const std::string& foundation)
-      : id_(id),
+      : id_(rtc::CreateRandomString(8)),
         component_(component),
         protocol_(protocol),
         address_(address),
@@ -154,15 +155,10 @@
     // We ignore the network name, since that is just debug information, and
     // the priority, since that should be the same if the rest is (and it's
     // a float so equality checking is always worrisome).
-    return (id_ == c.id_) &&
-           (component_ == c.component_) &&
-           (protocol_ == c.protocol_) &&
-           (address_ == c.address_) &&
-           (username_ == c.username_) &&
-           (password_ == c.password_) &&
-           (type_ == c.type_) &&
-           (generation_ == c.generation_) &&
-           (foundation_ == c.foundation_) &&
+    return (component_ == c.component_) && (protocol_ == c.protocol_) &&
+           (address_ == c.address_) && (username_ == c.username_) &&
+           (password_ == c.password_) && (type_ == c.type_) &&
+           (generation_ == c.generation_) && (foundation_ == c.foundation_) &&
            (related_address_ == c.related_address_);
   }
 
diff --git a/webrtc/p2p/base/p2ptransportchannel.cc b/webrtc/p2p/base/p2ptransportchannel.cc
index 8073f59..7b16a0c 100644
--- a/webrtc/p2p/base/p2ptransportchannel.cc
+++ b/webrtc/p2p/base/p2ptransportchannel.cc
@@ -505,14 +505,18 @@
       }
     }
 
-    std::string id = rtc::CreateRandomString(8);
     new_remote_candidate =
-        Candidate(id, component(), ProtoToString(proto), address, 0,
-                  remote_username, remote_password, type, 0U,
-                  rtc::ToString<uint32>(rtc::ComputeCrc32(id)));
-    new_remote_candidate.set_priority(
-        new_remote_candidate.GetPriority(ICE_TYPE_PREFERENCE_SRFLX,
-                                         port->Network()->preference(), 0));
+        Candidate(component(), ProtoToString(proto), address, 0,
+                  remote_username, remote_password, type, 0U, "");
+
+    // From RFC 5245, section-7.2.1.3:
+    // The foundation of the candidate is set to an arbitrary value, different
+    // from the foundation for all other remote candidates.
+    new_remote_candidate.set_foundation(
+        rtc::ToString<uint32>(rtc::ComputeCrc32(new_remote_candidate.id())));
+
+    new_remote_candidate.set_priority(new_remote_candidate.GetPriority(
+        ICE_TYPE_PREFERENCE_SRFLX, port->Network()->preference(), 0));
   }
 
   if (port->IceProtocol() == ICEPROTO_RFC5245) {