Custom hosts in QuicTestPacketMaker.

Custom hosts in QuicTestPacketMaker.

* Add host parameter to QuicTestPacketMaker constructor to allow for non-default
  hostnames in generated packets.
* Change hostname in QuicNetworkTransactionTest.SecureResourceOver* in
  anticipation for a future CL that will enforce certificate validation in these
  unittests.
* s/kServerHostname/kDefaultServerHostName/ in QuicHttpStreamTest to match
  QuicStreamFactoryTest.

BUG=474217
Committed: https://crrev.com/b07c0553bc25ef32023c8dfcb7df1516414daeac
Cr-Commit-Position: refs/heads/master@{#329890}

[Bence, 2015-05-14 18:06:21]

bnc@chromium.org changed reviewers:
+ rch@chromium.org

[Bence, 2015-05-14 18:06:22]

Ryan: PTAL.  Thanks.

[Ryan Hamilton, 2015-05-14 18:17:03]

Looks good. One small suggestion.

https://codereview.chromium.org/1142523004/diff/20001/net/quic/quic_network_t...
File net/quic/quic_network_transaction_unittest.cc (right):

https://codereview.chromium.org/1142523004/diff/20001/net/quic/quic_network_t...
net/quic/quic_network_transaction_unittest.cc:1212: QuicTestPacketMaker
maker(GetParam(), 0, clock_, "www.example.org");
So is the reason you need this maker so that you can change the host? If so, I
recommend adding a set_hostname() method to QuicTestPacketMaker and then just
calling that here on maker_.

[Bence, 2015-05-14 18:33:01]

PTAL.  Thanks.

https://codereview.chromium.org/1142523004/diff/20001/net/quic/quic_network_t...
File net/quic/quic_network_transaction_unittest.cc (right):

https://codereview.chromium.org/1142523004/diff/20001/net/quic/quic_network_t...
net/quic/quic_network_transaction_unittest.cc:1212: QuicTestPacketMaker
maker(GetParam(), 0, clock_, "www.example.org");
On 2015/05/14 18:17:02, Ryan Hamilton wrote:
> So is the reason you need this maker so that you can change the host? If so, I
> recommend adding a set_hostname() method to QuicTestPacketMaker and then just
> calling that here on maker_.

Done.

[Ryan Hamilton, 2015-05-14 18:57:16]

lgtm

https://codereview.chromium.org/1142523004/diff/40001/net/quic/quic_network_t...
File net/quic/quic_network_transaction_unittest.cc (right):

https://codereview.chromium.org/1142523004/diff/40001/net/quic/quic_network_t...
net/quic/quic_network_transaction_unittest.cc:1212:
maker_.set_hostname("www.example.org");
nice! This change makes it nice and clear what this TEST is doing.

(Though since this test is actually an "over insecure QUIC" test, I think the
old hostname would have worked.  But then the next test would be less symmetric
so keeping it this way is cool)

https://codereview.chromium.org/1142523004/diff/40001/net/quic/quic_network_t...
net/quic/quic_network_transaction_unittest.cc:1256:
cert->VerifyNameMatch("www.example.org", &common_name_fallback_used));
Good call! You could also add a maker_.host() and use it here, but perhaps
that's too much effort?

[Bence, 2015-05-14 18:57:59]

The CQ bit was checked by bnc@chromium.org

[commit-bot: I haz the power, 2015-05-14 18:58:26]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1142523004/40001

[Bence, 2015-05-14 19:00:54]

Thanks.

https://codereview.chromium.org/1142523004/diff/40001/net/quic/quic_network_t...
File net/quic/quic_network_transaction_unittest.cc (right):

https://codereview.chromium.org/1142523004/diff/40001/net/quic/quic_network_t...
net/quic/quic_network_transaction_unittest.cc:1212:
maker_.set_hostname("www.example.org");
On 2015/05/14 18:57:16, Ryan Hamilton wrote:
> nice! This change makes it nice and clear what this TEST is doing.
> 
> (Though since this test is actually an "over insecure QUIC" test, I think the
> old hostname would have worked.  But then the next test would be less
symmetric
> so keeping it this way is cool)

That's exactly what I was thinking.  Though the two tests are not quite symmetic
enough to make a test subclass, but you'll see a couple of those in my next two
CLs.

https://codereview.chromium.org/1142523004/diff/40001/net/quic/quic_network_t...
net/quic/quic_network_transaction_unittest.cc:1256:
cert->VerifyNameMatch("www.example.org", &common_name_fallback_used));
On 2015/05/14 18:57:16, Ryan Hamilton wrote:
> Good call! You could also add a maker_.host() and use it here, but perhaps
> that's too much effort?

Fine balance between aesthetic completeness and not having too much logic in
tests.  I'll pass on this one, sorry.

[commit-bot: I haz the power, 2015-05-14 19:07:33]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-05-14 19:08:26]

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