Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(33)

Issue 20898003: Revert 213862 "net: make QUIC ProofVerifier more generic." (Closed)

Created:
7 years, 4 months ago by kareng
Modified:
7 years, 4 months ago
Reviewers:
agl
CC:
chromium-reviews, cbentzel+watch_chromium.org
Visibility:
Public.

Description

Revert 213862 "net: make QUIC ProofVerifier more generic." > net: make QUIC ProofVerifier more generic. > > The QUIC ProofVerifier code is currently rather Chromium specific: it's using > weak pointers, base::Bind etc. This change wraps abstractions around things so > that the code is more portable again. > > It also solves an issue where, when a QUIC connection is canceled while a > verification is running, the verification can write into free memory. > > I went back and forth on this change a bit. It effectively reinvents weak > pointers and callbacks in order not to use the Chromium versions of these > things. This is unfortunate but it is desirable to minimise the amount of skew > between different copies of the QUIC code. In the end, the duplicate didn't > seem so bad to me. > > Weak pointers are replaced with an explicit callback interface for the > ProofVerifier. The QUIC client stream implements a Cancel method so that it can > cope with being deleted while a proof verification is still running. > > The need to store a CertVerifyDetails is taken care of with an abstract class > for wrapping "implementation specific" results from a verification. There is > still Chromium-specific code that casts it to a Chromium object, but that's > unavoidable somewhere. (Although it's not clear that the QUIC client stream is > the best place for it.) > > BUG=none > > Review URL: https://chromiumcodereview.appspot.com/20047002 TBR=agl@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=214033

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+192 lines, -340 lines) Patch
M net/net.gyp View 1 chunk +0 lines, -1 line 0 comments Download
M net/quic/crypto/crypto_handshake.h View 3 chunks +6 lines, -6 lines 0 comments Download
M net/quic/crypto/crypto_handshake.cc View 2 chunks +6 lines, -6 lines 0 comments Download
M net/quic/crypto/proof_test.cc View 3 chunks +74 lines, -89 lines 0 comments Download
M net/quic/crypto/proof_verifier.h View 1 chunk +16 lines, -52 lines 0 comments Download
D net/quic/crypto/proof_verifier.cc View 1 chunk +0 lines, -15 lines 0 comments Download
M net/quic/crypto/proof_verifier_chromium.h View 3 chunks +12 lines, -17 lines 0 comments Download
M net/quic/crypto/proof_verifier_chromium.cc View 7 chunks +25 lines, -40 lines 0 comments Download
M net/quic/quic_crypto_client_stream.h View 4 chunks +17 lines, -34 lines 0 comments Download
M net/quic/quic_crypto_client_stream.cc View 9 chunks +26 lines, -67 lines 0 comments Download
M net/tools/quic/test_tools/quic_test_client.cc View 2 chunks +10 lines, -13 lines 0 comments Download

Messages

Total messages: 2 (0 generated)
kareng
7 years, 4 months ago (2013-07-27 04:50:18 UTC) #1
kareng
7 years, 4 months ago (2013-07-27 04:50:33 UTC) #2
Message was sent while issue was closed.
Committed patchset #1 manually as r214033.

Powered by Google App Engine
This is Rietveld 408576698