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

webrtc/base/network.h

 /*
  *  Copyright 2004 The WebRTC Project Authors. All rights reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 10 matching lines @@
 #include "webrtc/base/messagehandler.h"
 #include "webrtc/base/scoped_ptr.h"
 #include "webrtc/base/sigslot.h"
 
 #if defined(WEBRTC_POSIX)
 struct ifaddrs;
 #endif  // defined(WEBRTC_POSIX)
 
 namespace rtc {
 
+extern const char kPublicIPv4Host[];
+extern const char kPublicIPv6Host[];
+
 class Network;
 class NetworkMonitorInterface;
 class Thread;
 
 enum AdapterType {
   // This enum resembles the one in Chromium net::ConnectionType.
   ADAPTER_TYPE_UNKNOWN = 0,
   ADAPTER_TYPE_ETHERNET = 1 << 0,
   ADAPTER_TYPE_WIFI = 1 << 1,
   ADAPTER_TYPE_CELLULAR = 1 << 2,
   ADAPTER_TYPE_VPN = 1 << 3,
   ADAPTER_TYPE_LOOPBACK = 1 << 4
 };
 
 // By default, ignore loopback interfaces on the host.
 const int kDefaultNetworkIgnoreMask = ADAPTER_TYPE_LOOPBACK;
 
 // Makes a string key for this network. Used in the network manager's maps.
 // Network objects are keyed on interface name, network prefix and the
 // length of that prefix.
 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;
 
   // This enum indicates whether adapter enumeration is allowed.
   enum EnumerationPermission {
     ENUMERATION_ALLOWED,  // Adapter enumeration is allowed. Getting 0 network
                           // from GetNetworks means that there is no network
                           // available.
     ENUMERATION_BLOCKED,  // Adapter enumeration is disabled.
                           // GetAnyAddressNetworks() should be used instead.
   };
 
   NetworkManager();
-  virtual ~NetworkManager();
+  ~NetworkManager() override;
 
   // Called when network list is updated.
   sigslot::signal0<> SignalNetworksChanged;
 
   // Indicates a failure when getting list of network interfaces.
   sigslot::signal0<> SignalError;
 
   // Start/Stop monitoring of network interfaces
   // list. SignalNetworksChanged or SignalError is emitted immediately
   // after StartUpdating() is called. After that SignalNetworksChanged
@@ skipping 11 matching lines @@
   // return the current permission state of GetNetworks()
   virtual EnumerationPermission enumeration_permission() const;
 
   // "AnyAddressNetwork" is a network which only contains single "any address"
   // IP address.  (i.e. INADDR_ANY for IPv4 or in6addr_any for IPv6). This is
   // useful as binding to such interfaces allow default routing behavior like
   // http traffic.
   // 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) {}
 
   struct Stats {
     int ipv4_network_count;
     int ipv6_network_count;
     Stats() {
       ipv4_network_count = 0;
       ipv6_network_count = 0;
     }
   };
 };
 
 // Base class for NetworkManager implementations.
 class NetworkManagerBase : public NetworkManager {
  public:
   NetworkManagerBase();
   ~NetworkManagerBase() override;
 
   void GetNetworks(std::vector<Network*>* networks) const override;
   void GetAnyAddressNetworks(NetworkList* networks) override;
   bool ipv6_enabled() const { return ipv6_enabled_; }
   void set_ipv6_enabled(bool enabled) { ipv6_enabled_ = enabled; }
 
   void set_max_ipv6_networks(int networks) { max_ipv6_networks_ = networks; }
   int max_ipv6_networks() { return max_ipv6_networks_; }
 
   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
   // |network_map_| already has a Network object for a network listed
   // in the |list| then it is reused. Accept ownership of the Network
   // objects in the |list|. |changed| will be set to true if there is
   // any change in the network list.
   void MergeNetworkList(const NetworkList& list, bool* changed);
 
   // |stats| will be populated even if |*changed| is false.
   void MergeNetworkList(const NetworkList& list,
                         bool* changed,
                         NetworkManager::Stats* stats);
 
   void set_enumeration_permission(EnumerationPermission state) {
     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;
 
   EnumerationPermission enumeration_permission_;
 
   NetworkList networks_;
   int max_ipv6_networks_;
 
   NetworkMap networks_map_;
   bool ipv6_enabled_;
 
   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
 // of networks using OS APIs.
 class BasicNetworkManager : public NetworkManagerBase,
                             public MessageHandler,
                             public sigslot::has_slots<> {
  public:
   BasicNetworkManager();
   ~BasicNetworkManager() override;
@@ skipping 41 matching lines @@
                       NetworkList* networks) const;
 #endif  // defined(WEBRTC_POSIX)
 
   // Creates a network object for each network available on the machine.
   bool CreateNetworks(bool include_ignored, NetworkList* networks) const;
 
   // Determines if a network should be ignored. This should only be determined
   // 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;
 
   // Creates a network monitor and listens for network updates.
   void StartNetworkMonitor();
   // Stops and removes the network monitor.
   void StopNetworkMonitor();
   // Called when it receives updates from the network monitor.
   void OnNetworksChanged();
 
   // Updates the networks and reschedules the next update.
   void UpdateNetworksContinually();
   // Only updates the networks; does not reschedule the next update.
   void UpdateNetworksOnce();
 
   Thread* thread_;
   bool sent_first_update_;
   int start_count_;
   std::vector<std::string> network_ignore_list_;
   int network_ignore_mask_;
   bool ignore_non_default_routes_;
   scoped_ptr<NetworkMonitorInterface> network_monitor_;
 };
 
 // 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);
 
-  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,
+          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_; }
 
   // Returns the OS-assigned name for this network. This is useful for
   // debugging but should not be sent over the wire (for privacy reasons).
   const std::string& description() const { return description_; }
 
   // Returns the prefix for this network.
   const IPAddress& prefix() const { return prefix_; }
   // Returns the length, in bits, of this network's prefix.
@@ skipping 49 matching lines @@
   void set_ignored(bool ignored) { ignored_ = ignored; }
 
   AdapterType type() const { return type_; }
   int preference() const { return preference_; }
   void set_preference(int preference) { preference_ = preference; }
 
   // Debugging description of this 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_;
   int prefix_length_;
   std::string key_;
   std::vector<InterfaceAddress> ips_;
   int scope_id_;
   bool ignored_;
   AdapterType type_;
   int preference_;
 
   friend class NetworkManager;
 };
 
 }  // namespace rtc
 
 #endif  // WEBRTC_BASE_NETWORK_H_