Bug 4865: Enable connectivity when the remote peer is on public internet.

webrtc/p2p/client/basicportallocator.cc

Index: webrtc/p2p/client/basicportallocator.cc
diff --git a/webrtc/p2p/client/basicportallocator.cc b/webrtc/p2p/client/basicportallocator.cc
index 3f5aa0a1ff396cbb512d65166d46c54c598b70b9..cc225f4c5a4031ce255db2f4bc17ca5e17204ccf 100644
--- a/webrtc/p2p/client/basicportallocator.cc
+++ b/webrtc/p2p/client/basicportallocator.cc
@@ -21,6 +21,7 @@
 #include "webrtc/p2p/base/tcpport.h"
 #include "webrtc/p2p/base/turnport.h"
 #include "webrtc/p2p/base/udpport.h"
+#include "webrtc/base/checks.h"
 #include "webrtc/base/common.h"
 #include "webrtc/base/helpers.h"
 #include "webrtc/base/logging.h"
@@ -440,26 +441,38 @@ void BasicPortAllocatorSession::OnCandidateReady(
   if (data->complete())
     return;
 
-  // Send candidates whose protocol is enabled.
-  std::vector<Candidate> candidates;
   ProtocolType pvalue;
   bool candidate_allowed_to_send = CheckCandidateFilter(c);

[juberti1, 2015/08/06 23:43:21]

I think the terminology here is confusing. |candidate_allowed_to_send| really
means "the candidate can be surfaced to the application". Maybe call this
|candidate_type_enabled| instead.

[guoweis_webrtc, 2015/08/07 18:03:43]

Done.

-  if (StringToProto(c.protocol().c_str(), &pvalue) &&
-      data->sequence()->ProtocolEnabled(pvalue) &&
-      candidate_allowed_to_send) {
+  bool port_bound_to_any_address =

[juberti1, 2015/08/06 23:43:21]

I don't think this is right. The port can be bound to an any address, but may
expose candidates that are not the any address (e.g. STUN/TURN). So this should
really be |candidate_is_any_address|.

[guoweis_webrtc, 2015/08/07 18:03:43]

Done.

+      c.address().IsAnyIP() && port->SharedSocket();
+  bool candidate_protocol_enabled =
+      StringToProto(c.protocol().c_str(), &pvalue) &&
+      data->sequence()->ProtocolEnabled(pvalue);
+
+  // These can't be true at the same time. The any address candidate shouldn't
+  // be allowed to send.
+  DCHECK(!candidate_allowed_to_send || !port_bound_to_any_address);
+
+  if (candidate_allowed_to_send && candidate_protocol_enabled) {
+    std::vector<Candidate> candidates;
     candidates.push_back(c);
+    SignalCandidatesReady(this, candidates);
   }
 
-  if (!candidates.empty()) {
-    SignalCandidatesReady(this, candidates);
+  // Port has been made ready. Nothing to do here.
+  if (data->ready()) {
+    return;
   }
 
-  // Moving to READY state as we have atleast one candidate from the port.
-  // Since this port has atleast one candidate we should forward this port
-  // to listners, to allow connections from this port.
-  // Also we should make sure that candidate gathered from this port is allowed
-  // to send outside.
-  if (!data->ready() && candidate_allowed_to_send) {
+  // Moving the port to READY state as we have at least one candidate from the
+  // port or the port is bound the any address. When this port has at least one
+  // candidate we should forward this port to listeners, to allow connections
+  // from this port. Also we should make sure that candidate gathered from this
+  // port is allowed to send outside. When the port address is bound to the any
+  // address, the any address candidate won't be allowed to send but we still
+  // need to move the port to READY such that connectivity is possible if the
+  // remote peer has a routable IP address on public Internet.
+  if (candidate_allowed_to_send || port_bound_to_any_address) {

[juberti1, 2015/08/06 23:43:21]

Rewording:

Move the port to the READY state, either because we have a usable candidate from
the port, or simply because the port is bound to the any address and therefore
has no host candidate. This will trigger the port to start creating candidate
pairs (connections) and issue connectivity checks.

[guoweis_webrtc, 2015/08/07 18:03:43]

Done, thanks.

     data->set_ready();
     SignalPortReady(this, port);
   }
@@ -508,9 +521,10 @@ void BasicPortAllocatorSession::OnProtocolEnabled(AllocationSequence* seq,
       if (!CheckCandidateFilter(potentials[i]))
         continue;
       ProtocolType pvalue;
-      if (!StringToProto(potentials[i].protocol().c_str(), &pvalue))
-        continue;
-      if (pvalue == proto) {
+      bool candidate_protocol_enabled =
+          StringToProto(potentials[i].protocol().c_str(), &pvalue) &&
+          pvalue == proto;
+      if (candidate_protocol_enabled) {
         candidates.push_back(potentials[i]);
       }
     }