Cronet: Use stale DNS cache entries experimentally.

components/cronet/stale_host_resolver_unittest.cc

Index: components/cronet/stale_host_resolver_unittest.cc
diff --git a/components/cronet/stale_host_resolver_unittest.cc b/components/cronet/stale_host_resolver_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..9f6d1188ce45b5a07b68ae58633410cee0969b6c
--- /dev/null
+++ b/components/cronet/stale_host_resolver_unittest.cc
@@ -0,0 +1,497 @@
+// 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.
+
+#include "components/cronet/stale_host_resolver.h"
+
+#include "base/callback_helpers.h"
+#include "base/logging.h"
+#include "base/memory/ptr_util.h"
+#include "base/message_loop/message_loop.h"
+#include "base/run_loop.h"
+#include "base/test/test_timeouts.h"
+#include "base/values.h"
+#include "components/cronet/url_request_context_config.h"
+#include "net/base/net_errors.h"
+#include "net/cert/cert_verifier.h"
+#include "net/dns/host_resolver_proc.h"
+#include "net/http/http_network_session.h"
+#include "net/proxy/proxy_config.h"
+#include "net/proxy/proxy_config_service_fixed.h"
+#include "net/url_request/url_request_context.h"
+#include "net/url_request/url_request_context_builder.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace cronet {
+
+namespace {
+
+const char kHostname[] = "example.com";
+const char kCacheAddress[] = "1.1.1.1";
+const char kNetworkAddress[] = "2.2.2.2";
+const char kUninitializedAddress[] = "3.3.3.3";
+const int kCacheEntryTTLSec = 300;
+
+const int kNoStaleDelaySec = 0;
+const int kLongStaleDelaySec = 3600;
+const uint16_t kPort = 12345;
+
+const int kAgeFreshSec = 0;
+const int kAgeExpiredSec = kCacheEntryTTLSec * 2;
+
+const int kWaitTimeoutSec = 1;
+
+net::AddressList MakeAddressList(const char* ip_address_str) {
+  net::IPAddress address;
+  bool rv = address.AssignFromIPLiteral(ip_address_str);
+  DCHECK(rv);
+
+  net::AddressList address_list;
+  address_list.push_back(net::IPEndPoint(address, 0u));
+  return address_list;
+}
+
+class MockHostResolverProc : public net::HostResolverProc {
+ public:
+  MockHostResolverProc() : HostResolverProc(nullptr) {}
+
+  ~MockHostResolverProc() override {}
+
+  int Resolve(const std::string& hostname,
+              net::AddressFamily address_family,
+              net::HostResolverFlags host_resolver_flags,
+              net::AddressList* address_list,
+              int* os_error) override {
+    *address_list = MakeAddressList(kNetworkAddress);
+    return net::OK;
+  }
+};
+
+class StaleHostResolverTest : public testing::Test {
+ protected:
+  StaleHostResolverTest()
+      : mock_proc_(new MockHostResolverProc()),
+        resolver_(nullptr),
+        resolve_pending_(false),
+        resolve_complete_(false) {}
+
+  ~StaleHostResolverTest() {}
+
+  void SetStaleDelay(int stale_delay_sec) {
+    DCHECK(!resolver_);
+
+    options_.delay = base::TimeDelta::FromSeconds(stale_delay_sec);
+  }
+
+  void SetStaleUsability(int max_expired_time_sec,
+                         int max_stale_uses,
+                         bool allow_other_network) {
+    DCHECK(!resolver_);
+
+    options_.max_expired_time =
+        base::TimeDelta::FromSeconds(max_expired_time_sec);
+    options_.max_stale_uses = max_stale_uses;
+    options_.allow_other_network = allow_other_network;
+  }
+
+  void CreateResolver() {
+    DCHECK(!resolver_);
+
+    std::unique_ptr<net::HostResolverImpl> inner_resolver(
+        net::HostResolver::CreateDefaultResolverImpl(nullptr));
+
+    net::HostResolverImpl::ProcTaskParams proc_params(mock_proc_.get(), 1u);
+    inner_resolver->set_proc_params_for_test(proc_params);
+
+    stale_resolver_ = base::WrapUnique(
+        new StaleHostResolver(std::move(inner_resolver), options_));
+    resolver_ = stale_resolver_.get();
+  }
+
+  void DestroyResolver() {
+    DCHECK(stale_resolver_);
+
+    stale_resolver_.reset();
+    resolver_ = nullptr;
+  }
+
+  void SetResolver(net::HostResolver* resolver) {
+    DCHECK(!resolver_);
+
+    resolver_ = resolver;
+  }
+
+  void ClearResolver() {
+    DCHECK(resolver_);
+    DCHECK(!stale_resolver_);
+
+    resolver_ = nullptr;
+  }
+
+  void PopulateCache(int age_sec) {

[xunjieli, 2016/07/25 17:18:15]

Could you add a documentation to this method? 
It isn't as generic as it sounds -- it populates the hostcache with a particular
key.

[Julia Tuttle, 2016/07/25 19:29:15]

Done.

+    DCHECK(resolver_);
+    DCHECK(resolver_->GetHostCache());
+
+    base::TimeDelta ttl(base::TimeDelta::FromSeconds(kCacheEntryTTLSec));
+    net::HostCache::Key key(kHostname, net::ADDRESS_FAMILY_IPV4, 0);
+    net::HostCache::Entry entry(net::OK, MakeAddressList(kCacheAddress), ttl);
+    base::TimeDelta age = base::TimeDelta::FromSeconds(age_sec);
+    base::TimeTicks then = base::TimeTicks::Now() - age;
+    resolver_->GetHostCache()->Set(key, entry, then, ttl);
+  }
+
+  void OnNetworkChange() {
+    DCHECK(resolver_);
+    DCHECK(resolver_->GetHostCache());
+
+    resolver_->GetHostCache()->OnNetworkChange();
+  }
+
+  void LookupStale() {
+    DCHECK(resolver_);
+    DCHECK(resolver_->GetHostCache());
+
+    net::HostCache::Key key(kHostname, net::ADDRESS_FAMILY_IPV4, 0);
+    base::TimeTicks now = base::TimeTicks::Now();
+    const net::HostCache::Entry* entry;
+    net::HostCache::EntryStaleness stale;
+    entry = resolver_->GetHostCache()->LookupStale(key, now, &stale);
+    EXPECT_TRUE(entry != nullptr);

[xunjieli, 2016/07/25 17:18:15]

EXPECT_NE?

[Julia Tuttle, 2016/07/25 19:29:15]

EXPECT_TRUE(entry)?

+    EXPECT_TRUE(stale.is_stale());
+  }
+
+  void Resolve() {
+    DCHECK(resolver_);
+    EXPECT_FALSE(resolve_pending_);
+
+    net::HostResolver::RequestInfo info(net::HostPortPair(kHostname, kPort));
+    info.set_address_family(net::ADDRESS_FAMILY_IPV4);
+
+    resolve_pending_ = true;
+    resolve_complete_ = false;
+    resolve_addresses_ = MakeAddressList(kUninitializedAddress);
+    resolve_error_ = net::ERR_UNEXPECTED;
+
+    int rv =
+        resolver_->Resolve(info, net::DEFAULT_PRIORITY, &resolve_addresses_,
+                           base::Bind(&StaleHostResolverTest::OnResolveComplete,
+                                      base::Unretained(this)),
+                           &request_handle_, net::BoundNetLog());
+    if (rv != net::ERR_IO_PENDING) {
+      resolve_pending_ = false;
+      resolve_complete_ = true;
+      resolve_error_ = rv;
+    }
+  }
+
+  void WaitForResolve() {
+    if (!resolve_pending_)
+      return;
+
+    base::RunLoop run_loop;
+
+    // Run until resolve completes or timeout.
+    resolve_closure_ = run_loop.QuitWhenIdleClosure();

[xunjieli, 2016/07/25 17:18:15]

Can we use TestCompletionCallback and just wait for OnResolveComplete to be
invoked? Why do we also do this QuitWhenIdleClosure() here in WaitForResolve()?

[Julia Tuttle, 2016/07/25 19:29:16]

TestCompletionCallback won't give me the timeout, which I want.

[xunjieli, 2016/07/25 21:09:14]

I am probably missing something. What is the significance of kWaitTimeoutSec =
1s here? Why do we timeout at 1second?

I think this is probably tied to each test case. Could you make the timeout as
an argument of WaitForResolve(), so it is clearer in the call places on why that
particular timeout is used? 

+    base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
+        FROM_HERE, run_loop.QuitWhenIdleClosure(),
+        base::TimeDelta::FromSeconds(kWaitTimeoutSec));
+    run_loop.Run();
+  }
+
+  void WaitForIdle() {
+    base::RunLoop run_loop;
+
+    base::ThreadTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE, run_loop.QuitWhenIdleClosure());
+    run_loop.Run();
+  }
+
+  void Cancel() {
+    DCHECK(resolver_);
+    EXPECT_TRUE(resolve_pending_);
+
+    resolver_->CancelRequest(request_handle_);
+    request_handle_ = nullptr;
+
+    resolve_pending_ = false;
+  }
+
+  void OnResolveComplete(int error) {
+    EXPECT_TRUE(resolve_pending_);
+
+    resolve_error_ = error;
+    resolve_pending_ = false;
+    resolve_complete_ = true;
+
+    if (!resolve_closure_.is_null())
+      base::ResetAndReturn(&resolve_closure_).Run();
+  }
+
+  size_t pending_request_count() const {
+    DCHECK(stale_resolver_);
+    return stale_resolver_->pending_request_count_for_testing();
+  }
+
+  base::MessageLoopForIO message_loop_for_io_;

