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 4e19e63af3f8311c507d195e3ecb933829995a34..d6c2ae1847b060aaa7f4b27050c8c3c17d8b8c32 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;
 
@@ -163,6 +163,38 @@ class NET_EXPORT HostResolver {
                                AddressList* addresses,
                                const BoundNetLog& net_log) = 0;
 
+  // Like |ResolveFromCache()|, but can return a stale result. Fills in
+  // |*stale_info| if a response is returned to indicate how stale (or not) it
+  // is. (Defaults to calling |ResolveFromCache()| if not overridden.)

[Randy Smith (Not in Mondays), 2016/05/04 19:46:59]

I'd personally vote for (i.e.: suggestion) leaving out the parenthetical comment
and just saying "... can return a stale result if the implementation supports
that behavior."  The "if not overridden" feels like implementation information
to me.

[Julia Tuttle, 2016/05/05 15:42:54]

Done.

+  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
+  // 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:

[Randy Smith (Not in Mondays), 2016/05/04 19:46:59]

nit: This is implicitly relying on the the comment above Resolve() about the
handling of |out_req|.  My preference would be for each of these comments to be
standalone (or only rely on top level comments) as opposed to having
inter-comment dependencies; could you duplicate the description of the handling
of out_req from Resolve()?

(By the way, thanks for the doc update--this is complex, but clear.)

[Julia Tuttle, 2016/05/05 15:42:54]

Done.

+  //
+  // 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.
+  int ResolveStale(const RequestInfo& info,

[Randy Smith (Not in Mondays), 2016/05/04 19:46:59]

In not marking this virtual, you're specifically indicating that implementations
can't override it.  Is that a conscious choice?  What are the design
considerations behind it?

[Julia Tuttle, 2016/05/05 15:42:54]

It is a conscious choice -- I see ResolveStale as a specific combination of
calls to ResolveStaleFromCache and Resolve. I don't see a purpose in letting a
subclass override it.

If you do, I'll happily make it virtual, or we can just leave it this way until
someone finds a reason to change it.

+                   RequestPriority priority,
+                   AddressList* addresses,
+                   const CompletionCallback& callback,
+                   RequestHandle* out_req,
+                   int* stale_error,
+                   AddressList* stale_addresses,
+                   HostCache::EntryStaleness* stale_info,
+                   const BoundNetLog& net_log);
+
   // Cancels the specified request. |req| is the handle returned by Resolve().
   // After a request is canceled, its completion callback will not be called.
   // CancelRequest must NOT be called after the request's completion callback