Move HostManager::RequestImpl NetLog events to RequestImpl itself.

Also correctly populate log entry fields that were not being correctly
populated, and rename one that now has a different meaning.

Bug: 997049
Change-Id: I646a8e837b2aca6631c701fba261d1ea149b19f3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1893733Reviewed-by: Eric Orth <ericorth@chromium.org>
Commit-Queue: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#711430}

net/dns/host_resolver_manager.cc

@@ -312,19 +312,6 @@ base::Value NetLogDnsTaskFailedParams(const HostCache::Entry& results,
   return std::move(dict);
 }
 
-// Creates NetLog parameters containing the information of the request. Use
-// NetLogRequestInfoCallback if the request is specified via RequestInfo.
-base::Value NetLogRequestParams(const HostPortPair& host) {
-  base::DictionaryValue dict;
-
-  dict.SetString("host", host.ToString());
-  dict.SetInteger("address_family",
-                  static_cast<int>(ADDRESS_FAMILY_UNSPECIFIED));
-  dict.SetBoolean("allow_cached_response", true);
-  dict.SetBoolean("is_speculative", false);
-  return std::move(dict);
-}
-
 // Creates NetLog parameters for the creation of a HostResolverManager::Job.
 base::Value NetLogJobCreationParams(const NetLogSource& source,
                                     const std::string& host) {
@@ -353,26 +340,6 @@ base::Value NetLogIPv6AvailableParams(bool ipv6_available, bool cached) {
 // The logging routines are defined here because some requests are resolved
 // without a Request object.
 
-// Logs when a request has just been started. Overloads for whether or not the
-// request information is specified via a RequestInfo object.
-void LogStartRequest(const NetLogWithSource& source_net_log,
-                     const HostPortPair& host) {
-  source_net_log.BeginEvent(NetLogEventType::HOST_RESOLVER_IMPL_REQUEST,
-                            [&] { return NetLogRequestParams(host); });
-}
-
-// Logs when a request has just completed (before its callback is run).
-void LogFinishRequest(const NetLogWithSource& source_net_log, int net_error) {
-  source_net_log.EndEventWithNetErrorCode(
-      NetLogEventType::HOST_RESOLVER_IMPL_REQUEST, net_error);
-}
-
-// Logs when a request has been cancelled.
-void LogCancelRequest(const NetLogWithSource& source_net_log) {
-  source_net_log.AddEvent(NetLogEventType::CANCELLED);
-  source_net_log.EndEvent(NetLogEventType::HOST_RESOLVER_IMPL_REQUEST);
-}
-
 //-----------------------------------------------------------------------------
 
 // Maximum of 6 concurrent resolver threads (excluding retries).
@@ -555,6 +522,7 @@ class HostResolverManager::RequestImpl
     // Parent HostResolver must still be alive to call Start().
     DCHECK(resolver_);
 
+    LogStartRequest();
     int rv = resolver_->Resolve(this);
     DCHECK(!complete_);
     if (rv == ERR_IO_PENDING) {
@@ -563,6 +531,7 @@ class HostResolverManager::RequestImpl
     } else {
       DCHECK(!job_);
       complete_ = true;
+      LogFinishRequest(rv);
     }
     resolver_ = nullptr;
 
@@ -636,6 +605,8 @@ class HostResolverManager::RequestImpl
 
     // No results should be set.
     DCHECK(!results_);
+
+    LogCancelRequest();
   }
 
   // Cleans up Job assignment, marks request completed, and calls the completion
@@ -647,6 +618,8 @@ class HostResolverManager::RequestImpl
     DCHECK(!complete_);
     complete_ = true;
 
+    LogFinishRequest(error);
+
     DCHECK(callback_);
     std::move(callback_).Run(error);
   }
@@ -682,6 +655,34 @@ class HostResolverManager::RequestImpl
   }
 
  private:
