Cronet: Use stale DNS cache entries experimentally.

net/dns/stale_host_resolver.h

+// Copyright 2016 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_STALE_HOST_RESOLVER_H_
+#define NET_DNS_STALE_HOST_RESOLVER_H_
+
+#include <set>
+
+#include "net/dns/host_resolver.h"
+
+namespace net {
+
+// A HostResolver that wraps another HostResolver that uses it to make requests
+// but "impatiently" returns stale data (if available and usable) after a delay,
+// to reduce DNS latency at the expense of accuracy.
+class NET_EXPORT StaleHostResolver : public HostResolver {
+ public:
+  struct NET_EXPORT StaleOptions {
+    // How long to wait before returning stale data, if available.
+    base::TimeDelta delay;
+
+    // If positive, how long stale data can be past the expiration time before
+    // it's considered unusable. If zero or negative, stale data can be used
+    // indefinitely.
+    base::TimeDelta max_expired_time;
+
+    // If set, stale data from previous networks is usable; if clear, it's not.

[Ryan Sleevi, 2016/04/29 01:04:09]

Suggestion: Explain a little more about the trade-offs here - what the risks are
about using stale data from other networks. It's not clear from here (but
perhaps I missed it in another CL or documentation)?

Maybe a scenario where it can improve good results, and a scenario where it can
cause bad results, so that callers have some conceptual overtones about the
trade-offs involved.

[Julia Tuttle, 2016/04/29 18:04:29]

Done.

+    bool allow_other_network;
+
+    // If positive, the maximum number of times a stale entry can be used. If
+    // zero, there is no limit. (Note that this count is kept by the HostCache,
+    // so it is shared across all StaleHostResolvers and any other callers who
+    // request potentially stale data.)

[Randy Smith (Not in Mondays), 2016/04/28 17:48:25]

nit: How hard would it be to change this?  I suspect your current use cases
don't require it, but to my mind it's a layering violation, so I'd like to at
least explore doing it right.

[Ryan Sleevi, 2016/04/29 01:04:09]

What HostCache? Who owns it? Is it a global?

(My guess, from this header, is that it's stored on inner_resolver's HostCache -
so the count is only unique for objects sharing the same inner_resolver /
sharing the same HostCache as the inner_resolver. Is that correct?)

[Julia Tuttle, 2016/04/29 18:04:30]

We could keep a std::map<HostCache::Key, int> stale_uses_ in the
StaleHostResolver, increment it on returning a stale entry, and clear it on...
setting an entry? but we don't actually know when that happens, because it
happens in the inner HostResolverImpl.

It's a layering violation, except the HostCache is created by the
HostResolverImpl, and then the StaleHostResolver takes ownership of the
HostResolverImpl, so it can't actually be shared once the StaleHostResolver is
created unless someone kept a pointer to the HostResolverImpl.

[Randy Smith (Not in Mondays), 2016/05/04 21:00:28]

So currently StaleHostResolver owns (does not share) the inner HostResolver,
and, when that inner HostResolver is actually a HostResolverImpl, the
HostResolverImpl owns (does not share) a HostCache.  So in that case, there's no
problem, because the only place that hits can come from is through this class. 
That right?

If so, I think we can solve this problem by simply documenting that
HostResolver::ResolveStale will return hit information in |*stale_info| only
based on lookups through that HostResolver.  That right?

+    unsigned max_stale_uses;
+  };
+
+  // Creates a StaleHostResolver that uses |inner_resolver| for actual
+  // resolution but potentially returns stale data according to |stale_options|.

[Ryan Sleevi, 2016/04/29 01:04:09]

s/resolution but/resolution, but/

[Julia Tuttle, 2016/04/29 18:04:30]

Done.

+  StaleHostResolver(std::unique_ptr<HostResolver> inner_resolver,
+                    const StaleOptions& stale_options);
+
+  ~StaleHostResolver() override;
+
+  // Resolves as a regular HostResolver, but if stale data is available and
+  // usable (according to the options passed to the constructor), returns it
+  // after the specified delay instead.

[Randy Smith (Not in Mondays), 2016/04/28 17:48:25]

I presume if fresh data is available in that time period, it's returned
preferentially?  The doc doesn't make that clear.

[Julia Tuttle, 2016/04/29 18:04:29]

Clarified.

+  //
+  // If stale data is returned, the StaleHostResolver allows the underlying
+  // request to continue in order to repopulate the cache.
+  int Resolve(const RequestInfo& info,
+              RequestPriority priority,
+              AddressList* addresses,
+              const CompletionCallback& callback,
+              RequestHandle* out_req,
+              const BoundNetLog& net_log) override;
+
+  // Cancels a request created by |Resolve|.
+  //
+  // It is not valid to cancel a request once it has returned a result, even if
+  // the result was stale data and the underlying request on the inner resolver
+  // is still running.
+  void CancelRequest(RequestHandle req) override;
+
+  // The remaining methods pass through to the inner resolver:
+

[Ryan Sleevi, 2016/04/29 01:04:09]

delete newline?

[Julia Tuttle, 2016/04/29 18:04:29]

Done.

+  int ResolveFromCache(const RequestInfo& info,
+                       AddressList* addresses,
+                       const BoundNetLog& net_log) override;
+  int ResolveStaleFromCache(const RequestInfo& info,
+                            AddressList* addresses,
+                            HostCache::StaleEntryInfo* stale_info,
+                            const BoundNetLog& net_log) override;
+  void SetDnsClientEnabled(bool enabled) override;
+  HostCache* GetHostCache() override;
+  std::unique_ptr<base::Value> GetDnsConfigAsValue() const override;
+
+ private:
+  class Request;
+
+  void OnRequestComplete(Request* request,
+                         const CompletionCallback& outer_callback,
+                         int error);
+
+  std::unique_ptr<HostResolver> resolver_;
+  StaleOptions options_;
+  std::set<Request*> pending_requests_;
+};
+
+}  // namespace net
+
+#endif  // NET_DNS_STALE_HOST_RESOLVER_H_