Provide Shill IP Address to myIpAddress()

net/dns/host_resolver.h

Index: net/dns/host_resolver.h
diff --git a/net/dns/host_resolver.h b/net/dns/host_resolver.h
index 2964fe600df6284e852cd7cdf29db72d9bd2c9e7..13883ec3adc4a8a0f60d9846cf791d350c0acd66 100644
--- a/net/dns/host_resolver.h
+++ b/net/dns/host_resolver.h
@@ -177,6 +177,16 @@ class NET_EXPORT HostResolver {
   // ownership of the returned Value.
   virtual base::Value* GetDnsConfigAsValue() const;
 
+  // Sets the IP addresss to use instead of the one provided by doing a lookup
+  // on gethostname() for operating systems where gethostname() returns
+  // localhost (e.g. Chrome OS).
+  void SetMyIpAddresses(const std::string& ipv4_address,
+                        const std::string& ipv6_address);
+
+  // Fills in |addresses| from my_ip*_addresses and returns true if set.
+  bool ResolveFromMyIpAddress(bool include_ipv6,

[eroman, 2014/05/02 01:48:18]

I am still unhappy with this design. Adding string overrides that are set
externally, and then tested by calling this function is subtle. The name of this
function sounds like it will get you the current IPs, however it is really just
getting the "override" ones.

My view for the "right way" of layering this is to add a new virtual method to
this interface such as:

  virtual int ResolveMyIpAddress(AddressFamily address_family, RequestPriority
priority, AddressList* addresses, const CompletionCallback callback,
RequestHandle* out_req, const BoundNetLog& net_log) = 0;

Which essentially mirrors how Resolve() works, but instead of a RequestInfo
parameter it takes a simplified AddressFamily.

The default implementation of ResolveMyIpAddress() can simply forward requests
to Resolve(), by constructing a RequestInfo() using gethostname().

I would accept the patch if that default implementation lived in HostResolver
itself along with a method to provide a static override. However I would prefer
if instead on the chrome side of things, for ChromeOS when instantiating the
"global host resovler"
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/io_...,
it wrapped this resolver with some sort of ChromeOSHostResolver which overrides
ResolveMyIpAddress() and provides that synchronously using the other hooks you
added.

Thanks for working on this, I know it is important! Apologies for not being
faster on these reviews. Please ping me over IM in the future if I don't respond
quickly :)

[stevenjb, 2014/05/02 16:56:13]

I could certainly make the name more clear and would be happy to do that.

WRT using a virtual method, while I understand the perspective, I avoided that
specifically because I was concerned about adding complexity to code that I
still only peripherally understand.

I think I need to put this on ice for a while until I have more time to wrap my
head around it. If you know anyone willing to pick it up, I will be more than
happy to review the chromeos side of things :)

+                              AddressList* addresses) const;
+
   // Creates a HostResolver implementation that queries the underlying system.
   // (Except if a unit-test has changed the global HostResolverProc using
   // ScopedHostResolverProc to intercept requests to the system).
@@ -190,7 +200,10 @@ class NET_EXPORT HostResolver {
  protected:
   HostResolver();
 
- private:
+private:
+  std::string my_ipv4_address_;

[eroman, 2014/05/02 01:48:18]

Please store these instead as IpAddressNumber

+  std::string my_ipv6_address_;
+
   DISALLOW_COPY_AND_ASSIGN(HostResolver);
 };