New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 648346 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Nov 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug

Blocking:
issue 629194



Sign in to add a comment

Expose per-request metrics information in BidirectionalStream

Project Member Reported by xunji...@chromium.org, Sep 19 2016

Issue description

Implement a GetLoadTimingInfo method in net::BidirectionalStream

 
Project Member

Comment 1 by bugdroid1@chromium.org, Sep 20 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1f85ba7bedef44c5de50c55b6f636f4173c7dbd6

commit 1f85ba7bedef44c5de50c55b6f636f4173c7dbd6
Author: xunjieli <xunjieli@chromium.org>
Date: Tue Sep 20 19:31:37 2016

Change CronetUrlRequestContext slightly to allow reporting of BidirectionalStream's RequestFinishedInfo

This CL changes CronetUrlRequestContext#reportFinished slightly to allow
BidirectionalStream to report RequestFinishedInfo as well.

BUG= 648346 

Review-Url: https://codereview.chromium.org/2359583002
Cr-Commit-Position: refs/heads/master@{#419840}

[modify] https://crrev.com/1f85ba7bedef44c5de50c55b6f636f4173c7dbd6/components/cronet/android/java/src/org/chromium/net/impl/CronetUrlRequest.java
[modify] https://crrev.com/1f85ba7bedef44c5de50c55b6f636f4173c7dbd6/components/cronet/android/java/src/org/chromium/net/impl/CronetUrlRequestContext.java

Project Member

Comment 2 by bugdroid1@chromium.org, Sep 20 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ba8de43245cf4da038b3a178b6908ad2404466be

commit ba8de43245cf4da038b3a178b6908ad2404466be
Author: xunjieli <xunjieli@chromium.org>
Date: Tue Sep 20 21:57:54 2016

Adds GetLoadTimingInfo to net::BidirectionalStreamImpl

This CL adds a virtual function to net::BidirectionalStreamImpl to
get LoadTimingInfo. There will be follow-up CLs to add the
implementation for H2 and QUIC separately.

This is a part of the effort to expose timing metrics to net
embedders.

BUG= 648346 

Review-Url: https://codereview.chromium.org/2349363002
Cr-Commit-Position: refs/heads/master@{#419864}

[modify] https://crrev.com/ba8de43245cf4da038b3a178b6908ad2404466be/net/http/bidirectional_stream_impl.h
[modify] https://crrev.com/ba8de43245cf4da038b3a178b6908ad2404466be/net/quic/chromium/bidirectional_stream_quic_impl.cc
[modify] https://crrev.com/ba8de43245cf4da038b3a178b6908ad2404466be/net/quic/chromium/bidirectional_stream_quic_impl.h
[modify] https://crrev.com/ba8de43245cf4da038b3a178b6908ad2404466be/net/spdy/bidirectional_stream_spdy_impl.cc
[modify] https://crrev.com/ba8de43245cf4da038b3a178b6908ad2404466be/net/spdy/bidirectional_stream_spdy_impl.h

Project Member

Comment 3 by bugdroid1@chromium.org, Sep 21 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6d0b944d77930676e355406f795d6f51fe49daec

commit 6d0b944d77930676e355406f795d6f51fe49daec
Author: xunjieli <xunjieli@chromium.org>
Date: Wed Sep 21 01:53:57 2016

Implements BidirectionalStreamSpdyImpl::GetLoadTimingInfo

This CL adds an implementation of GetLoadTimingInfo in
BidirectionalStreamSpdyImpl. This is a part of the effort to
expose timing metrics to net embedders.

BUG= 648346 

Review-Url: https://codereview.chromium.org/2356463002
Cr-Commit-Position: refs/heads/master@{#419938}

[modify] https://crrev.com/6d0b944d77930676e355406f795d6f51fe49daec/net/spdy/bidirectional_stream_spdy_impl.cc
[modify] https://crrev.com/6d0b944d77930676e355406f795d6f51fe49daec/net/spdy/bidirectional_stream_spdy_impl.h
[modify] https://crrev.com/6d0b944d77930676e355406f795d6f51fe49daec/net/spdy/bidirectional_stream_spdy_impl_unittest.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Sep 21 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/35cf8a18703ec277c344b70eb2f84623693d9179

commit 35cf8a18703ec277c344b70eb2f84623693d9179
Author: xunjieli <xunjieli@chromium.org>
Date: Wed Sep 21 01:56:32 2016

Implements BidirectionalStreamQuicImpl::GetLoadTimingInfo

This CL adds an implementation of GetLoadTimingInfo
in BidirectionalStreamQuicImpl. This is a part of the effort
to expose timing metrics to net embedders.

BUG= 648346 

Review-Url: https://codereview.chromium.org/2354653003
Cr-Commit-Position: refs/heads/master@{#419939}

[modify] https://crrev.com/35cf8a18703ec277c344b70eb2f84623693d9179/net/quic/chromium/bidirectional_stream_quic_impl.cc
[modify] https://crrev.com/35cf8a18703ec277c344b70eb2f84623693d9179/net/quic/chromium/bidirectional_stream_quic_impl.h
[modify] https://crrev.com/35cf8a18703ec277c344b70eb2f84623693d9179/net/quic/chromium/bidirectional_stream_quic_impl_unittest.cc

Summary: Expose per-request metrics information in BidirectionalStream (was: Implement GetLoadTimingInfo in net::BidirectionalStream)
Project Member

Comment 6 by bugdroid1@chromium.org, Sep 23 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/369d092c31f1bec8a61eb2802274bae38e29261a

commit 369d092c31f1bec8a61eb2802274bae38e29261a
Author: xunjieli <xunjieli@chromium.org>
Date: Fri Sep 23 18:45:06 2016

Add GetLoadTimingInfo to net::BidirectionalStream

This CL adds GetLoadTimingInfo to net::BidirectionalStream.
This is a part of the effort to expose timing metrics to
net embedders.

This CL additionally moves |start_time| to BidirectionalStream's
constructor so we measure the true request start time (before
dns, ssl)

BUG= 648346 

Review-Url: https://codereview.chromium.org/2359493003
Cr-Commit-Position: refs/heads/master@{#420676}

[modify] https://crrev.com/369d092c31f1bec8a61eb2802274bae38e29261a/net/http/bidirectional_stream.cc
[modify] https://crrev.com/369d092c31f1bec8a61eb2802274bae38e29261a/net/http/bidirectional_stream.h
[modify] https://crrev.com/369d092c31f1bec8a61eb2802274bae38e29261a/net/http/bidirectional_stream_unittest.cc

Labels: -Pri-2 OS-Android Pri-1
Owner: mge...@chromium.org
After crrev.com/2360813003, there are a couple of things that need follow-up. 
- Passing the correct FinishedReason when constructing RequestFinishedInfo
- Passing the correct Exception if any when constructing RequestFinishedInfo

I might be missing some others. I think we need these in M55 as well. I can  help with the merges if needed. Handing this off to Miriam who is working on the brand new Metrics API.
Project Member

Comment 8 by bugdroid1@chromium.org, Oct 6 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/abea839761daacc8c926ed6230c14c2a14d4e1c0

commit abea839761daacc8c926ed6230c14c2a14d4e1c0
Author: xunjieli <xunjieli@chromium.org>
Date: Thu Oct 06 18:43:26 2016

[Cronet] Pass metrics information from C++ BidirectionalStream to Java

This CL adds plumbing to pass metrics information from C++
BidirectionalStream to its Java counterpart.

BUG= 648346 

Review-Url: https://codereview.chromium.org/2360813003
Cr-Commit-Position: refs/heads/master@{#423603}

[modify] https://crrev.com/abea839761daacc8c926ed6230c14c2a14d4e1c0/components/cronet/android/BUILD.gn
[modify] https://crrev.com/abea839761daacc8c926ed6230c14c2a14d4e1c0/components/cronet/android/cronet_bidirectional_stream_adapter.cc
[modify] https://crrev.com/abea839761daacc8c926ed6230c14c2a14d4e1c0/components/cronet/android/cronet_bidirectional_stream_adapter.h
[modify] https://crrev.com/abea839761daacc8c926ed6230c14c2a14d4e1c0/components/cronet/android/cronet_url_request_adapter.cc
[modify] https://crrev.com/abea839761daacc8c926ed6230c14c2a14d4e1c0/components/cronet/android/java/src/org/chromium/net/impl/CronetBidirectionalStream.java
[add] https://crrev.com/abea839761daacc8c926ed6230c14c2a14d4e1c0/components/cronet/android/metrics_util.cc
[add] https://crrev.com/abea839761daacc8c926ed6230c14c2a14d4e1c0/components/cronet/android/metrics_util.h
[modify] https://crrev.com/abea839761daacc8c926ed6230c14c2a14d4e1c0/components/cronet/android/test/javatests/src/org/chromium/net/BidirectionalStreamQuicTest.java
[modify] https://crrev.com/abea839761daacc8c926ed6230c14c2a14d4e1c0/components/cronet/android/test/javatests/src/org/chromium/net/BidirectionalStreamTest.java
[modify] https://crrev.com/abea839761daacc8c926ed6230c14c2a14d4e1c0/components/cronet/android/test/javatests/src/org/chromium/net/MetricsTestUtil.java

Project Member

Comment 9 by bugdroid1@chromium.org, Oct 27 2016

Labels: merge-merged-2840
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/abea839761daacc8c926ed6230c14c2a14d4e1c0

commit abea839761daacc8c926ed6230c14c2a14d4e1c0
Author: xunjieli <xunjieli@chromium.org>
Date: Thu Oct 06 18:43:26 2016

[Cronet] Pass metrics information from C++ BidirectionalStream to Java

This CL adds plumbing to pass metrics information from C++
BidirectionalStream to its Java counterpart.

BUG= 648346 

Review-Url: https://codereview.chromium.org/2360813003
Cr-Commit-Position: refs/heads/master@{#423603}

[modify] https://crrev.com/abea839761daacc8c926ed6230c14c2a14d4e1c0/components/cronet/android/BUILD.gn
[modify] https://crrev.com/abea839761daacc8c926ed6230c14c2a14d4e1c0/components/cronet/android/cronet_bidirectional_stream_adapter.cc
[modify] https://crrev.com/abea839761daacc8c926ed6230c14c2a14d4e1c0/components/cronet/android/cronet_bidirectional_stream_adapter.h
[modify] https://crrev.com/abea839761daacc8c926ed6230c14c2a14d4e1c0/components/cronet/android/cronet_url_request_adapter.cc
[modify] https://crrev.com/abea839761daacc8c926ed6230c14c2a14d4e1c0/components/cronet/android/java/src/org/chromium/net/impl/CronetBidirectionalStream.java
[add] https://crrev.com/abea839761daacc8c926ed6230c14c2a14d4e1c0/components/cronet/android/metrics_util.cc
[add] https://crrev.com/abea839761daacc8c926ed6230c14c2a14d4e1c0/components/cronet/android/metrics_util.h
[modify] https://crrev.com/abea839761daacc8c926ed6230c14c2a14d4e1c0/components/cronet/android/test/javatests/src/org/chromium/net/BidirectionalStreamQuicTest.java
[modify] https://crrev.com/abea839761daacc8c926ed6230c14c2a14d4e1c0/components/cronet/android/test/javatests/src/org/chromium/net/BidirectionalStreamTest.java
[modify] https://crrev.com/abea839761daacc8c926ed6230c14c2a14d4e1c0/components/cronet/android/test/javatests/src/org/chromium/net/MetricsTestUtil.java

Comment 10 by dimu@google.com, Nov 4 2016

Labels: -merge-merged-2840
[Automated comment] removing mislabelled merge-merged-2840
Status: Fixed (was: Started)

Sign in to add a comment