Fix bug in net::RequestPriority -> HTTP/2 dependency conversion.

net/spdy/spdy_session_pool_unittest.cc

 // Copyright (c) 2013 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/spdy/spdy_session_pool.h"
 
 #include <cstddef>
 #include <string>
 #include <utility>
 
@@ skipping 496 matching lines @@
 
 TEST_P(SpdySessionPoolTest, IPPoolingCloseIdleSessions) {
   RunIPPoolingTest(SPDY_POOL_CLOSE_IDLE_SESSIONS);
 }
 
 // Construct a Pool with SpdySessions in various availability states. Simulate
 // an IP address change. Ensure sessions gracefully shut down. Regression test
 // for crbug.com/379469.
 TEST_P(SpdySessionPoolTest, IPAddressChanged) {
   MockConnect connect_data(SYNCHRONOUS, OK);
-  session_deps_.host_resolver->set_synchronous_mode(true);
 
   // This isn't testing anything having to do with SPDY frames; we
   // can ignore issues of how dependencies are set.  We default to
   // setting them (when doing the appropriate protocol) since that's
   // where we're eventually headed for all HTTP/2 connections.
+  session_deps_.host_resolver->set_synchronous_mode(true);

[Bence, 2016/03/17 01:48:59]

Why did you move the synchronous line below the comment about dependencies?

[Randy Smith (Not in Mondays), 2016/03/17 20:27:42]

Whoops, sorry, I think I misread it and thought it had to do with the
dependencies.  Fixed.

+  session_deps_.enable_priority_dependencies = true;

[Bence, 2016/03/17 01:48:59]

Optional: const bool enable_priority_dependencies = true; and use that local
instead of true in these two lines, or add /*enable_priority_dependencies=*/ in
spdy_util arguments, so that the reader of this code does not have to look up
what the second constructor argument is.

[Bence, 2016/03/17 12:25:59]

I guess I mean

const bool enable_priority_dependencies = true;
session_deps_.enable_priority_dependencies = enable_priority_dependencies;
SpdyTestUtil spdy_util(GetParam(), enable_priority_dependencies);

or

session_deps_.enable_priority_dependencies = true;
SpdyTestUtil spdy_util(GetParam(), /*enable_priority_dependencies=*/ true);

or 

session_deps_.enable_priority_dependencies = true;
SpdyTestUtil spdy_util(GetParam(), session_deps_.enable_priority_dependencies);

or you can even omit setting |session_deps_.enable_priority_dependencies| and
modify the comment to say that you set up the test class to match default
behavior.

This is all still optional.

[Randy Smith (Not in Mondays), 2016/03/17 20:27:42]

I took the comment option.

   SpdyTestUtil spdy_util(GetParam(), true);
-  SpdySession::SetPriorityDependencyDefaultForTesting(true);
 
   MockRead reads[] = {
       MockRead(SYNCHRONOUS, ERR_IO_PENDING)  // Stall forever.
   };
   scoped_ptr<SpdyFrame> req(
       spdy_util.ConstructSpdyGet("http://www.a.com", 1, MEDIUM));
   MockWrite writes[] = {CreateMockWrite(*req, 1)};
 
   StaticSocketDataProvider dataA(reads, arraysize(reads), writes,
                                  arraysize(writes));
@@ skipping 88 matching lines @@
   EXPECT_TRUE(sessionA->IsDraining());
   EXPECT_TRUE(sessionB->IsDraining());
   EXPECT_TRUE(sessionC->IsDraining());
 
   // Both streams were closed with an error.
   EXPECT_TRUE(delegateA.StreamIsClosed());
   EXPECT_EQ(ERR_NETWORK_CHANGED, delegateA.WaitForClose());
   EXPECT_TRUE(delegateB.StreamIsClosed());
   EXPECT_EQ(ERR_NETWORK_CHANGED, delegateB.WaitForClose());
 #endif  // defined(OS_ANDROID) || defined(OS_WIN) || defined(OS_IOS)
-  SpdySession::SetPriorityDependencyDefaultForTesting(false);
 }
 
 }  // namespace net