DNS: Expose stale results through HostResolverImpl.

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 840aeb80e35de806ec97caa2ef86e07825b78e3b..3581889b13daad628630ccda019fe16748dcaa2f 100644
--- a/net/dns/host_resolver.h
+++ b/net/dns/host_resolver.h
@@ -18,6 +18,7 @@
 #include "net/base/net_export.h"
 #include "net/base/prioritized_dispatcher.h"
 #include "net/base/request_priority.h"
+#include "net/dns/host_cache.h"
 
 namespace base {
 class Value;
@@ -27,7 +28,6 @@ namespace net {
 
 class AddressList;
 class BoundNetLog;
-class HostCache;
 class HostResolverProc;
 class NetLog;
 
@@ -155,14 +155,6 @@ class NET_EXPORT HostResolver {
                       RequestHandle* out_req,
                       const BoundNetLog& net_log) = 0;
 
-  // Resolves the given hostname (or IP address literal) out of cache or HOSTS
-  // file (if enabled) only. This is guaranteed to complete synchronously.
-  // This acts like |Resolve()| if the hostname is IP literal, or cached value
-  // or HOSTS entry exists. Otherwise, ERR_DNS_CACHE_MISS is returned.
-  virtual int ResolveFromCache(const RequestInfo& info,
-                               AddressList* addresses,
-                               const BoundNetLog& net_log) = 0;
-
   // Changes the priority of the specified request. |req| is the handle returned
   // by Resolve(). ChangeRequestPriority must NOT be called after the request's
   // completion callback has already run or the request was canceled.
@@ -175,6 +167,52 @@ class NET_EXPORT HostResolver {
   // has already run or the request was canceled.
   virtual void CancelRequest(RequestHandle req) = 0;
 
+  // Resolves the given hostname (or IP address literal) out of cache or HOSTS
+  // file (if enabled) only. This is guaranteed to complete synchronously.
+  // This acts like |Resolve()| if the hostname is IP literal, or cached value
+  // or HOSTS entry exists. Otherwise, ERR_DNS_CACHE_MISS is returned.
+  virtual int ResolveFromCache(const RequestInfo& info,
+                               AddressList* addresses,
+                               const BoundNetLog& net_log) = 0;
+
+  // Like |ResolveFromCache()|, but can return a stale result if the
+  // implementation supports it. Fills in |*stale_info| if a response is
+  // returned to indicate how stale (or not) it is.

[Randy Smith (Not in Mondays), 2016/05/07 01:36:31]

Based on host_resolver_impl.cc, it's valid to call this function with a null
stale_info.  If that's the case, I think it should be documented here (I take
the above description as implying that it isn't, and I don't think it's a good
idea to allow for different implementations that do different things in this
space; it'll cause a null pointer dereference at some point down the line.)

[Julia Tuttle, 2016/05/11 20:19:48]

Yeah, it shouldn't be. I've added a DCHECK.

+  virtual int ResolveStaleFromCache(const RequestInfo& info,
+                                    AddressList* addresses,
+                                    HostCache::EntryStaleness* stale_info,
+                                    const BoundNetLog& net_log);
+
+  // Uses |ResolveStaleFromCache| and |Resolve| to get fresh data (either from

[Randy Smith (Not in Mondays), 2016/05/07 01:36:31]

Just putting in a reminder note (to us both) that we may want to modify this
interface based on the results of the conversation on the other CL around
inheritance versus adding methods for giving access to stale info.

(One possible way to deal with this section of that larger issue is just nuking
this interface; as I understand it, it's just syntactic sugar wrapping around
Resolve and ResolveStaleFromCache, and I'm not sure it would be that
unreasonable to force callers that are interested in the issue to implement it
themselves.  And it's a pretty complex interface and behavior for just being a
helper routine.)

(Going further down the path of deciding I don't like this interface: If we're
going to implement a helper function for some use, I'd really like to see a use
case or two to make sure we're getting the best semantics for the helper
function.  Without that, keeping the interface minimal seems higher priority.)

[Julia Tuttle, 2016/05/11 20:19:47]

If you want, I can move it into the StaleHostResolver, and if and when we do
this kind of thing in multiple places, we can factor it back out in a sensible
way?

[Randy Smith (Not in Mondays), 2016/05/17 20:39:35]

I think I'd really prefer that.  Thanks.

+  // the cache or from the network) but also return stale data if any is
+  // available.
+  //
+  // If a fresh result is available in cache, populates |addresses| and returns
+  // it synchronously.
+  //
+  // Otherwise, starts a network request, returns ERR_IO_PENDING, and:
+  //
+  // If a stale result is available in cache, returns it through
+  // |*stale_error|, |*stale_addresses|, and |*stale_info|.
+  //
+  // If no result is available in cache, sets |*stale_error| to
+  // ERR_DNS_CACHE_MISS.
+  //
+  // In either asynchronous case:
+  //
+  // If |out_req| is non-NULL, then |*out_req| will be filled with a handle to
+  // the async request. This handle is not valid after the request has
+  // completed.
+  int ResolveStale(const RequestInfo& info,
+                   RequestPriority priority,
+                   AddressList* addresses,
+                   const CompletionCallback& callback,
+                   RequestHandle* out_req,
+                   int* stale_error,
+                   AddressList* stale_addresses,
+                   HostCache::EntryStaleness* stale_info,
+                   const BoundNetLog& net_log);
+
   // Enable or disable the built-in asynchronous DnsClient.
   virtual void SetDnsClientEnabled(bool enabled);