DeterministicSocketDataProvider should not be RefCounted

net/http/http_pipelined_network_transaction_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 <string>
 
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_vector.h"
 #include "base/stringprintf.h"
 #include "base/utf_string_conversions.h"
@@ skipping 209 matching lines @@
     ExpectResponse("three.html", three_transaction, SYNCHRONOUS);
 
     one_transaction.reset();
     EXPECT_EQ(OK, four_callback.WaitForResult());
     ExpectResponse("four.html", four_transaction, SYNCHRONOUS);
   }
 
   DeterministicMockClientSocketFactory factory_;
   ClientSocketPoolHistograms histograms_;
   MockTransportClientSocketPool pool_;
-  std::vector<scoped_refptr<DeterministicSocketData> > data_vector_;
+  ScopedVector<DeterministicSocketData> data_vector_;
   TestCompletionCallback callback_;
   ScopedVector<HttpRequestInfo> request_info_vector_;
   bool default_pipelining_enabled_;
 
   SimpleProxyConfigService* proxy_config_service_;
   scoped_ptr<ProxyService> proxy_service_;
   MockHostResolver mock_resolver_;
   scoped_refptr<SSLConfigService> ssl_config_;
   scoped_ptr<HttpAuthHandlerMock::Factory> auth_handler_factory_;
   HttpServerPropertiesImpl http_server_properties_;
@@ skipping 584 matching lines @@
             two_transaction.Start(GetRequestInfo("two.html"),
                                   two_callback.callback(), BoundNetLog()));
   // Posted tasks should be:
   // 1. MockHostResolverBase::ResolveNow
   // 2. HttpStreamFactoryImpl::Job::OnStreamReadyCallback for job 1
   // 3. HttpStreamFactoryImpl::Job::OnStreamReadyCallback for job 2
   //
   // We need to make sure that the response that triggers OnPipelineFeedback(OK)
   // is called in between when task #3 is scheduled and when it runs. The
   // DataRunnerObserver does that.
-  DataRunnerObserver observer(data_vector_[0].get(), 3);
+  DataRunnerObserver observer(data_vector_[0], 3);
   MessageLoop::current()->AddTaskObserver(&observer);
   data_vector_[0]->SetStop(4);
   MessageLoop::current()->RunAllPending();
   data_vector_[0]->SetStop(10);
 
   EXPECT_EQ(OK, one_callback.WaitForResult());
   ExpectResponse("one.html", one_transaction, SYNCHRONOUS);
   EXPECT_EQ(OK, two_callback.WaitForResult());
   ExpectResponse("two.html", two_transaction, SYNCHRONOUS);
 }
 
 TEST_F(HttpPipelinedNetworkTransactionTest, ProxyChangesWhileConnecting) {
   Initialize(false);
 
-  scoped_refptr<DeterministicSocketData> data(
-      new DeterministicSocketData(NULL, 0, NULL, 0));
+  DeterministicSocketData* data =

[Ryan Hamilton, 2012/07/09 17:35:47]

Is this a memory leak?  (Or does AddSocketDataProvider transfer ownership?  If
so, I wonder if scoped_ptr<> and then .release() would be more clear.  Your
call.)

+      new DeterministicSocketData(NULL, 0, NULL, 0);
   data->set_connect_data(MockConnect(ASYNC, ERR_CONNECTION_REFUSED));
   factory_.AddSocketDataProvider(data);
 
-  scoped_refptr<DeterministicSocketData> data2(
-      new DeterministicSocketData(NULL, 0, NULL, 0));
+  DeterministicSocketData* data2 =
+      new DeterministicSocketData(NULL, 0, NULL, 0);
   data2->set_connect_data(MockConnect(ASYNC, ERR_FAILED));
   factory_.AddSocketDataProvider(data2);
 
   HttpNetworkTransaction transaction(session_.get());
   EXPECT_EQ(ERR_IO_PENDING,
             transaction.Start(GetRequestInfo("test.html"), callback_.callback(),
                               BoundNetLog()));
 
   proxy_config_service_->IncrementConfigId();
 
@@ skipping 41 matching lines @@
 
   data_vector_[0]->RunFor(2);  // 2 lines of headers.
   EXPECT_EQ(OK, two_callback.WaitForResult());
   ExpectResponse("two.html", two_transaction, ASYNC);
 }
 
 TEST_F(HttpPipelinedNetworkTransactionTest,
        ForcedPipelineConnectionErrorFailsBoth) {
   Initialize(true);
 
-  scoped_refptr<DeterministicSocketData> data(
-      new DeterministicSocketData(NULL, 0, NULL, 0));
+  DeterministicSocketData* data =
+      new DeterministicSocketData(NULL, 0, NULL, 0);
   data->set_connect_data(MockConnect(ASYNC, ERR_FAILED));
   factory_.AddSocketDataProvider(data);
 
   scoped_ptr<HttpNetworkTransaction> one_transaction(
       new HttpNetworkTransaction(session_.get()));
   TestCompletionCallback one_callback;
   EXPECT_EQ(ERR_IO_PENDING,
             one_transaction->Start(GetRequestInfo("one.html"),
                                    one_callback.callback(), BoundNetLog()));
 
@@ skipping 58 matching lines @@
               "Host: localhost\r\n"
               "Connection: keep-alive\r\n\r\n"
               "GET /four.html HTTP/1.1\r\n"
               "Host: localhost\r\n"
               "Connection: keep-alive\r\n\r\n"
               ),
   };
   MockRead reads[] = {
     MockRead(ASYNC, ERR_FAILED, 1),
   };
-  scoped_refptr<DeterministicSocketData> data(new DeterministicSocketData(
-      reads, arraysize(reads), writes, arraysize(writes)));
+  DeterministicSocketData* data = new DeterministicSocketData(
+      reads, arraysize(reads), writes, arraysize(writes));
   data->set_connect_data(MockConnect(ASYNC, OK));
   factory_.AddSocketDataProvider(data);
 
   scoped_ptr<HttpNetworkTransaction> one_transaction(
       new HttpNetworkTransaction(session_.get()));
   TestCompletionCallback one_callback;
   EXPECT_EQ(ERR_IO_PENDING,
             one_transaction->Start(GetRequestInfo("one.html"),
                                    one_callback.callback(), BoundNetLog()));
 
@@ skipping 24 matching lines @@
   EXPECT_EQ(ERR_PIPELINE_EVICTION, two_callback.WaitForResult());
   two_transaction.reset();
   EXPECT_EQ(ERR_PIPELINE_EVICTION, three_callback.WaitForResult());
   three_transaction.reset();
   EXPECT_EQ(ERR_PIPELINE_EVICTION, four_callback.WaitForResult());
 }
 
 }  // anonymous namespace
 
 }  // namespace net