DNS: Expose stale results through HostResolverImpl.

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 <stddef.h>
 #include <stdint.h>
 
 #include <memory>
 #include <string>
 
 #include "base/macros.h"
 #include "net/base/address_family.h"
 #include "net/base/completion_callback.h"
 #include "net/base/host_port_pair.h"
 #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;
 }
 
 namespace net {
 
 class AddressList;
 class BoundNetLog;
-class HostCache;
 class HostResolverProc;
 class NetLog;
 
 // This class represents the task of resolving hostnames (or IP address
 // literal) to an AddressList object.
 //
 // HostResolver can handle multiple requests at a time, so when cancelling a
 // request the RequestHandle that was returned by Resolve() needs to be
 // given.  A simpler alternative for consumers that only have 1 outstanding
 // request at a time is to create a SingleRequestHostResolver wrapper around
@@ skipping 115 matching lines @@
                       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;
 
+  // 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
   // has already run or the request was canceled.
   virtual void CancelRequest(RequestHandle req) = 0;
 
   // Enable or disable the built-in asynchronous DnsClient.
   virtual void SetDnsClientEnabled(bool enabled);
 
   // Returns the HostResolverCache |this| uses, or NULL if there isn't one.
@@ skipping 17 matching lines @@
  protected:
   HostResolver();
 
  private:
   DISALLOW_COPY_AND_ASSIGN(HostResolver);
 };
 
 }  // namespace net
 
 #endif  // NET_DNS_HOST_RESOLVER_H_