NetLogEventParameter to Callback refactoring 1,

net/base/host_resolver_impl.cc

 // 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.
 
 #include "net/base/host_resolver_impl.h"
 
 #if defined(OS_WIN)
 #include <Winsock2.h>
 #elif defined(OS_POSIX)
 #include <netdb.h>
@@ skipping 14 matching lines @@
 #include "base/metrics/field_trial.h"
 #include "base/metrics/histogram.h"
 #include "base/stl_util.h"
 #include "base/string_util.h"
 #include "base/threading/worker_pool.h"
 #include "base/time.h"
 #include "base/utf_string_conversions.h"
 #include "base/values.h"
 #include "net/base/address_family.h"
 #include "net/base/address_list.h"
-#include "net/base/address_list_net_log_param.h"
 #include "net/base/dns_reloader.h"
 #include "net/base/host_port_pair.h"
 #include "net/base/host_resolver_proc.h"
 #include "net/base/net_errors.h"
 #include "net/base/net_log.h"
 #include "net/base/net_util.h"
 #include "net/dns/dns_client.h"
 #include "net/dns/dns_config_service.h"
 #include "net/dns/dns_protocol.h"
 #include "net/dns/dns_response.h"
@@ skipping 124 matching lines @@
   virtual ~CallSystemHostResolverProc() {}
 };
 
 void EnsurePortOnAddressList(uint16 port, AddressList* list) {
   DCHECK(list);
   if (list->empty() || list->front().port() == port)
     return;
   SetPortOnAddressList(port, list);
 }
 