[xunjieli, 2016/07/25 17:18:14]

Could you add a comment to why |message_loop_for_io_| is needed here?

[Julia Tuttle, 2016/07/25 19:29:15]

Done.

+  scoped_refptr<MockHostResolverProc> mock_proc_;

[xunjieli, 2016/07/25 17:18:14]

Include base/memory/ref_counted.h ?

[Julia Tuttle, 2016/07/25 19:29:15]

Done.

+
+  net::HostResolver* resolver_;
+  StaleHostResolver::StaleOptions options_;
+  std::unique_ptr<StaleHostResolver> stale_resolver_;

[xunjieli, 2016/07/25 17:18:15]

A lot of these member variables should be private and not protected.

[Julia Tuttle, 2016/07/25 19:29:15]

Done.

+
+  base::TimeTicks now_;
+  net::HostResolver::RequestHandle request_handle_;
+  bool resolve_pending_;
+  bool resolve_complete_;
+  net::AddressList resolve_addresses_;
+  int resolve_error_;
+
+  base::Closure resolve_closure_;
+};
+
+TEST_F(StaleHostResolverTest, Null) {}

[xunjieli, 2016/07/25 17:18:15]

What dos this test do? It creates the test fixture and tears it down? It seems
that it doesn't do anything meaningful. Could you add a comment?

