[net] Change factory methods for HostResolver and HostCache to return a scoped_ptr

net/base/host_resolver.cc

Index: net/base/host_resolver.cc
diff --git a/net/base/host_resolver.cc b/net/base/host_resolver.cc
index 3e86aff4165e2a6da71f18f6b0a1f8a1f5f907cb..ece95d459c141215b63187ff393680ca215b93e1 100644
--- a/net/base/host_resolver.cc
+++ b/net/base/host_resolver.cc
@@ -4,8 +4,23 @@
 
 #include "net/base/host_resolver.h"
 
+#include "net/base/host_cache.h"
+#include "net/base/host_resolver_impl.h"
+#include "net/dns/dns_client.h"
+#include "net/dns/dns_config_service.h"
+
 namespace net {
 
+namespace {

[darin (slow to review), 2012/10/09 17:38:34]

nit: new line after open of namespace

[szym, 2012/10/09 21:40:43]

Done.

+// Maximum of 6 concurrent resolver threads (excluding retries).
+// Some routers (or resolvers) appear to start to provide host-not-found if
+// too many simultaneous resolutions are pending.  This number needs to be
+// further optimized, but 8 is what FF currently does. We found some routers
+// that limit this to 6, so we're temporarily holding it at that level.
+static const size_t kDefaultMaxProcTasks = 6u;

[darin (slow to review), 2012/10/09 17:38:34]

static and anonymous namespace are redundant.  you only need one.

[szym, 2012/10/09 21:40:43]

Done.

+
+}  // namespace
+
 HostResolver::RequestInfo::RequestInfo(const HostPortPair& host_port_pair)
     : host_port_pair_(host_port_pair),
       address_family_(ADDRESS_FAMILY_UNSPECIFIED),
@@ -33,6 +48,47 @@ base::Value* HostResolver::GetDnsConfigAsValue() const {
   return NULL;
 }
 
+// static
+scoped_ptr<HostResolver>
+HostResolver::CreateSystemResolver(size_t max_concurrent_resolves,

[darin (slow to review), 2012/10/09 17:38:34]

nit: looks like you can format like this:

  scoped_ptr<HostResolver> HostResolver::CreateSystemResolver(
      size_t max_concurrent_resolves,
      ...) {
  }

[szym, 2012/10/09 21:40:43]

Done.

+                                   size_t max_retry_attempts,
+                                   bool enable_cache,
+                                   bool enable_async,
+                                   NetLog* net_log) {
+  if (max_concurrent_resolves == kDefaultParallelism)
+    max_concurrent_resolves = kDefaultMaxProcTasks;
+
+  scoped_ptr<HostCache> cache(NULL);

[szym, 2012/10/09 21:40:43]

Replaced those NULLs with default constructors.

+  if (enable_cache)
+    cache = HostCache::CreateDefaultCache();
+  scoped_ptr<DnsClient> dns_client(NULL);
+  if (enable_async) {
+#if !defined(ENABLE_BUILT_IN_DNS)
+    NOTREACHED();
+    return scoped_ptr<HostResolver>(NULL);
+#else
+    dns_client = DnsClient::CreateClient(net_log);
+#endif
+  }
+
+  return scoped_ptr<HostResolver>(new HostResolverImpl(
+      cache.Pass(),
+      PrioritizedDispatcher::Limits(NUM_PRIORITIES, max_concurrent_resolves),
+      HostResolverImpl::ProcTaskParams(NULL, max_retry_attempts),
+      dns_client.Pass(),
+      net_log));
+}
+
+// static
+scoped_ptr<HostResolver>

[darin (slow to review), 2012/10/09 17:38:34]

ditto

[szym, 2012/10/09 21:40:43]

Done. But now that HostResolver::Options has a default ctor, is it really needed
or should it be replaced by:
  CreateSystemResolver(HostResolver::Options(), net_log);

+HostResolver::CreateDefaultResolver(NetLog* net_log) {
+  return CreateSystemResolver(kDefaultParallelism,
+                              kDefaultRetryAttempts,
+                              true /* enable_cache */,
+                              false /* enable_async */,
+                              net_log);
+}
+
 HostResolver::HostResolver() {
 }