+  // Logs when a request has just been started.
+  void LogStartRequest() {
+    source_net_log_.BeginEvent(
+        NetLogEventType::HOST_RESOLVER_IMPL_REQUEST, [this] {
+          base::Value dict(base::Value::Type::DICTIONARY);
+          dict.SetStringKey("host", request_host_.ToString());
+          dict.SetIntKey("dns_query_type",
+                         static_cast<int>(parameters_.dns_query_type));
+          dict.SetBoolKey("allow_cached_response",
+                          parameters_.cache_usage !=
+                              ResolveHostParameters::CacheUsage::DISALLOWED);
+          dict.SetBoolKey("is_speculative", parameters_.is_speculative);
+          return dict;
+        });
+  }
+
+  // Logs when a request has just completed (before its callback is run).
+  void LogFinishRequest(int net_error) {
+    source_net_log_.EndEventWithNetErrorCode(
+        NetLogEventType::HOST_RESOLVER_IMPL_REQUEST, net_error);
+  }
+
+  // Logs when a request has been cancelled.
+  void LogCancelRequest() {
+    source_net_log_.AddEvent(NetLogEventType::CANCELLED);
+    source_net_log_.EndEvent(NetLogEventType::HOST_RESOLVER_IMPL_REQUEST);
+  }
+
   const NetLogWithSource source_net_log_;
 
   const HostPortPair request_host_;
@@ -1523,7 +1524,6 @@ class HostResolverManager::Job : public PrioritizedDispatcher::Job,
       RequestImpl* req = requests_.head()->value();
       req->RemoveFromList();
       DCHECK_EQ(this, req->job());
-      LogCancelRequest(req->source_net_log());
       req->OnJobCancelled(this);
     }
   }
@@ -1591,8 +1591,6 @@ class HostResolverManager::Job : public PrioritizedDispatcher::Job,
     DCHECK_EQ(hostname_, request->request_host().host());
     DCHECK(!requests_.empty());
 
-    LogCancelRequest(request->source_net_log());
-
     priority_tracker_.Remove(request->priority());
     net_log_.AddEvent(NetLogEventType::HOST_RESOLVER_IMPL_JOB_REQUEST_DETACH,
                       [&] {
@@ -2283,7 +2281,6 @@ class HostResolverManager::Job : public PrioritizedDispatcher::Job,
       req->RemoveFromList();
       DCHECK_EQ(this, req->job());
       // Update the net log and notify registered observers.
-      LogFinishRequest(req->source_net_log(), results.error());
       if (results.did_complete()) {
         // Record effective total time from creation to completion.
         resolver_->RecordTotalTime(
@@ -2665,8 +2662,6 @@ int HostResolverManager::Resolve(RequestImpl* request) {
 
   request->set_request_time(tick_clock_->NowTicks());
 
-  LogStartRequest(request->source_net_log(), request->request_host());
-
   DnsQueryType effective_query_type;
   HostResolverFlags effective_host_resolver_flags;
   DnsConfig::SecureDnsMode effective_secure_dns_mode;
@@ -2689,7 +2684,6 @@ int HostResolverManager::Resolve(RequestImpl* request) {
     }
     if (stale_info && !request->parameters().is_speculative)
       request->set_stale_info(std::move(stale_info).value());
-    LogFinishRequest(request->source_net_log(), results.error());
     RecordTotalTime(request->parameters().is_speculative, true /* from_cache */,
                     effective_secure_dns_mode, base::TimeDelta());
     return results.error();
@@ -3513,6 +3507,8 @@ void HostResolverManager::RequestImpl::Cancel() {
   job_->CancelRequest(this);
   job_ = nullptr;
   callback_.Reset();
+
+  LogCancelRequest();
 }
 
 void HostResolverManager::RequestImpl::ChangeRequestPriority(

net/log/net_log_event_type_list.h

@@ -48,7 +48,7 @@ EVENT_TYPE(REQUEST_ALIVE)
 //
 //   {
 //     "host": <Hostname associated with the request>,
-//     "address_family": <The address family to restrict results to>,
+//     "dns_query_type": <The type of the DNS query>,
 //     "allow_cached_response": <Whether it is ok to return a result from
 //                               the host cache>,
 //     "is_speculative": <Whether this request was started by the DNS