Improve HttpStreamFactory NetLog events

net/http/http_stream_factory_impl_job.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/http/http_stream_factory_impl_job.h"
 
 #include <algorithm>
 #include <string>
 
 #include "base/bind.h"
@@ skipping 668 matching lines @@
   CHECK_EQ(STATE_NONE, next_state_);
   next_state_ = STATE_START;
   int rv = RunLoop(OK);
   DCHECK_EQ(ERR_IO_PENDING, rv);
   return rv;
 }
 
 int HttpStreamFactoryImpl::Job::DoStart() {
   const NetLogWithSource* net_log = delegate_->GetNetLog(this);
 
-  if (net_log) {

[eroman, 2017/01/25 19:55:26]

It isn't clear why this removal is safe when reading this line. From an
interface perspective, GetNetLog() can return nullptr.

It may be that in practice based on how the pieces are connected (does this
delegate come from JobController, which now returns non-null?) this is never
null, but that coupling between implementations is not obvious.

I suggest keeping the null-check, even if in practice the concrete delegate
implementation won't hit it.

[xunjieli, 2017/01/25 21:41:48]

Done.

-    net_log_.BeginEvent(
-        NetLogEventType::HTTP_STREAM_JOB,
-        base::Bind(&NetLogHttpStreamJobCallback, net_log->source(),
-                   &request_info_.url, &origin_url_, &alternative_service_,
-                   priority_));
-    net_log->AddEvent(NetLogEventType::HTTP_STREAM_REQUEST_STARTED_JOB,
-                      net_log_.source().ToEventParametersCallback());
-  }
+  net_log_.BeginEvent(
+      NetLogEventType::HTTP_STREAM_JOB,
+      base::Bind(&NetLogHttpStreamJobCallback, net_log->source(),
+                 &request_info_.url, &origin_url_, &alternative_service_,
+                 priority_));
+  net_log->AddEvent(NetLogEventType::HTTP_STREAM_REQUEST_STARTED_JOB,
+                    net_log_.source().ToEventParametersCallback());
 
   // Don't connect to restricted ports.
   if (!IsPortAllowedForScheme(destination_.port(),
                               request_info_.url.scheme())) {
     return ERR_UNSAFE_PORT;
   }
 
   next_state_ = STATE_RESOLVE_PROXY;
   return OK;
 }
@@ skipping 798 matching lines @@
 
   ConnectionAttempts socket_attempts = connection_->connection_attempts();
   if (connection_->socket()) {
     connection_->socket()->GetConnectionAttempts(&socket_attempts);
   }
 
   delegate_->AddConnectionAttemptsToRequest(this, socket_attempts);
 }
 
 }  // namespace net