Set alternative service even if it is broken.

net/http/http_server_properties_impl_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/http/http_server_properties_impl.h"
 
 #include <string>
 #include <vector>
 
 #include "base/basictypes.h"
@@ skipping 347 matching lines @@
   ASSERT_TRUE(HasAlternativeService(test_host_port_pair));
   EXPECT_TRUE(impl_.IsAlternativeServiceBroken(alternative_service1));
 
   const AlternativeService alternative_service2(NPN_SPDY_4, "foo", 1234);
   impl_.SetAlternativeService(test_host_port_pair, alternative_service2, 1.0);
   EXPECT_TRUE(impl_.IsAlternativeServiceBroken(alternative_service1));
   EXPECT_FALSE(impl_.IsAlternativeServiceBroken(alternative_service2));
   EXPECT_EQ(1234, impl_.GetAlternativeService(test_host_port_pair).port);
 }
 
+// A broken alternative service in the mapping carries meaningful information,
+// therefore it should not be ignored by SetAlternativeService().
+TEST_F(AlternateProtocolServerPropertiesTest, AddBroken) {
+  HostPortPair test_host_port_pair("foo", 80);
+  ASSERT_FALSE(HasAlternativeService(test_host_port_pair));
+
+  const AlternativeService alternative_service(NPN_SPDY_4, "foo", 443);
+  impl_.MarkAlternativeServiceBroken(alternative_service);
+  EXPECT_TRUE(impl_.IsAlternativeServiceBroken(alternative_service));
+
+  impl_.SetAlternativeService(test_host_port_pair, alternative_service, 1.0);
+  ASSERT_TRUE(HasAlternativeService(test_host_port_pair));

[Ryan Hamilton, 2015/06/24 18:55:21]

What will GetAlternateService return? Does this test fail on HEAD? (I assume so)

[Bence, 2015/06/24 20:30:46]

GetAlternativeService() will return alternative_service, I have added a check on
that.

Yes, this test fails on HEAD in its current (Patch Set 3) form, see trybot
output of Patch Set 2.

Yes, this test fails on HEAD in its most recent (Patch Set 5) form, I confirmed
locally.

+}

[Ryan Hamilton, 2015/06/24 18:55:21]

Also, your CL comment mentions the interaction with canonical hosts. Can you add
a test which demonstrates that behavior too? (One which fails on HEAD but passes
with this CL). 

[Bence, 2015/06/24 20:30:46]

Good point.  I combined the two tests into one, because the new one (testing
interaction with canonical host) is really a superset of the first one.

+
 TEST_F(AlternateProtocolServerPropertiesTest, ClearBroken) {
   HostPortPair test_host_port_pair("foo", 80);
   const AlternativeService alternative_service(NPN_SPDY_4, "foo", 443);
   impl_.SetAlternativeService(test_host_port_pair, alternative_service, 1.0);
   impl_.MarkAlternativeServiceBroken(alternative_service);
   ASSERT_TRUE(HasAlternativeService(test_host_port_pair));
   EXPECT_TRUE(impl_.IsAlternativeServiceBroken(alternative_service));
   impl_.ClearAlternativeService(test_host_port_pair);
   EXPECT_FALSE(impl_.IsAlternativeServiceBroken(alternative_service));
 }
@@ skipping 411 matching lines @@
   EXPECT_EQ(100, stats2->bandwidth_estimate.ToBitsPerSecond());
 
   impl_.Clear();
   const ServerNetworkStats* stats3 = impl_.GetServerNetworkStats(foo_server);
   EXPECT_EQ(NULL, stats3);
 }
 
 }  // namespace
 
 }  // namespace net