Log BIDIRECTIONL_STREAM_ALIVE before HTTP_STREAM_REQUEST

Log BIDIRECTIONL_STREAM_ALIVE before HTTP_STREAM_REQUEST

This CL logs BIDIRECTIONL_STREAM_ALIVE before we start any stream request.
This makes sure that HTTP_STREAM_REQUEST is nested inside
BIDIRECTIONL_STREAM_ALIVE begin and end events.

BUG=516342
Committed: https://crrev.com/f88028e345674e0e76677164dd415b4dee886590
Cr-Commit-Position: refs/heads/master@{#397205}

[xunjieli, 2016-05-26 21:08:15]

Description was changed from

==========
Log BIDIRECTIONL_STREAM_ALIVE before HTTP_STREAM_REQUEST

This CL logs BIDIRECTIONL_STREAM_ALIVE before we starts any stream request.
This makes sure that HTTP_STREAM_REQUEST is nested inside
BIDIRECTIONL_STREAM_ALIVE begin and end events.
==========

to

==========
Log BIDIRECTIONL_STREAM_ALIVE before HTTP_STREAM_REQUEST

This CL logs BIDIRECTIONL_STREAM_ALIVE before we start any stream request.
This makes sure that HTTP_STREAM_REQUEST is nested inside
BIDIRECTIONL_STREAM_ALIVE begin and end events.
==========

[xunjieli, 2016-05-26 21:08:55]

xunjieli@chromium.org changed reviewers:
+ mef@chromium.org

[xunjieli, 2016-05-26 21:08:56]

Misha, PTAL. Thank you!

[xunjieli, 2016-05-26 21:09:54]

Description was changed from

==========
Log BIDIRECTIONL_STREAM_ALIVE before HTTP_STREAM_REQUEST

This CL logs BIDIRECTIONL_STREAM_ALIVE before we start any stream request.
This makes sure that HTTP_STREAM_REQUEST is nested inside
BIDIRECTIONL_STREAM_ALIVE begin and end events.
==========

to

==========
Log BIDIRECTIONL_STREAM_ALIVE before HTTP_STREAM_REQUEST

This CL logs BIDIRECTIONL_STREAM_ALIVE before we start any stream request.
This makes sure that HTTP_STREAM_REQUEST is nested inside
BIDIRECTIONL_STREAM_ALIVE begin and end events.

BUG=516342
==========

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

https://codereview.chromium.org/2016863002/diff/20001/net/http/bidirectional_...
File net/http/bidirectional_stream.cc (right):

https://codereview.chromium.org/2016863002/diff/20001/net/http/bidirectional_...
net/http/bidirectional_stream.cc:93: if (net_log_.IsCapturing()) {
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]

https://codereview.chromium.org/2016863002/diff/20001/net/http/bidirectional_...
File net/http/bidirectional_stream.cc (right):

https://codereview.chromium.org/2016863002/diff/20001/net/http/bidirectional_...
net/http/bidirectional_stream.cc:93: if (net_log_.IsCapturing()) {
On 2016/06/01 17:25:40, mef wrote:
> Should this go down below the failure exit if not Https? Also, do we need a
test
> for that?

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]

lgtm

https://codereview.chromium.org/2016863002/diff/20001/net/http/bidirectional_...
File net/http/bidirectional_stream.cc (right):

https://codereview.chromium.org/2016863002/diff/20001/net/http/bidirectional_...
net/http/bidirectional_stream.cc:93: if (net_log_.IsCapturing()) {
On 2016/06/01 17:38:45, xunjieli wrote:
> On 2016/06/01 17:25:40, mef wrote:
> > Should this go down below the failure exit if not Https? Also, do we need a
> test
> > for that?
> 
> 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.

sgtm, thanks for explanation!

[xunjieli, 2016-06-01 17:46:34]

The CQ bit was checked by xunjieli@chromium.org

[commit-bot: I haz the power, 2016-06-01 17:47:10]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2016863002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2016863002/20001

[commit-bot: I haz the power, 2016-06-01 19:27:32]

Description was changed from

==========
Log BIDIRECTIONL_STREAM_ALIVE before HTTP_STREAM_REQUEST

This CL logs BIDIRECTIONL_STREAM_ALIVE before we start any stream request.
This makes sure that HTTP_STREAM_REQUEST is nested inside
BIDIRECTIONL_STREAM_ALIVE begin and end events.

BUG=516342
==========

to

==========
Log BIDIRECTIONL_STREAM_ALIVE before HTTP_STREAM_REQUEST

This CL logs BIDIRECTIONL_STREAM_ALIVE before we start any stream request.
This makes sure that HTTP_STREAM_REQUEST is nested inside
BIDIRECTIONL_STREAM_ALIVE begin and end events.

BUG=516342
==========

[commit-bot: I haz the power, 2016-06-01 19:27:33]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-06-01 19:29:52]

Description was changed from

==========
Log BIDIRECTIONL_STREAM_ALIVE before HTTP_STREAM_REQUEST

This CL logs BIDIRECTIONL_STREAM_ALIVE before we start any stream request.
This makes sure that HTTP_STREAM_REQUEST is nested inside
BIDIRECTIONL_STREAM_ALIVE begin and end events.

BUG=516342
==========

to

==========
Log BIDIRECTIONL_STREAM_ALIVE before HTTP_STREAM_REQUEST

This CL logs BIDIRECTIONL_STREAM_ALIVE before we start any stream request.
This makes sure that HTTP_STREAM_REQUEST is nested inside
BIDIRECTIONL_STREAM_ALIVE begin and end events.

BUG=516342

Committed: https://crrev.com/f88028e345674e0e76677164dd415b4dee886590
Cr-Commit-Position: refs/heads/master@{#397205}
==========

[commit-bot: I haz the power, 2016-06-01 19:29:52]

Patchset 2 (id:??) landed as
https://crrev.com/f88028e345674e0e76677164dd415b4dee886590
Cr-Commit-Position: refs/heads/master@{#397205}

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

https://codereview.chromium.org/2016863002/diff/20001/net/http/bidirectional_...
File net/http/bidirectional_stream.cc (right):

https://codereview.chromium.org/2016863002/diff/20001/net/http/bidirectional_...
net/http/bidirectional_stream.cc:93: if (net_log_.IsCapturing()) {
On 2016/06/01 17:45:33, mef wrote:
> On 2016/06/01 17:38:45, xunjieli wrote:
> > On 2016/06/01 17:25:40, mef wrote:
> > > Should this go down below the failure exit if not Https? Also, do we need
a
> > test
> > > for that?
> > 
> > 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.
> 
> sgtm, thanks for explanation!

thanks for the review!