-// Extra parameters to attach to the NetLog when the resolve failed.
-class ProcTaskFailedParams : public NetLog::EventParameters {
- public:
-  ProcTaskFailedParams(uint32 attempt_number, int net_error, int os_error)
-      : attempt_number_(attempt_number),
-        net_error_(net_error),
-        os_error_(os_error) {
+// Creates NetLog parameters when the resolve failed.
+base::Value* NetLogProcTaskFailedCallback(uint32 attempt_number,
+                                          int net_error,
+                                          int os_error,
+                                          NetLog::LogLevel /* log_level */) {
+  DictionaryValue* dict = new DictionaryValue();
+  if (attempt_number)
+    dict->SetInteger("attempt_number", attempt_number);
+
+  dict->SetInteger("net_error", net_error);
+
+  if (os_error) {
+    dict->SetInteger("os_error", os_error);
+#if defined(OS_POSIX)
+    dict->SetString("os_error_string", gai_strerror(os_error));
+#elif defined(OS_WIN)
+    // Map the error code to a human-readable string.
+    LPWSTR error_string = NULL;
+    int size = FormatMessage(FORMAT_MESSAGE_ALLOCATE_BUFFER |
+                             FORMAT_MESSAGE_FROM_SYSTEM,
+                             0,  // Use the internal message table.
+                             os_error,
+                             0,  // Use default language.
+                             (LPWSTR)&error_string,
+                             0,  // Buffer size.
+                             0);  // Arguments (unused).
+    dict->SetString("os_error_string", WideToUTF8(error_string));
+    LocalFree(error_string);
+#endif
   }
 
-  virtual Value* ToValue() const OVERRIDE {
-    DictionaryValue* dict = new DictionaryValue();
-    if (attempt_number_)
-      dict->SetInteger("attempt_number", attempt_number_);
+  return dict;
+}
 
-    dict->SetInteger("net_error", net_error_);
-
-    if (os_error_) {
-      dict->SetInteger("os_error", os_error_);
-#if defined(OS_POSIX)
-      dict->SetString("os_error_string", gai_strerror(os_error_));
-#elif defined(OS_WIN)
-      // Map the error code to a human-readable string.
-      LPWSTR error_string = NULL;
-      int size = FormatMessage(FORMAT_MESSAGE_ALLOCATE_BUFFER |
-                               FORMAT_MESSAGE_FROM_SYSTEM,
-                               0,  // Use the internal message table.
-                               os_error_,
-                               0,  // Use default language.
-                               (LPWSTR)&error_string,
-                               0,  // Buffer size.
-                               0);  // Arguments (unused).
-      dict->SetString("os_error_string", WideToUTF8(error_string));
-      LocalFree(error_string);
-#endif
-    }
-
-    return dict;
-  }
-
- protected:
-  virtual ~ProcTaskFailedParams() {}
-
- private:
-  const uint32 attempt_number_;
-  const int net_error_;
-  const int os_error_;
+// Creates NetLog parameters when the DnsTask failed.
+base::Value* NetLogDnsTaskFailedCallback(int net_error,
+                                         int dns_error,
+                                         NetLog::LogLevel /* log_level */) {
+  DictionaryValue* dict = new DictionaryValue();
+  dict->SetInteger("net_error", net_error);
+  if (dns_error)
+    dict->SetInteger("dns_error", dns_error);
+  return dict;
 };
 
-// Extra parameters to attach to the NetLog when the DnsTask failed.
-class DnsTaskFailedParams : public NetLog::EventParameters {
- public:
-  DnsTaskFailedParams(int net_error, int dns_error)
-      : net_error_(net_error), dns_error_(dns_error) {
-  }
+// Creates NetLog parameters containing the information in a RequestInfo object,
+// along with the associated NetLog::Source.
+base::Value* NetLogRequestInfoCallback(const NetLog::Source& source,
+                                       const HostResolver::RequestInfo* info,
+                                       NetLog::LogLevel /* log_level */) {
+  DictionaryValue* dict = new DictionaryValue();
+  source.AddToEventParameters(dict);
 
-  virtual Value* ToValue() const OVERRIDE {
-    DictionaryValue* dict = new DictionaryValue();
-    dict->SetInteger("net_error", net_error_);
-    if (dns_error_)
-      dict->SetInteger("dns_error", dns_error_);
-    return dict;
-  }
+  dict->SetString("host", info->host_port_pair().ToString());
+  dict->SetInteger("address_family",
+                   static_cast<int>(info->address_family()));
+  dict->SetBoolean("allow_cached_response", info->allow_cached_response());
+  dict->SetBoolean("is_speculative", info->is_speculative());
+  dict->SetInteger("priority", info->priority());
+  return dict;
+}
 
- protected:
-  virtual ~DnsTaskFailedParams() {}
+// Creates NetLog parameters for the creation of a HostResolverImpl::Job.
+base::Value* NetLogJobCreationCallback(const NetLog::Source& source,
+                                       const std::string* host,
+                                       NetLog::LogLevel /* log_level */) {
+  DictionaryValue* dict = new DictionaryValue();
+  source.AddToEventParameters(dict);
+  dict->SetString("host", *host);
+  return dict;
+}
 
- private:
-  const int net_error_;
-  const int dns_error_;
-};
+// Creates NetLog parameters for HOST_RESOLVER_IMPL_JOB_ATTACH/DETACH events.
+base::Value* NetLogJobAttachCallback(const NetLog::Source& source,
+                                     RequestPriority priority,
+                                     NetLog::LogLevel /* log_level */) {
+  DictionaryValue* dict = new DictionaryValue();
+  source.AddToEventParameters(dict);
+  dict->SetInteger("priority", priority);
+  return dict;
+}
 
-// Parameters representing the information in a RequestInfo object, along with
-// the associated NetLog::Source.
-class RequestInfoParameters : public NetLog::EventParameters {
- public:
-  RequestInfoParameters(const HostResolver::RequestInfo& info,
-                        const NetLog::Source& source)
-      : info_(info), source_(source) {}
-
-  virtual Value* ToValue() const OVERRIDE {
-    DictionaryValue* dict = new DictionaryValue();
-    dict->SetString("host", info_.host_port_pair().ToString());
-    dict->SetInteger("address_family",
-                     static_cast<int>(info_.address_family()));
-    dict->SetBoolean("allow_cached_response", info_.allow_cached_response());
-    dict->SetBoolean("is_speculative", info_.is_speculative());
-    dict->SetInteger("priority", info_.priority());
-
-    if (source_.is_valid())
-      dict->Set("source_dependency", source_.ToValue());
-
-    return dict;
-  }
-
- protected:
-  virtual ~RequestInfoParameters() {}
-
- private:
-  const HostResolver::RequestInfo info_;
-  const NetLog::Source source_;
-};
-
-// Parameters associated with the creation of a HostResolverImpl::Job.
-class JobCreationParameters : public NetLog::EventParameters {
- public:
-  JobCreationParameters(const std::string& host,
-                        const NetLog::Source& source)
-      : host_(host), source_(source) {}
-
-  virtual Value* ToValue() const OVERRIDE {
-    DictionaryValue* dict = new DictionaryValue();
-    dict->SetString("host", host_);
-    dict->Set("source_dependency", source_.ToValue());
-    return dict;
-  }
-
- protected:
-  virtual ~JobCreationParameters() {}
-
- private:
-  const std::string host_;
-  const NetLog::Source source_;
-};
-
-// Parameters of the HOST_RESOLVER_IMPL_JOB_ATTACH/DETACH event.
-class JobAttachParameters : public NetLog::EventParameters {
- public:
-  JobAttachParameters(const NetLog::Source& source,
-                      RequestPriority priority)
-      : source_(source), priority_(priority) {}
-
-  virtual Value* ToValue() const OVERRIDE {
-    DictionaryValue* dict = new DictionaryValue();
-    dict->Set("source_dependency", source_.ToValue());
-    dict->SetInteger("priority", priority_);
-    return dict;
-  }
-
- protected:
-  virtual ~JobAttachParameters() {}
-
- private:
-  const NetLog::Source source_;
-  const RequestPriority priority_;
-};
-
-// Parameters of the DNS_CONFIG_CHANGED event.
-class DnsConfigParameters : public NetLog::EventParameters {
- public:
-  explicit DnsConfigParameters(const DnsConfig& config)
-      : num_hosts_(config.hosts.size()) {
-    config_.CopyIgnoreHosts(config);
-  }
-
-  virtual Value* ToValue() const OVERRIDE {
-    Value* value = config_.ToValue();
-    if (!value)
-      return NULL;
-    DictionaryValue* dict;
-    if (value->GetAsDictionary(&dict))
-      dict->SetInteger("num_hosts", num_hosts_);
-    return value;
-  }
-
- protected:
-  virtual ~DnsConfigParameters() {}
-
- private:
-  DnsConfig config_;  // Does not include DnsHosts to save memory and work.
-  const size_t num_hosts_;
-};
+// Creates NetLog parameters for the DNS_CONFIG_CHANGED event.
+base::Value* NetLogDnsConfigCallback(const DnsConfig* config,
+                                     NetLog::LogLevel /* log_level */) {
+  return config->ToValue();

[eroman, 2012/06/11 23:42:25]

I didn't look in detail, but the original also set a num_hosts parameter.

[mmenke, 2012/06/12 00:42:19]

In the original, we made a copy of the config, except for the hosts part (Since
it can be big).  We go the count from the original, and passed it along.  In
this, since we're using the original one, we don't need to overwrite the
num_hosts, which is also written by ToValue.

+}
 
 // The logging routines are defined here because some requests are resolved
 // without a Request object.
 
 // Logs when a request has just been started.
 void LogStartRequest(const BoundNetLog& source_net_log,
                      const BoundNetLog& request_net_log,
                      const HostResolver::RequestInfo& info) {
   source_net_log.BeginEvent(
       NetLog::TYPE_HOST_RESOLVER_IMPL,
-      make_scoped_refptr(new NetLogSourceParameter(
-          "source_dependency", request_net_log.source())));
+      request_net_log.source().ToEventParametersCallback());
 
   request_net_log.BeginEvent(
       NetLog::TYPE_HOST_RESOLVER_IMPL_REQUEST,
-      make_scoped_refptr(new RequestInfoParameters(
-          info, source_net_log.source())));
+      base::Bind(&NetLogRequestInfoCallback, source_net_log.source(), &info));
 }
 
 // Logs when a request has just completed (before its callback is run).
 void LogFinishRequest(const BoundNetLog& source_net_log,
                       const BoundNetLog& request_net_log,
                       const HostResolver::RequestInfo& info,
                       int net_error) {
   request_net_log.EndEventWithNetErrorCode(
       NetLog::TYPE_HOST_RESOLVER_IMPL_REQUEST, net_error);
   source_net_log.EndEvent(NetLog::TYPE_HOST_RESOLVER_IMPL, NULL);
@@ skipping 299 matching lines @@
       // returned (IO_PENDING).
       origin_loop_->PostTask(
           FROM_HERE,
           base::Bind(&ProcTask::OnLookupComplete, this, AddressList(),
                      start_time, attempt_number_, ERR_UNEXPECTED, 0));
       return;
     }
 
     net_log_.AddEvent(
         NetLog::TYPE_HOST_RESOLVER_IMPL_ATTEMPT_STARTED,
-        make_scoped_refptr(new NetLogIntegerParameter(
-            "attempt_number", attempt_number_)));
+        NetLog::IntegerCallback("attempt_number", attempt_number_));
 
     // If we don't get the results within a given time, RetryIfNotComplete
     // will start a new attempt on a different worker thread if none of our
     // outstanding attempts have completed yet.
     if (attempt_number_ <= params_.max_retry_attempts) {
       origin_loop_->PostDelayedTask(
           FROM_HERE,
           base::Bind(&ProcTask::RetryIfNotComplete, this),
           params_.unresponsive_delay);
     }
