Log BIDIRECTIONL_STREAM_ALIVE before HTTP_STREAM_REQUEST

net/http/bidirectional_stream.cc

 // Copyright 2015 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/bidirectional_stream.h"
 
 #include <memory>
 #include <string>
 
 #include "base/bind.h"
@@ skipping 72 matching lines @@
     : request_info_(std::move(request_info)),
       net_log_(BoundNetLog::Make(session->net_log(),
                                  NetLog::SOURCE_BIDIRECTIONAL_STREAM)),
       session_(session),
       disable_auto_flush_(disable_auto_flush),
       delegate_(delegate),
       timer_(std::move(timer)) {
   DCHECK(delegate_);
   DCHECK(request_info_);
 
+  if (net_log_.IsCapturing()) {

[mef, 2016/06/01 17:25:40]

Should this go down below the failure exit if not Https? Also, do we need a test
for that?

[xunjieli, 2016/06/01 17:38:45]

I think it should be before the failure exit. In case we have the disallowed
schemes, we have logs for it too. When we enter the failure exit, we still
create the BidirectionalStream object, so I think it makes sense to log the
event.

We do have a test for the early exit. It is in
BidirectionalStreamTest.CreateInsecureStream.

[mef, 2016/06/01 17:45:33]

sgtm, thanks for explanation!

[xunjieli, 2016/06/01 22:23:28]

thanks for the review!

+    net_log_.BeginEvent(
+        NetLog::TYPE_BIDIRECTIONAL_STREAM_ALIVE,
+        base::Bind(&NetLogCallback, &request_info_->url, &request_info_->method,
+                   base::Unretained(&request_info_->extra_headers)));
+  }
+
   SSLConfig server_ssl_config;
   session->ssl_config_service()->GetSSLConfig(&server_ssl_config);
   session->GetAlpnProtos(&server_ssl_config.alpn_protos);
   session->GetNpnProtos(&server_ssl_config.npn_protos);
 
   if (!request_info_->url.SchemeIs(url::kHttpsScheme)) {
     base::ThreadTaskRunnerHandle::Get()->PostTask(
         FROM_HERE,
         base::Bind(&BidirectionalStream::Delegate::OnFailed,
                    base::Unretained(delegate_), ERR_DISALLOWED_URL_SCHEME));
     return;
   }
 
   HttpRequestInfo http_request_info;
   http_request_info.url = request_info_->url;
   http_request_info.method = request_info_->method;
   http_request_info.extra_headers = request_info_->extra_headers;
   stream_request_.reset(
       session->http_stream_factory()->RequestBidirectionalStreamImpl(
           http_request_info, request_info_->priority, server_ssl_config,
           server_ssl_config, this, net_log_));
   // Check that this call cannot fail to set a non-NULL |stream_request_|.
   DCHECK(stream_request_);
   // Check that HttpStreamFactory does not invoke OnBidirectionalStreamImplReady
   // synchronously.
   DCHECK(!stream_impl_);
-  if (net_log_.IsCapturing()) {
-    net_log_.BeginEvent(
-        NetLog::TYPE_BIDIRECTIONAL_STREAM_ALIVE,
-        base::Bind(&NetLogCallback, &request_info_->url, &request_info_->method,
-                   base::Unretained(&request_info_->extra_headers)));
-  }
 }
 
 BidirectionalStream::~BidirectionalStream() {
   Cancel();
   if (net_log_.IsCapturing()) {
     net_log_.EndEvent(NetLog::TYPE_BIDIRECTIONAL_STREAM_ALIVE);
   }
 }
 
 int BidirectionalStream::ReadData(IOBuffer* buf, int buf_len) {
@@ skipping 197 matching lines @@
     const ProxyInfo& used_proxy_info,
     HttpStream* stream) {
   DCHECK(stream_request_);
 
   delegate_->OnFailed(ERR_HTTPS_PROXY_TUNNEL_RESPONSE);
 }
 
 void BidirectionalStream::OnQuicBroken() {}
 
 }  // namespace net