Add new Cronet exception class for QUIC errors

Add new Cronet exception class for QUIC errors

Create a subclass of CronetException, called QuicException, which contains the
detailed QUIC error code when it exists.
Propagate the error code from NetErrorDetails through Cronet. For now,
only UrlRequest reports the detailed error, not BidirectionalStream.

BUG=614507
Committed: https://crrev.com/a8228bdbdd1840d925baf93067057daccae1e60a
Cr-Commit-Position: refs/heads/master@{#403525}

[mgersh, 2016-06-15 23:46:31]

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

[mgersh, 2016-06-15 23:46:32]

PTAL.

Particular question: I changed the signature for CronetException, because it
doesn't look like there are any existing uses to break, but there are for
UrlRequestException, so I added another constructor instead of replacing. Do you
think this is a good way to do it?

[xunjieli, 2016-06-16 12:56:47]

On 2016/06/15 23:46:32, mgersh wrote:
> PTAL.
> 
> Particular question: I changed the signature for CronetException, because it
> doesn't look like there are any existing uses to break, but there are for
> UrlRequestException, so I added another constructor instead of replacing. Do
you
> think this is a good way to do it?

Hi Miriam, I am on bug triage duty today. I probably won't be able to look at
this until tomorrow. The last person who touched this code is Paul, who might be
able to review, if you'd like to get this in before the weekend.

[mgersh, 2016-06-16 15:19:35]

mgersh@chromium.org changed reviewers:
+ pauljensen@chromium.org

[mgersh, 2016-06-16 15:19:36]

Okay, thanks for telling me. Paul, PTAL.

[xunjieli, 2016-06-16 21:35:43]

Rather than exposing Quic internal error as an int, maybe we can create a Class
that is QUIC specific? Something like QuicError. We can create an instance of
that class and put in UrlRequestException/CronetException when protocol
negotiated is Quic. That way, it will be very clear that this error code is Quic
specific. WDYT?

We won't have too many objects flying around, since we only instantiate one
extra when there is an exception and when QUIC is negotiated.

[mgersh, 2016-06-16 21:45:24]

On 2016/06/16 21:35:43, xunjieli wrote:
> Rather than exposing Quic internal error as an int, maybe we can create a
Class
> that is QUIC specific? Something like QuicError. We can create an instance of
> that class and put in UrlRequestException/CronetException when protocol
> negotiated is Quic. That way, it will be very clear that this error code is
Quic
> specific. WDYT?
> 
> We won't have too many objects flying around, since we only instantiate one
> extra when there is an exception and when QUIC is negotiated.

On the bug, Misha suggested leaving the API fairly general so we can add more to
it in the future. Instead of an int, I could make a class with a general name
which has a field with a QUIC-specific name. Does that sound reasonable?

[pauljensen, 2016-06-17 12:05:59]

I don't like the idea of adding a function that is destined to be overloaded for
various purposes.  I don't think we're so space constrained we need to start
overloading methods.

I like the idea of creating a new "class QuicException extends IOException" that
has just one method, "int getQuicInternalErrorCode()" and setting this as the
"cause" of the UrlRequestException when getCronetInternalErrorCode() returns
QUIC_PROTOCOL_ERROR.

https://codereview.chromium.org/2069303002/diff/1/components/cronet/android/c...
File components/cronet/android/cronet_bidirectional_stream_adapter.cc (right):

https://codereview.chromium.org/2069303002/diff/1/components/cronet/android/c...
components/cronet/android/cronet_bidirectional_stream_adapter.cc:320: env,
owner_.obj(), NetErrorToUrlRequestError(error), error, 0,
0 -> QUIC_NO_ERROR

https://codereview.chromium.org/2069303002/diff/1/net/test/url_request/url_re...
File net/test/url_request/url_request_failed_job.cc (right):

https://codereview.chromium.org/2069303002/diff/1/net/test/url_request/url_re...
net/test/url_request/url_request_failed_job.cc:129:
details->quic_connection_error = QUIC_INTERNAL_ERROR;
I don't think we should set |quic_connection_error| for all errors, that seems
somewhat incorrect, for example for non-QUIC requests.  How about adding "if
(net_error_ == ERR_QUIC_PROTOCOL_ERROR)"?

[mgersh, 2016-06-29 23:00:23]

Description was changed from

==========
Add QUIC error code to Cronet's UrlRequestException

Propagate QUIC error code from NetErrorDetails through Cronet. For now,
only UrlRequest reports the detailed error, not BidirectionalStream.

BUG=614507
==========

to

==========
Add new Cronet exception class for QUIC errors

Create a subclass of CronetException, called QuicException, which contains the
detailed QUIC error code when it exists.
Propagate the error code from NetErrorDetails through Cronet. For now,
only UrlRequest reports the detailed error, not BidirectionalStream.

BUG=614507
==========

[mgersh, 2016-06-29 23:01:09]

PTAL at new version

https://codereview.chromium.org/2069303002/diff/1/components/cronet/android/c...
File components/cronet/android/cronet_bidirectional_stream_adapter.cc (right):

https://codereview.chromium.org/2069303002/diff/1/components/cronet/android/c...
components/cronet/android/cronet_bidirectional_stream_adapter.cc:320: env,
owner_.obj(), NetErrorToUrlRequestError(error), error, 0,
On 2016/06/17 12:05:59, pauljensen wrote:
> 0 -> QUIC_NO_ERROR

Done (here and in the next file)

https://codereview.chromium.org/2069303002/diff/1/net/test/url_request/url_re...
File net/test/url_request/url_request_failed_job.cc (right):

https://codereview.chromium.org/2069303002/diff/1/net/test/url_request/url_re...
net/test/url_request/url_request_failed_job.cc:129:
details->quic_connection_error = QUIC_INTERNAL_ERROR;
On 2016/06/17 12:05:59, pauljensen wrote:
> I don't think we should set |quic_connection_error| for all errors, that seems
> somewhat incorrect, for example for non-QUIC requests.  How about adding "if
> (net_error_ == ERR_QUIC_PROTOCOL_ERROR)"?

Done.

[xunjieli, 2016-06-30 14:13:44]

The new approach lgtm. One suggestion below to get rid of QUIC_PROTOCOL_ERROR
and use the one from NetError.java directly.

https://codereview.chromium.org/2069303002/diff/20001/components/cronet/andro...
File components/cronet/android/api/src/org/chromium/net/UrlRequestException.java
(right):

https://codereview.chromium.org/2069303002/diff/20001/components/cronet/andro...
components/cronet/android/api/src/org/chromium/net/UrlRequestException.java:79:
public static final int QUIC_PROTOCOL_ERROR = -356;
We can just reuse NetError.ERR_QUIC_PROTOCOL_ERROR.

https://codereview.chromium.org/2069303002/diff/20001/components/cronet/andro...
File
components/cronet/android/java/src/org/chromium/net/CronetBidirectionalStream.java
(right):

https://codereview.chromium.org/2069303002/diff/20001/components/cronet/andro...
components/cronet/android/java/src/org/chromium/net/CronetBidirectionalStream.java:564:
if (nativeError == UrlRequestException.QUIC_PROTOCOL_ERROR) {
Can reuse NetError.ERR_QUIC_PROTOCOL_ERROR.

[mgersh, 2016-06-30 16:50:53]

https://codereview.chromium.org/2069303002/diff/20001/components/cronet/andro...
File components/cronet/android/api/src/org/chromium/net/UrlRequestException.java
(right):

https://codereview.chromium.org/2069303002/diff/20001/components/cronet/andro...
components/cronet/android/api/src/org/chromium/net/UrlRequestException.java:79:
public static final int QUIC_PROTOCOL_ERROR = -356;
On 2016/06/30 14:13:43, xunjieli wrote:
> We can just reuse NetError.ERR_QUIC_PROTOCOL_ERROR.

Done.

https://codereview.chromium.org/2069303002/diff/20001/components/cronet/andro...
File
components/cronet/android/java/src/org/chromium/net/CronetBidirectionalStream.java
(right):

https://codereview.chromium.org/2069303002/diff/20001/components/cronet/andro...
components/cronet/android/java/src/org/chromium/net/CronetBidirectionalStream.java:564:
if (nativeError == UrlRequestException.QUIC_PROTOCOL_ERROR) {
On 2016/06/30 14:13:43, xunjieli wrote:
> Can reuse NetError.ERR_QUIC_PROTOCOL_ERROR.

Done.

[pauljensen, 2016-06-30 17:35:13]

I like this new solution.  Thanks for putting it together.

https://codereview.chromium.org/2069303002/diff/40001/components/cronet/andro...
File components/cronet/android/api/src/org/chromium/net/QuicException.java
(right):

https://codereview.chromium.org/2069303002/diff/40001/components/cronet/andro...
components/cronet/android/api/src/org/chromium/net/QuicException.java:9: *
detailed QUIC error code from
I'd recommend linkifying QUIC here and on line 37 like we've done elsewhere in
Cronet javadocs.

https://codereview.chromium.org/2069303002/diff/40001/components/cronet/andro...
File components/cronet/android/api/src/org/chromium/net/UrlRequestException.java
(right):

https://codereview.chromium.org/2069303002/diff/40001/components/cronet/andro...
components/cronet/android/api/src/org/chromium/net/UrlRequestException.java:18:
*     returns QUIC_PROTOCOL_ERROR, this exception can be cast to a {@link
QuicException}
It's quite awkward to reference a value here without an easy way of knowing how
to use that value. (users would have to know to click the
getCronetInternalErrorCode link and then click the link to the errors and then
grep for QUIC_PROTOCOL_ERROR and then define their own constant in Java for this
value)  I'd propose adding ERROR_QUIC_PROTOCOL_FAILED to this class.

https://codereview.chromium.org/2069303002/diff/40001/components/cronet/andro...
File components/cronet/android/cronet_bidirectional_stream_adapter.cc (right):

https://codereview.chromium.org/2069303002/diff/40001/components/cronet/andro...
components/cronet/android/cronet_bidirectional_stream_adapter.cc:319: //
TODO(mgersh): Add support for NetErrorDetails
probably a good idea to file a bug for this and include the http://crbug.com/123
link here.  if embedders are asking for this change, they'll probably be asking
for this next.

https://codereview.chromium.org/2069303002/diff/40001/components/cronet/andro...
File
components/cronet/android/java/src/org/chromium/net/CronetBidirectionalStream.java
(right):

https://codereview.chromium.org/2069303002/diff/40001/components/cronet/andro...
components/cronet/android/java/src/org/chromium/net/CronetBidirectionalStream.java:567:
// TODO(mgersh): plumb through the real QUIC error code
I'm not a fan of having a comment in one place talk about the impl in another
place.  These sorts of comments frequently are subject to code rot.  I'd prefer
removing these three lines.

https://codereview.chromium.org/2069303002/diff/40001/components/cronet/andro...
File components/cronet/android/java/src/org/chromium/net/CronetUrlRequest.java
(right):

https://codereview.chromium.org/2069303002/diff/40001/components/cronet/andro...
components/cronet/android/java/src/org/chromium/net/CronetUrlRequest.java:652:
requestError = new QuicException(
I don't think we need the requestError variable here or in bidistream.  We can
just do:
if (...) {
  failWithException(new ...);
} else {
  failWithException(new ...);
}

https://codereview.chromium.org/2069303002/diff/40001/components/cronet/andro...
File
components/cronet/android/test/javatests/src/org/chromium/net/CronetUrlRequestTest.java
(right):

https://codereview.chromium.org/2069303002/diff/40001/components/cronet/andro...
components/cronet/android/test/javatests/src/org/chromium/net/CronetUrlRequestTest.java:1793:
QuicException quicException = (QuicException) callback.mError;
might want to add before this line assertTrue(callback.mError instanceof
QuicException);

[mgersh, 2016-06-30 22:55:53]

https://codereview.chromium.org/2069303002/diff/40001/components/cronet/andro...
File components/cronet/android/api/src/org/chromium/net/QuicException.java
(right):

https://codereview.chromium.org/2069303002/diff/40001/components/cronet/andro...
components/cronet/android/api/src/org/chromium/net/QuicException.java:9: *
detailed QUIC error code from
On 2016/06/30 17:35:12, pauljensen wrote:
> I'd recommend linkifying QUIC here and on line 37 like we've done elsewhere in
> Cronet javadocs.

Done.

https://codereview.chromium.org/2069303002/diff/40001/components/cronet/andro...
File components/cronet/android/api/src/org/chromium/net/UrlRequestException.java
(right):

https://codereview.chromium.org/2069303002/diff/40001/components/cronet/andro...
components/cronet/android/api/src/org/chromium/net/UrlRequestException.java:18:
*     returns QUIC_PROTOCOL_ERROR, this exception can be cast to a {@link
QuicException}
On 2016/06/30 17:35:12, pauljensen wrote:
> It's quite awkward to reference a value here without an easy way of knowing
how
> to use that value. (users would have to know to click the
> getCronetInternalErrorCode link and then click the link to the errors and then
> grep for QUIC_PROTOCOL_ERROR and then define their own constant in Java for
this
> value)  I'd propose adding ERROR_QUIC_PROTOCOL_FAILED to this class.

Done. (Was the different name just to avoid saying "ERROR" twice?)

https://codereview.chromium.org/2069303002/diff/40001/components/cronet/andro...
File components/cronet/android/cronet_bidirectional_stream_adapter.cc (right):

https://codereview.chromium.org/2069303002/diff/40001/components/cronet/andro...
components/cronet/android/cronet_bidirectional_stream_adapter.cc:319: //
TODO(mgersh): Add support for NetErrorDetails
On 2016/06/30 17:35:12, pauljensen wrote:
> probably a good idea to file a bug for this and include the
http://crbug.com/123
> link here.  if embedders are asking for this change, they'll probably be
asking
> for this next.

Done.

https://codereview.chromium.org/2069303002/diff/40001/components/cronet/andro...
File
components/cronet/android/java/src/org/chromium/net/CronetBidirectionalStream.java
(right):

https://codereview.chromium.org/2069303002/diff/40001/components/cronet/andro...
components/cronet/android/java/src/org/chromium/net/CronetBidirectionalStream.java:567:
// TODO(mgersh): plumb through the real QUIC error code
On 2016/06/30 17:35:12, pauljensen wrote:
> I'm not a fan of having a comment in one place talk about the impl in another
> place.  These sorts of comments frequently are subject to code rot.  I'd
prefer
> removing these three lines.

Done.

https://codereview.chromium.org/2069303002/diff/40001/components/cronet/andro...
File components/cronet/android/java/src/org/chromium/net/CronetUrlRequest.java
(right):

https://codereview.chromium.org/2069303002/diff/40001/components/cronet/andro...
components/cronet/android/java/src/org/chromium/net/CronetUrlRequest.java:652:
requestError = new QuicException(
On 2016/06/30 17:35:12, pauljensen wrote:
> I don't think we need the requestError variable here or in bidistream.  We can
> just do:
> if (...) {
>   failWithException(new ...);
> } else {
>   failWithException(new ...);
> }

Done.

https://codereview.chromium.org/2069303002/diff/40001/components/cronet/andro...
File
components/cronet/android/test/javatests/src/org/chromium/net/CronetUrlRequestTest.java
(right):

https://codereview.chromium.org/2069303002/diff/40001/components/cronet/andro...
components/cronet/android/test/javatests/src/org/chromium/net/CronetUrlRequestTest.java:1793:
QuicException quicException = (QuicException) callback.mError;
On 2016/06/30 17:35:12, pauljensen wrote:
> might want to add before this line assertTrue(callback.mError instanceof
> QuicException);

Done.

[pauljensen, 2016-07-01 12:22:02]

https://codereview.chromium.org/2069303002/diff/60001/components/cronet/andro...
File components/cronet/android/api/src/org/chromium/net/QuicException.java
(right):

https://codereview.chromium.org/2069303002/diff/60001/components/cronet/andro...
components/cronet/android/api/src/org/chromium/net/QuicException.java:8: *
Subclass of {@link UrlRequestException} (through {@link CronetException}) which
contains a
I don't think we need to mention CronetException...hopefully we'll get rid of it
soon enough

https://codereview.chromium.org/2069303002/diff/60001/components/cronet/andro...
components/cronet/android/api/src/org/chromium/net/QuicException.java:10: * <a
href=https://chromium.googlesource.com/chromium/src/+/master/net/quic/quic_protocol.h#503>
Line 503 is already incorrect.
I was going to suggest including a git hash in the URL so the line number stays
accurate forever, but then embedders won't see new error codes.  How about this
URL:
https://cs.chromium.org/chromium/src/net/quic/quic_protocol.h?type=cs&q="enum...

https://codereview.chromium.org/2069303002/diff/60001/components/cronet/andro...
components/cronet/android/api/src/org/chromium/net/QuicException.java:12: * An
instance of QuicException is passed to
can this go on prior line?

https://codereview.chromium.org/2069303002/diff/60001/components/cronet/andro...
components/cronet/android/api/src/org/chromium/net/QuicException.java:12: * An
instance of QuicException is passed to
QuicException->{@code QuicException}

https://codereview.chromium.org/2069303002/diff/60001/components/cronet/andro...
components/cronet/android/api/src/org/chromium/net/QuicException.java:13: *
{@link UrlRequest.Callback#onFailed UrlRequest.Callback.onFailed()} when the
error code is
I'm not sure we should mention UrlRequest here as we expect this to soon be
returned by BiDirectionalStream too

https://codereview.chromium.org/2069303002/diff/60001/components/cronet/andro...
components/cronet/android/api/src/org/chromium/net/QuicException.java:18: //
QUIC detailed error code
Remove this comment as it is redundant with the nicely self-describing variable
name.
From https://google.github.io/styleguide/cppguide.html#Comments "Self-describing
code doesn't need a comment."

https://codereview.chromium.org/2069303002/diff/60001/components/cronet/andro...
components/cronet/android/api/src/org/chromium/net/QuicException.java:30: * <a
href=https://chromium.googlesource.com/chromium/src/+/master/net/quic/quic_protocol.h#503>
Update URL as per line 10 comment

https://codereview.chromium.org/2069303002/diff/60001/components/cronet/andro...
components/cronet/android/api/src/org/chromium/net/QuicException.java:41: * <a
href=https://chromium.googlesource.com/chromium/src/+/master/net/quic/quic_protocol.h>
update URL as per line 10 comment

https://codereview.chromium.org/2069303002/diff/60001/components/cronet/andro...
components/cronet/android/api/src/org/chromium/net/QuicException.java:42: *
QuicErrorCode</a>.
please make this consistent with the other two references; either
"QuicErrorCode" in all three places or "this file" in all three places.

https://codereview.chromium.org/2069303002/diff/60001/components/cronet/andro...
File components/cronet/android/api/src/org/chromium/net/UrlRequestException.java
(right):

https://codereview.chromium.org/2069303002/diff/60001/components/cronet/andro...
components/cronet/android/api/src/org/chromium/net/UrlRequestException.java:19:
*     {@link QuicException}
funky line wrapping

https://codereview.chromium.org/2069303002/diff/60001/components/cronet/andro...
components/cronet/android/api/src/org/chromium/net/UrlRequestException.java:70:
* Error code indicating an error related to the QUIC protocol.
linkify QUIC

https://codereview.chromium.org/2069303002/diff/60001/components/cronet/andro...
components/cronet/android/api/src/org/chromium/net/UrlRequestException.java:70:
* Error code indicating an error related to the QUIC protocol.
mention this class can be cast to QuicException for more detailed information

https://codereview.chromium.org/2069303002/diff/60001/components/cronet/andro...
File
components/cronet/android/java/src/org/chromium/net/CronetBidirectionalStream.java
(right):

https://codereview.chromium.org/2069303002/diff/60001/components/cronet/andro...
components/cronet/android/java/src/org/chromium/net/CronetBidirectionalStream.java:559:
long receivedBytesCount) {
ugh, duplicated code...  fixing is not for this CL.  I'm just grumbling.

[mgersh, 2016-07-01 15:21:15]

https://codereview.chromium.org/2069303002/diff/60001/components/cronet/andro...
File components/cronet/android/api/src/org/chromium/net/QuicException.java
(right):

https://codereview.chromium.org/2069303002/diff/60001/components/cronet/andro...
components/cronet/android/api/src/org/chromium/net/QuicException.java:8: *
Subclass of {@link UrlRequestException} (through {@link CronetException}) which
contains a
On 2016/07/01 12:22:02, pauljensen wrote:
> I don't think we need to mention CronetException...hopefully we'll get rid of
it
> soon enough

Done.

https://codereview.chromium.org/2069303002/diff/60001/components/cronet/andro...
components/cronet/android/api/src/org/chromium/net/QuicException.java:10: * <a
href=https://chromium.googlesource.com/chromium/src/+/master/net/quic/quic_protocol.h#503>
On 2016/07/01 12:22:02, pauljensen wrote:
> Line 503 is already incorrect.
> I was going to suggest including a git hash in the URL so the line number
stays
> accurate forever, but then embedders won't see new error codes.  How about
this
> URL:
>
https://cs.chromium.org/chromium/src/net/quic/quic_protocol.h?type=cs&q="enum...

Huh, cool. Done.

https://codereview.chromium.org/2069303002/diff/60001/components/cronet/andro...
components/cronet/android/api/src/org/chromium/net/QuicException.java:12: * An
instance of QuicException is passed to
On 2016/07/01 12:22:02, pauljensen wrote:
> can this go on prior line?

You mean I can't just solve all my formatting problems ever with git cl format?
:)
(Done.)

https://codereview.chromium.org/2069303002/diff/60001/components/cronet/andro...
components/cronet/android/api/src/org/chromium/net/QuicException.java:12: * An
instance of QuicException is passed to
On 2016/07/01 12:22:02, pauljensen wrote:
> QuicException->{@code QuicException}

Done.

https://codereview.chromium.org/2069303002/diff/60001/components/cronet/andro...
components/cronet/android/api/src/org/chromium/net/QuicException.java:13: *
{@link UrlRequest.Callback#onFailed UrlRequest.Callback.onFailed()} when the
error code is
On 2016/07/01 12:22:02, pauljensen wrote:
> I'm not sure we should mention UrlRequest here as we expect this to soon be
> returned by BiDirectionalStream too

Done.

https://codereview.chromium.org/2069303002/diff/60001/components/cronet/andro...
components/cronet/android/api/src/org/chromium/net/QuicException.java:18: //
QUIC detailed error code
On 2016/07/01 12:22:02, pauljensen wrote:
> Remove this comment as it is redundant with the nicely self-describing
variable
> name.
> From https://google.github.io/styleguide/cppguide.html#Comments
"Self-describing
> code doesn't need a comment."

Done. I agree with you that it's weird, I just did it to be consistent with
UrlRequestException's private fields.

https://codereview.chromium.org/2069303002/diff/60001/components/cronet/andro...
components/cronet/android/api/src/org/chromium/net/QuicException.java:30: * <a
href=https://chromium.googlesource.com/chromium/src/+/master/net/quic/quic_protocol.h#503>
On 2016/07/01 12:22:02, pauljensen wrote:
> Update URL as per line 10 comment

Done.

https://codereview.chromium.org/2069303002/diff/60001/components/cronet/andro...
components/cronet/android/api/src/org/chromium/net/QuicException.java:41: * <a
href=https://chromium.googlesource.com/chromium/src/+/master/net/quic/quic_protocol.h>
On 2016/07/01 12:22:02, pauljensen wrote:
> update URL as per line 10 comment

Done.

https://codereview.chromium.org/2069303002/diff/60001/components/cronet/andro...
components/cronet/android/api/src/org/chromium/net/QuicException.java:42: *
QuicErrorCode</a>.
On 2016/07/01 12:22:02, pauljensen wrote:
> please make this consistent with the other two references; either
> "QuicErrorCode" in all three places or "this file" in all three places.

Done.

https://codereview.chromium.org/2069303002/diff/60001/components/cronet/andro...
File components/cronet/android/api/src/org/chromium/net/UrlRequestException.java
(right):

https://codereview.chromium.org/2069303002/diff/60001/components/cronet/andro...
components/cronet/android/api/src/org/chromium/net/UrlRequestException.java:19:
*     {@link QuicException}
On 2016/07/01 12:22:02, pauljensen wrote:
> funky line wrapping

Done.

https://codereview.chromium.org/2069303002/diff/60001/components/cronet/andro...
components/cronet/android/api/src/org/chromium/net/UrlRequestException.java:70:
* Error code indicating an error related to the QUIC protocol.
On 2016/07/01 12:22:02, pauljensen wrote:
> linkify QUIC

Done.

https://codereview.chromium.org/2069303002/diff/60001/components/cronet/andro...
components/cronet/android/api/src/org/chromium/net/UrlRequestException.java:70:
* Error code indicating an error related to the QUIC protocol.
On 2016/07/01 12:22:02, pauljensen wrote:
> mention this class can be cast to QuicException for more detailed information

Done.

[pauljensen, 2016-07-01 18:40:41]

lgtm

https://codereview.chromium.org/2069303002/diff/80001/components/cronet/andro...
File components/cronet/android/cronet_url_request_adapter.cc (right):

https://codereview.chromium.org/2069303002/diff/80001/components/cronet/andro...
components/cronet/android/cronet_url_request_adapter.cc:25: #include
"net/http/http_util.h"
#include "net/quic/quic_protocol.h"

[mgersh, 2016-07-01 19:27:54]

https://codereview.chromium.org/2069303002/diff/80001/components/cronet/andro...
File components/cronet/android/cronet_url_request_adapter.cc (right):

https://codereview.chromium.org/2069303002/diff/80001/components/cronet/andro...
components/cronet/android/cronet_url_request_adapter.cc:25: #include
"net/http/http_util.h"
On 2016/07/01 18:40:41, pauljensen wrote:
> #include "net/quic/quic_protocol.h"

Done.

[mgersh, 2016-07-01 19:53:05]

The CQ bit was checked by mgersh@chromium.org

[mgersh, 2016-07-01 19:53:06]

The patchset sent to the CQ was uploaded after l-g-t-m from
xunjieli@chromium.org, pauljensen@chromium.org
Link to the patchset: https://codereview.chromium.org/2069303002/#ps100001
(title: "header")

[commit-bot: I haz the power, 2016-07-01 19:53:42]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-01 20:57:22]

Description was changed from

==========
Add new Cronet exception class for QUIC errors

Create a subclass of CronetException, called QuicException, which contains the
detailed QUIC error code when it exists.
Propagate the error code from NetErrorDetails through Cronet. For now,
only UrlRequest reports the detailed error, not BidirectionalStream.

BUG=614507
==========

to

==========
Add new Cronet exception class for QUIC errors

Create a subclass of CronetException, called QuicException, which contains the
detailed QUIC error code when it exists.
Propagate the error code from NetErrorDetails through Cronet. For now,
only UrlRequest reports the detailed error, not BidirectionalStream.

BUG=614507
==========

[commit-bot: I haz the power, 2016-07-01 20:57:23]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-07-01 20:57:32]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-01 20:58:53]

Description was changed from

==========
Add new Cronet exception class for QUIC errors

Create a subclass of CronetException, called QuicException, which contains the
detailed QUIC error code when it exists.
Propagate the error code from NetErrorDetails through Cronet. For now,
only UrlRequest reports the detailed error, not BidirectionalStream.

BUG=614507
==========

to

==========
Add new Cronet exception class for QUIC errors

Create a subclass of CronetException, called QuicException, which contains the
detailed QUIC error code when it exists.
Propagate the error code from NetErrorDetails through Cronet. For now,
only UrlRequest reports the detailed error, not BidirectionalStream.

BUG=614507

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

[commit-bot: I haz the power, 2016-07-01 20:58:54]

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