@@ skipping 52 matching lines @@
     // If this is the first attempt that is finishing later, then record data
     // for the first attempt. Won't contaminate with retry attempt's data.
     if (!was_retry_attempt)
       RecordPerformanceHistograms(start_time, error, os_error);
 
     RecordAttemptHistograms(start_time, attempt_number, error, os_error);
 
     if (was_canceled())
       return;
 
-    scoped_refptr<NetLog::EventParameters> params;
+    NetLog::ParametersCallback callback;

[eroman, 2012/06/11 23:42:25]

optional nit: I suggest calling this "netlog_callback" or "params_callback"
simply because when reading "callback" I think the callback for the lookup
completion.

[mmenke, 2012/06/12 00:42:19]

Done.

     if (error != OK) {
-      params = new ProcTaskFailedParams(attempt_number, error, os_error);
+      callback = base::Bind(&NetLogProcTaskFailedCallback,
+                            attempt_number,
+                            error,
+                            os_error);
     } else {
-      params = new NetLogIntegerParameter("attempt_number", attempt_number);
+      callback = NetLog::IntegerCallback("attempt_number", attempt_number);
     }
-    net_log_.AddEvent(NetLog::TYPE_HOST_RESOLVER_IMPL_ATTEMPT_FINISHED, params);
+    net_log_.AddEvent(NetLog::TYPE_HOST_RESOLVER_IMPL_ATTEMPT_FINISHED,
+                      callback);
 
     if (was_completed())
       return;
 
     // Copy the results from the first worker thread that resolves the host.
     results_ = results;
     completed_attempt_number_ = attempt_number;
     completed_attempt_error_ = error;
 
     if (was_retry_attempt) {
       // If retry attempt finishes before 1st attempt, then get stats on how
       // much time is saved by having spawned an extra attempt.
       retry_attempt_finished_time_ = base::TimeTicks::Now();
     }
 
     if (error != OK) {
-      params = new ProcTaskFailedParams(0, error, os_error);
+      callback = base::Bind(&NetLogProcTaskFailedCallback, 0, error, os_error);
     } else {
-      params = new AddressListNetLogParam(results_);
+      callback = base::Bind(&AddressList::NetLogCallback,
+                            base::Unretained(&results_));
     }
