Provide Shill IP Address to myIpAddress()

net/dns/host_resolver.h

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef NET_DNS_HOST_RESOLVER_H_
 #define NET_DNS_HOST_RESOLVER_H_
 
 #include <string>
 
 #include "base/memory/scoped_ptr.h"
@@ skipping 159 matching lines @@
 
   // Returns the HostResolverCache |this| uses, or NULL if there isn't one.
   // Used primarily to clear the cache and for getting debug information.
   virtual HostCache* GetHostCache();
 
   // Returns the current DNS configuration |this| is using, as a Value, or NULL
   // if it's configured to always use the system host resolver.  Caller takes
   // 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).
   static scoped_ptr<HostResolver> CreateSystemResolver(
       const Options& options,
       NetLog* net_log);
 
   // As above, but uses default parameters.
   static scoped_ptr<HostResolver> CreateDefaultResolver(NetLog* net_log);
 
  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);
 };
 
 }  // namespace net
 
 #endif  // NET_DNS_HOST_RESOLVER_H_