From 202e802d0db865031c18a2bc1b7ce924c5489b10 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bence=20B=C3=A9ky?= <bnc@chromium.org>
Date: Tue, 27 Mar 2018 00:29:41 +0000
Subject: [PATCH] Revert "Add WebSocket-related CHECKs to
HttpStreamFactoryImpl::Job."
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
This reverts commit 1a47d5d605a9c91366b5d73e9381d602b5c5884b.
Reason for revert: https://crbug.com/825804
Original change's description:
> Add WebSocket-related CHECKs to HttpStreamFactoryImpl::Job.
>
> The primary reason for these CHECKs is to better document expectations
> in code. Also, they are CHECKs instead of DCHECKs to help track down
> crashes in case these expectations do not hold.
>
> Bug: 819101
> Change-Id: Ia309bbff2f4e048926930cd9a15fbc107994895d
> Reviewed-on: https://chromium-review.googlesource.com/975621
> Reviewed-by: Paul Jensen <pauljensen@chromium.org>
> Commit-Queue: Bence Béky <bnc@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#545196}
TBR=pauljensen@chromium.org,bnc@chromium.org
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Skipping CQ (even though original CL landed more than 24 hours ago) because
this is the top crasher in the browser process on Mac, so this should go in
before next Canary cut, and definitely before next Dev cut, but ios-simulator
bot fails with exception on multiple tries (and also with other CLs).
Bug: 819101
Change-Id: Ib998f6b92dd70df943cfb25b1418e1070cb839aa
Reviewed-on: https://chromium-review.googlesource.com/980632
Commit-Queue: Bence Béky <bnc@chromium.org>
Reviewed-by: Bence Béky <bnc@chromium.org>
Reviewed-by: Paul Jensen <pauljensen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#545902}
---
net/http/http_stream_factory_impl_job.cc | 12 +-----------
1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/net/http/http_stream_factory_impl_job.cc b/net/http/http_stream_factory_impl_job.cc
index c90a039d2c509..fd0ff13c748a0 100644
--- a/net/http/http_stream_factory_impl_job.cc
+++ b/net/http/http_stream_factory_impl_job.cc
@@ -1051,14 +1051,8 @@ int HttpStreamFactoryImpl::Job::DoInitConnectionComplete(int result) {
net_log_.AddEvent(
NetLogEventType::HTTP_STREAM_REQUEST_PROTO,
base::Bind(&NetLogHttpStreamProtoCallback, negotiated_protocol_));
- if (negotiated_protocol_ == kProtoHTTP2) {
- // If request is WebSocket, HTTP/2 must not have been advertised in
- // the TLS handshake. The TLS layer must not have accepted the
- // server choosing HTTP/2.
- // TODO(bnc): Change to DCHECK once https://crbug.com/819101 is fixed.
- CHECK(!is_websocket_);
+ if (negotiated_protocol_ == kProtoHTTP2)
using_spdy_ = true;
- }
}
}
} else if (proxy_info_.is_https() && connection_->socket() &&
@@ -1219,10 +1213,6 @@ int HttpStreamFactoryImpl::Job::DoCreateStream() {
CHECK(!stream_.get());
- // WebSocket over HTTP/2 is only allowed to use existing connections.
- // TODO(bnc): Change to DCHECK once https://crbug.com/819101 is fixed.
- CHECK(!is_websocket_ || existing_spdy_session_);
-
// It is possible that a pushed stream has been opened by a server since last
// time Job checked above.
if (!existing_spdy_session_) {