-    net_log_.EndEvent(NetLog::TYPE_HOST_RESOLVER_IMPL_PROC_TASK, params);
+    net_log_.EndEvent(NetLog::TYPE_HOST_RESOLVER_IMPL_PROC_TASK, callback);
 
     callback_.Run(error, results_);
   }
 
   void RecordPerformanceHistograms(const base::TimeTicks& start_time,
                                    const int error,
                                    const int os_error) const {
     DCHECK(origin_loop_->BelongsToCurrentThread());
     enum Category {  // Used in HISTOGRAM_ENUMERATION.
       RESOLVE_SUCCESS,
@@ skipping 292 matching lines @@
       CHECK(response);
       DNS_HISTOGRAM("AsyncDNS.TransactionSuccess",
                     base::TimeTicks::Now() - start_time);
       AddressList addr_list;
       base::TimeDelta ttl;
       result = response->ParseToAddressList(&addr_list, &ttl);
       UMA_HISTOGRAM_ENUMERATION("AsyncDNS.ParseToAddressList",
                                 result,
                                 DnsResponse::DNS_PARSE_RESULT_MAX);
       if (result == DnsResponse::DNS_SUCCESS) {
-        net_log_.EndEvent(NetLog::TYPE_HOST_RESOLVER_IMPL_DNS_TASK,
-                          new AddressListNetLogParam(addr_list));
+        net_log_.EndEvent(
+            NetLog::TYPE_HOST_RESOLVER_IMPL_DNS_TASK,
+            base::Bind(&AddressList::NetLogCallback,
+                       base::Unretained(&addr_list)));
         callback_.Run(net_error, addr_list, ttl);
         return;
       }
       net_error = ERR_DNS_MALFORMED_RESPONSE;
     } else {
       DNS_HISTOGRAM("AsyncDNS.TransactionFailure",
                     base::TimeTicks::Now() - start_time);
     }
-    net_log_.EndEvent(NetLog::TYPE_HOST_RESOLVER_IMPL_DNS_TASK,
-                      new DnsTaskFailedParams(net_error, result));
+    net_log_.EndEvent(
+        NetLog::TYPE_HOST_RESOLVER_IMPL_DNS_TASK,
+        base::Bind(&NetLogDnsTaskFailedCallback, net_error, result));
     callback_.Run(net_error, AddressList(), base::TimeDelta());
   }
 
  private:
   // The listener to the results of this DnsTask.
   Callback callback_;
 
   const BoundNetLog net_log_;
 
   scoped_ptr<DnsTransaction> transaction_;
