WebRTC should generate default private address even when adapter enumeration is disabled.

webrtc/base/network.h

Index: webrtc/base/network.h
diff --git a/webrtc/base/network.h b/webrtc/base/network.h
index ab3a88dc7dc0c4dd511d5da7aee4b7c035975657..94cf366afcb41bc46e3b487274f07120de321960 100644
--- a/webrtc/base/network.h
+++ b/webrtc/base/network.h
@@ -28,6 +28,9 @@ struct ifaddrs;
 
 namespace rtc {
 
+extern const char kPublicIPv4Host[];
+extern const char kPublicIPv6Host[];
+
 class Network;
 class NetworkMonitorInterface;
 class Thread;
@@ -51,9 +54,17 @@ const int kDefaultNetworkIgnoreMask = ADAPTER_TYPE_LOOPBACK;
 std::string MakeNetworkKey(const std::string& name, const IPAddress& prefix,
                            int prefix_length);
 
+class DefaultAddressProvider {

[pthatcher1, 2015/11/10 18:52:34]

The DefaultAddressProvider has a GetDefaultLocalAddress.  Does that mean the
class name should be DefaultLocalAddressProviders, or that mean the method name
should be GetDefaultAddress, or does that mean we will have more methods in the
future to get other default addresses other than than the local one?

[guoweis_webrtc, 2015/11/10 20:37:29]

Done.

+ public:
+  virtual ~DefaultAddressProvider() = default;
+  // Default local address is the interface used to route to public internet on
+  // a multi-homed endpoint.

[pthatcher1, 2015/11/10 18:52:34]

This could be more clear.  Perhaps "The default local address is the local
address used in multi-homed endpoint when the any address (0.0.0.0 or ::) is
used as the local address".

[guoweis_webrtc, 2015/11/10 20:37:29]

Done.

[pthatcher1, 2015/11/10 21:29:01]

It doesn't appear changed.

+  virtual bool GetDefaultLocalAddress(int family, IPAddress* ipaddr) const = 0;

[pthatcher1, 2015/11/10 18:52:34]

Would this be more simple as GetDefaultLocalAddresses(IPAddress* ipv4,
IPAddress* ipv6) ?

[guoweis_webrtc, 2015/11/10 20:37:29]

I like returning a bool which makes the caller to check the result more
explicit.

+};
+
 // Generic network manager interface. It provides list of local
 // networks.
-class NetworkManager {
+class NetworkManager : public DefaultAddressProvider {
  public:
   typedef std::vector<Network*> NetworkList;
 
@@ -67,7 +78,7 @@ class NetworkManager {
   };
 
   NetworkManager();
-  virtual ~NetworkManager();
+  ~NetworkManager() override;
 
   // Called when network list is updated.
   sigslot::signal0<> SignalNetworksChanged;
@@ -99,6 +110,8 @@ class NetworkManager {
   // TODO(guoweis): remove this body when chromium implements this.
   virtual void GetAnyAddressNetworks(NetworkList* networks) {}
 
+  bool GetDefaultLocalAddress(int family, IPAddress* ipaddr) const override;
+
   // Dumps a list of networks available to LS_INFO.
   virtual void DumpNetworks(bool include_ignored) {}
 
@@ -128,6 +141,8 @@ class NetworkManagerBase : public NetworkManager {
 
   EnumerationPermission enumeration_permission() const override;
 
+  bool GetDefaultLocalAddress(int family, IPAddress* ipaddr) const override;
+
  protected:
   typedef std::map<std::string, Network*> NetworkMap;
   // Updates |networks_| with the networks listed in |list|. If
@@ -146,6 +161,8 @@ class NetworkManagerBase : public NetworkManager {
     enumeration_permission_ = state;
   }
 
+  void set_default_local_address(const IPAddress& ip);

[pthatcher1, 2015/11/10 18:52:34]

This is kind of awkward that we have a "set_foo" which actually sets two
different "foo"s.  Should this be set_default_local_addresses(const IPAddress&
ipv4, const IPAddress& ipv6)?

[guoweis_webrtc, 2015/11/10 20:37:29]

Done.

+
  private:
   friend class NetworkTest;
 
@@ -159,6 +176,9 @@ class NetworkManagerBase : public NetworkManager {
 
   rtc::scoped_ptr<rtc::Network> ipv4_any_address_network_;
   rtc::scoped_ptr<rtc::Network> ipv6_any_address_network_;
+
+  IPAddress default_ipv4_address_;
+  IPAddress default_ipv6_address_;

[pthatcher1, 2015/11/10 18:52:34]

To be consistent, this should be default_local_ipX_address_.

[guoweis_webrtc, 2015/11/10 20:37:29]

Done.

 };
 
 // Basic implementation of the NetworkManager interface that gets list
@@ -220,6 +240,8 @@ class BasicNetworkManager : public NetworkManagerBase,
   // based on the network's property instead of any individual IP.
   bool IsIgnoredNetwork(const Network& network) const;
 
+  IPAddress QueryDefaultAddress(int family) const;

[pthatcher1, 2015/11/10 18:52:34]

This could use a comment to make clear the difference between "Get" and "Query".
 And it should probably be "QueryDefaultLocalAddress". 

[guoweis_webrtc, 2015/11/10 20:37:29]

Done.

+
  private:
   friend class NetworkTest;
 
@@ -247,13 +269,23 @@ class BasicNetworkManager : public NetworkManagerBase,
 // Represents a Unix-type network interface, with a name and single address.
 class Network {
  public:
-  Network(const std::string& name, const std::string& description,
-          const IPAddress& prefix, int prefix_length);
-
-  Network(const std::string& name, const std::string& description,
-          const IPAddress& prefix, int prefix_length, AdapterType type);
+  Network(const std::string& name,
+          const std::string& description,
+          const IPAddress& prefix,
+          int prefix_length);
+
+  Network(const std::string& name,
+          const std::string& description,
+          const IPAddress& prefix,
+          int prefix_length,
+          AdapterType type);
   ~Network();
 
+  const DefaultAddressProvider* default_address_provider() { return provider_; }

[pthatcher1, 2015/11/10 18:52:34]

Between Network and DefaultAddressProvider, you embedded the provider in the
network.  But between NetworkManager and DefaultAddressProvider, you had manager
subclass the provider.  Why did you do it one way one place and another way the
other place?  Why not just embed in both places?

[guoweis_webrtc, 2015/11/10 20:37:29]

This is to reflect the fact that a single network shouldn't know the global
property like default local address.

+  void set_default_address_provider(const DefaultAddressProvider* provider) {
+    provider_ = provider;
+  }
+
   // Returns the name of the interface this network is associated wtih.
   const std::string& name() const { return name_; }
 
@@ -323,6 +355,7 @@ class Network {
   std::string ToString() const;
 
  private:
+  const DefaultAddressProvider* provider_ = nullptr;

[pthatcher1, 2015/11/10 18:52:34]

To be consistent, this should be default_address_provider_.

[guoweis_webrtc, 2015/11/10 20:37:29]

Done.

   std::string name_;
   std::string description_;
   IPAddress prefix_;