[Julia Tuttle, 2016/07/25 19:29:16]

This and the next case are just here to make it easier to tell whether a crash
is in the test harness setup and teardown, or the resolver setup and teardown,
or the request itself. I'll add comments to make it clear I'm testing each of
those parts.

+
+TEST_F(StaleHostResolverTest, Create) {
+  CreateResolver();

[xunjieli, 2016/07/25 17:18:15]

This test create the resolver, but it doesn't do anything with it. Why do we
need this test case?

[Julia Tuttle, 2016/07/25 19:29:15]

See above.

+}
+
+TEST_F(StaleHostResolverTest, Network) {
+  CreateResolver();
+
+  Resolve();
+  EXPECT_EQ(1u, pending_request_count());
+  WaitForResolve();
+  EXPECT_EQ(0u, pending_request_count());
+
+  EXPECT_TRUE(resolve_complete_);
+  EXPECT_EQ(net::OK, resolve_error_);
+  EXPECT_EQ(1u, resolve_addresses_.size());
+  EXPECT_EQ(kNetworkAddress, resolve_addresses_[0].ToStringWithoutPort());
+}
+
+TEST_F(StaleHostResolverTest, FreshCache) {
+  CreateResolver();
+  PopulateCache(kAgeFreshSec);
+
+  Resolve();
+  EXPECT_EQ(0u, pending_request_count());
+
+  EXPECT_TRUE(resolve_complete_);
+  EXPECT_EQ(net::OK, resolve_error_);
+  EXPECT_EQ(1u, resolve_addresses_.size());
+  EXPECT_EQ(kCacheAddress, resolve_addresses_[0].ToStringWithoutPort());
+
+  WaitForIdle();
+}
+
+TEST_F(StaleHostResolverTest, StaleCache) {
+  SetStaleDelay(kNoStaleDelaySec);
+  CreateResolver();
+  PopulateCache(kAgeExpiredSec);
+
+  Resolve();
+  EXPECT_EQ(1u, pending_request_count());
+  WaitForResolve();
+  EXPECT_EQ(0u, pending_request_count());
+
+  EXPECT_TRUE(resolve_complete_);
+  EXPECT_EQ(net::OK, resolve_error_);
+  EXPECT_EQ(1u, resolve_addresses_.size());
+  EXPECT_EQ(kCacheAddress, resolve_addresses_[0].ToStringWithoutPort());
+}
+
+TEST_F(StaleHostResolverTest, NetworkWithStaleCache) {
+  SetStaleDelay(kLongStaleDelaySec);
+  CreateResolver();
+  PopulateCache(kAgeExpiredSec);
+
+  Resolve();
+  EXPECT_EQ(1u, pending_request_count());
+  WaitForResolve();
+  EXPECT_EQ(0u, pending_request_count());
+
+  EXPECT_TRUE(resolve_complete_);
+  EXPECT_EQ(net::OK, resolve_error_);
+  EXPECT_EQ(1u, resolve_addresses_.size());
+  EXPECT_EQ(kNetworkAddress, resolve_addresses_[0].ToStringWithoutPort());
+}
+
+TEST_F(StaleHostResolverTest, CancelWithNoCache) {
+  SetStaleDelay(kNoStaleDelaySec);
+  CreateResolver();
+
+  Resolve();
+  EXPECT_EQ(1u, pending_request_count());
+
+  Cancel();
+  EXPECT_EQ(0u, pending_request_count());
+
+  EXPECT_FALSE(resolve_complete_);
+
+  // Make sure there's no lingering |OnResolveComplete()| callback waiting.
+  WaitForIdle();
+}
+
+TEST_F(StaleHostResolverTest, CancelWithStaleCache) {
+  SetStaleDelay(kLongStaleDelaySec);
+  CreateResolver();
+  PopulateCache(kAgeExpiredSec);
+
+  Resolve();
+  EXPECT_EQ(1u, pending_request_count());
+
+  Cancel();
+  EXPECT_EQ(0u, pending_request_count());
+
+  EXPECT_FALSE(resolve_complete_);
+
+  // Make sure there's no lingering |OnResolveComplete()| callback waiting.
+  WaitForIdle();
+}
+
+// CancelWithFreshCache makes no sense; the request would've returned
+// synchronously.
+
+TEST_F(StaleHostResolverTest, StaleUsability) {
+  const struct {
+    int max_expired_time_sec;
+    int max_stale_uses;
+    bool allow_other_network;
+
+    int age_sec;
+    int stale_use;
+    int network_changes;
+
+    bool usable;
+  } kUsabilityTestCases[] = {
+      // Fresh data always accepted.
+      {0, 0, true, -1, 1, 0, true},
+      {1, 1, false, -1, 1, 0, true},
+
+      // Unlimited expired time accepts non-zero time.
+      {0, 0, true, 1, 1, 0, true},
+
+      // Limited expired time accepts before but not after limit.
+      {2, 0, true, 1, 1, 0, true},
+      {2, 0, true, 3, 1, 0, false},
+
+      // Unlimited stale uses accepts first and later uses.
+      {2, 0, true, 1, 1, 0, true},
+      {2, 0, true, 1, 9, 0, true},
+
+      // Limited stale uses accepts up to and including limit.
+      {2, 2, true, 1, 1, 0, true},
+      {2, 2, true, 1, 2, 0, true},
+      {2, 2, true, 1, 3, 0, false},
+      {2, 2, true, 1, 9, 0, false},
+
+      // Allowing other networks accepts zero or more network changes.
+      {2, 0, true, 1, 1, 0, true},
+      {2, 0, true, 1, 1, 1, true},
+      {2, 0, true, 1, 1, 9, true},
+
+      // Disallowing other networks only accepts zero network changes.
+      {2, 0, false, 1, 1, 0, true},
+      {2, 0, false, 1, 1, 1, false},
+      {2, 0, false, 1, 1, 9, false},
+  };
+
+  SetStaleDelay(kNoStaleDelaySec);
+
+  for (size_t i = 0; i < arraysize(kUsabilityTestCases); ++i) {
+    const auto& test_case = kUsabilityTestCases[i];
+
+    SetStaleUsability(test_case.max_expired_time_sec, test_case.max_stale_uses,
+                      test_case.allow_other_network);
+    CreateResolver();
+    PopulateCache(kCacheEntryTTLSec + test_case.age_sec);
+    for (int j = 0; j < test_case.network_changes; ++j)
+      OnNetworkChange();
+    for (int j = 0; j < test_case.stale_use - 1; ++j)
+      LookupStale();
+
+    Resolve();
+    WaitForResolve();
+    EXPECT_TRUE(resolve_complete_) << i;
+    EXPECT_EQ(net::OK, resolve_error_) << i;
+    EXPECT_EQ(1u, resolve_addresses_.size()) << i;
+    {
+      const char* expected = test_case.usable ? kCacheAddress : kNetworkAddress;
+      EXPECT_EQ(expected, resolve_addresses_[0].ToStringWithoutPort()) << i;
+    }
+
+    DestroyResolver();
+  }
+}
+
+TEST_F(StaleHostResolverTest, CreatedByContext) {
+  URLRequestContextConfig config(
+      // Enable QUIC.
+      true,
+      // QUIC User Agent ID.
+      "Default QUIC User Agent ID",
+      // Enable SPDY.
+      true,
+      // Enable SDCH.
+      false,
+      // Type of http cache.
+      URLRequestContextConfig::HttpCacheType::DISK,
+      // Max size of http cache in bytes.
+      1024000,
+      // Disable caching for HTTP responses. Other information may be stored in
+      // the cache.
+      false,
+      // Storage path for http cache and cookie storage.
+      "/data/data/org.chromium.net/app_cronet_test/test_storage",
+      // User-Agent request header field.
+      "fake agent",
+      // JSON encoded experimental options.
+      "{\"AsyncDNS\":{\"enable\":false},"
+      "\"StaleDNS\":{\"enable\":true,"
+      "\"delay_ms\":0,"
+      "\"max_expired_time_ms\":0,"
+      "\"max_stale_uses\":0}}",
+      // Data reduction proxy key.
+      "",
+      // Data reduction proxy.
+      "",
+      // Fallback data reduction proxy.
+      "",
+      // Data reduction proxy secure proxy check URL.
+      "",
+      // MockCertVerifier to use for testing purposes.
+      std::unique_ptr<net::CertVerifier>(),
+      // Enable network quality estimator.
+      false,
+      // Enable Public Key Pinning bypass for local trust anchors.
+      true,
+      // Certificate verifier cache data.
+      "");
+
+  net::URLRequestContextBuilder builder;
+  net::NetLog net_log;
+  config.ConfigureURLRequestContextBuilder(&builder, &net_log, nullptr);
+  // Set a ProxyConfigService to avoid DCHECK failure when building.
+  builder.set_proxy_config_service(base::WrapUnique(
+      new net::ProxyConfigServiceFixed(net::ProxyConfig::CreateDirect())));
+  std::unique_ptr<net::URLRequestContext> context(builder.Build());
+
+  // Duplicate StaleCache test case to ensure StaleHostResolver was created:
+
+  // N.B. Experimental config above sets 0ms stale delay.
+  SetResolver(context->host_resolver());
+  PopulateCache(kAgeExpiredSec);
+
+  Resolve();
+  EXPECT_FALSE(resolve_complete_);
+  WaitForResolve();
+
+  EXPECT_TRUE(resolve_complete_);
+  EXPECT_EQ(net::OK, resolve_error_);
+  EXPECT_EQ(1u, resolve_addresses_.size());
+  EXPECT_EQ(kCacheAddress, resolve_addresses_[0].ToStringWithoutPort());
+}
+
+}  // namespace
+
+}  // namespace cronet