@@ skipping 12 matching lines @@
       : resolver_(resolver->AsWeakPtr()),
         key_(key),
         had_non_speculative_request_(false),
         had_dns_config_(false),
         net_log_(BoundNetLog::Make(request_net_log.net_log(),
                                    NetLog::SOURCE_HOST_RESOLVER_IMPL_JOB)) {
     request_net_log.AddEvent(NetLog::TYPE_HOST_RESOLVER_IMPL_CREATE_JOB, NULL);
 
     net_log_.BeginEvent(
         NetLog::TYPE_HOST_RESOLVER_IMPL_JOB,
-        make_scoped_refptr(new JobCreationParameters(
-            key_.hostname, request_net_log.source())));
+        base::Bind(&NetLogJobCreationCallback,
+                   request_net_log.source(),
+                   &key_.hostname));
   }
 
   virtual ~Job() {
     if (is_running()) {
       // |resolver_| was destroyed with this Job still in flight.
       // Clean-up, record in the log, but don't run any callbacks.
       if (is_proc_running()) {
         proc_task_->Cancel();
         proc_task_ = NULL;
       }
@@ skipping 27 matching lines @@
   }
 
   void AddRequest(scoped_ptr<Request> req) {
     DCHECK_EQ(key_.hostname, req->info().hostname());
 
     req->set_job(this);
     priority_tracker_.Add(req->info().priority());
 
     req->request_net_log().AddEvent(
         NetLog::TYPE_HOST_RESOLVER_IMPL_JOB_ATTACH,
-        make_scoped_refptr(new NetLogSourceParameter(
-            "source_dependency", net_log_.source())));
+        net_log_.source().ToEventParametersCallback());
 
     net_log_.AddEvent(
         NetLog::TYPE_HOST_RESOLVER_IMPL_JOB_REQUEST_ATTACH,
-        make_scoped_refptr(new JobAttachParameters(
-            req->request_net_log().source(), priority())));
+        base::Bind(&NetLogJobAttachCallback,
+                   req->request_net_log().source(),
+                   priority()));
 
     // TODO(szym): Check if this is still needed.
     if (!req->info().is_speculative()) {
       had_non_speculative_request_ = true;
       if (proc_task_)
         proc_task_->set_had_non_speculative_request();
     }
 
     requests_.push_back(req.release());
 
     if (is_queued())
       handle_ = resolver_->dispatcher_.ChangePriority(handle_, priority());
   }
 
   // Marks |req| as cancelled. If it was the last active Request, also finishes
   // this Job marking it either as aborted or cancelled, and deletes it.
   void CancelRequest(Request* req) {
     DCHECK_EQ(key_.hostname, req->info().hostname());
     DCHECK(!req->was_canceled());
 
     // Don't remove it from |requests_| just mark it canceled.
     req->MarkAsCanceled();
     LogCancelRequest(req->source_net_log(), req->request_net_log(),
                      req->info());
 
     priority_tracker_.Remove(req->info().priority());
     net_log_.AddEvent(
         NetLog::TYPE_HOST_RESOLVER_IMPL_JOB_REQUEST_DETACH,
-        make_scoped_refptr(new JobAttachParameters(
-            req->request_net_log().source(), priority())));
+        base::Bind(&NetLogJobAttachCallback,
+                   req->request_net_log().source(),
+                   priority()));
 
     if (num_active_requests() > 0) {
       if (is_queued())
         handle_ = resolver_->dispatcher_.ChangePriority(handle_, priority());
     } else {
       // If we were called from a Request's callback within CompleteRequests,
       // that Request could not have been cancelled, so num_active_requests()
       // could not be 0. Therefore, we are not in CompleteRequests().
       CompleteRequests(OK, AddressList(), base::TimeDelta());
     }
