Chromium Code Reviews
[email protected] (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(111)

Issue 2109803002: Change a number of SPDY unittests from http to https. (Closed)

Created:
4 years, 5 months ago by Bence
Modified:
4 years, 5 months ago
Reviewers:
Ryan Hamilton
CC:
chromium-reviews, cbentzel+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Change a number of SPDY unittests from http to https. * Remove enable_alternative_service_for_insecure_origins; * change alternative service unittests from http to https; * change kDefaultUrl from http to https; * s/kDefaultURL/kDefaultUrl/ (to match the one in QuicStreamFactoryTest); * adapt a number of tests, explicitly use old URL in others; * remove SpdyTestUtil::default_url(); * remove moot SpdyAlternativeServiceThroughProxy; * remove SpdyNetworkTransactionTest::GetDefaultUrl() and port_; * add SpdyNetworkTransactionTest members to make tests more concise. Besides an improvement in code health, this CL will make it easier to write unittests for https://crbug.com/615413. BUG=621621, 624073 Committed: https://crrev.com/b2602438cbcdd99e6eb34d8be9c54d65e8bf087f Cr-Commit-Position: refs/heads/master@{#402686}

Patch Set 1 #

Total comments: 4

Patch Set 2 : Rebase. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+374 lines, -518 lines) Patch
M net/http/http_network_session.h View 1 chunk +0 lines, -4 lines 0 comments Download
M net/http/http_network_session.cc View 1 chunk +0 lines, -1 line 0 comments Download
M net/http/http_network_transaction.cc View 1 chunk +1 line, -2 lines 0 comments Download
M net/http/http_network_transaction_unittest.cc View 45 chunks +118 lines, -278 lines 0 comments Download
M net/http/http_stream_factory_impl_job.cc View 1 chunk +1 line, -2 lines 0 comments Download
M net/http/http_stream_factory_impl_job_controller.cc View 1 chunk +1 line, -5 lines 0 comments Download
M net/http/http_stream_factory_impl_request_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M net/quic/quic_network_transaction_unittest.cc View 2 chunks +5 lines, -7 lines 0 comments Download
M net/spdy/spdy_http_stream_unittest.cc View 1 chunk +1 line, -0 lines 0 comments Download
M net/spdy/spdy_network_transaction_unittest.cc View 62 chunks +77 lines, -102 lines 0 comments Download
M net/spdy/spdy_session_unittest.cc View 78 chunks +166 lines, -109 lines 0 comments Download
M net/spdy/spdy_stream_unittest.cc View 1 chunk +1 line, -0 lines 0 comments Download
M net/spdy/spdy_test_util_common.h View 3 chunks +1 line, -3 lines 0 comments Download
M net/spdy/spdy_test_util_common.cc View 3 chunks +1 line, -4 lines 0 comments Download

Messages

Total messages: 10 (4 generated)
Bence
Ryan: PTAL. Thank you. https://codereview.chromium.org/2109803002/diff/1/net/http/http_network_transaction_unittest.cc File net/http/http_network_transaction_unittest.cc (left): https://codereview.chromium.org/2109803002/diff/1/net/http/http_network_transaction_unittest.cc#oldcode10263 net/http/http_network_transaction_unittest.cc:10263: TEST_P(HttpNetworkTransactionTest, DisableAlternativeServiceToDifferentHost) { After changing ...
4 years, 5 months ago (2016-06-28 20:09:21 UTC) #2
Ryan Hamilton
lgtm https://codereview.chromium.org/2109803002/diff/1/net/spdy/spdy_network_transaction_unittest.cc File net/spdy/spdy_network_transaction_unittest.cc (right): https://codereview.chromium.org/2109803002/diff/1/net/spdy/spdy_network_transaction_unittest.cc#newcode64 net/spdy/spdy_network_transaction_unittest.cc:64: enum SpdyNetworkTransactionTestSSLType { Is this really "SSL type" ...
4 years, 5 months ago (2016-06-28 22:57:35 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2109803002/20001
4 years, 5 months ago (2016-06-29 00:41:08 UTC) #6
commit-bot: I haz the power
Committed patchset #2 (id:20001)
4 years, 5 months ago (2016-06-29 02:39:58 UTC) #7
commit-bot: I haz the power
Patchset 2 (id:??) landed as https://crrev.com/b2602438cbcdd99e6eb34d8be9c54d65e8bf087f Cr-Commit-Position: refs/heads/master@{#402686}
4 years, 5 months ago (2016-06-29 02:43:19 UTC) #9
Bence
4 years, 5 months ago (2016-06-30 18:19:53 UTC) #10
Message was sent while issue was closed.
Thank you.

https://codereview.chromium.org/2109803002/diff/1/net/spdy/spdy_network_trans...
File net/spdy/spdy_network_transaction_unittest.cc (right):

https://codereview.chromium.org/2109803002/diff/1/net/spdy/spdy_network_trans...
net/spdy/spdy_network_transaction_unittest.cc:64: enum
SpdyNetworkTransactionTestSSLType {
On 2016/06/28 22:57:35, Ryan Hamilton wrote:
> Is this really "SSL type" any more? Fix in a followup?

Aaaand https://crrev.com/2109593007 fixes it for good.

Powered by Google App Engine
This is Rietveld 408576698