@@ skipping 714 matching lines @@
   // Existing jobs will have been sent to the original server so they need to
   // be aborted.
   AbortAllInProgressJobs();
   // |this| may be deleted inside AbortAllInProgressJobs().
 }
 
 void HostResolverImpl::OnDnsConfigChanged(const DnsConfig& dns_config) {
   if (net_log_) {
     net_log_->AddGlobalEntry(
         NetLog::TYPE_DNS_CONFIG_CHANGED,
-        make_scoped_refptr(new DnsConfigParameters(dns_config)));
+        base::Bind(&NetLogDnsConfigCallback, &dns_config));
   }
 
   // TODO(szym): Remove once http://crbug.com/125599 is resolved.
   received_dns_config_ = dns_config.IsValid();
 
   // Life check to bail once |this| is deleted.
   base::WeakPtr<HostResolverImpl> self = AsWeakPtr();
 
   if (dns_client_.get()) {
     // We want a new factory in place, before we Abort running Jobs, so that the
     // newly started jobs use the new factory.
     dns_client_->SetConfig(dns_config);
     OnDNSChanged(NetworkChangeNotifier::CHANGE_DNS_SETTINGS);
     // |this| may be deleted inside OnDNSChanged().
     if (self)
       TryServingAllJobsFromHosts();
   }
 }
 
 bool HostResolverImpl::HaveDnsConfig() const {
   return (dns_client_.get() != NULL) && (dns_client_->GetConfig() != NULL);
 }
 
 }  // namespace net