Remove HttpTransaction::Destroy(), and do automatic memory management with scoped_ptr<>.

Review URL: http://codereview.chromium.org/7532

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@3701 0039d316-1c4b-4281-b951-d872f2087c98

11 files changed, 107 insertions, 154 deletions

diff --git a/net/http/http_cache.cc b/net/http/http_cache.cc
index 19c57ed..10fa6b0 100644
--- a/net/http/http_cache.cc
+++ b/net/http/http_cache.cc
@@ -156,8 +156,7 @@ HttpCache::ActiveEntry::~ActiveEntry() {
 
 //-----------------------------------------------------------------------------
 
-class HttpCache::Transaction : public HttpTransaction,
-                               public base::RefCounted<HttpCache::Transaction> {
+class HttpCache::Transaction : public HttpTransaction {
  public:
   explicit Transaction(HttpCache* cache)
       : request_(NULL),
@@ -175,12 +174,14 @@ class HttpCache::Transaction : public HttpTransaction,
         ALLOW_THIS_IN_INITIALIZER_LIST(
             network_read_callback_(this, &Transaction::OnNetworkReadCompleted)),
         ALLOW_THIS_IN_INITIALIZER_LIST(
-            cache_read_callback_(this, &Transaction::OnCacheReadCompleted)) {
-    AddRef();  // Balanced in Destroy
+            cache_read_callback_(new CancelableCompletionCallback<Transaction>(
+                this, &Transaction::OnCacheReadCompleted))) {
   }
 
+  // Clean up the transaction.
+  virtual ~Transaction();
+
   // HttpTransaction methods:
-  virtual void Destroy();
   virtual int Start(const HttpRequestInfo*, CompletionCallback*);
   virtual int RestartIgnoringLastError(CompletionCallback*);
   virtual int RestartWithAuth(const std::wstring& username,
@@ -298,7 +299,7 @@ class HttpCache::Transaction : public HttpTransaction,
   scoped_ptr<HttpRequestInfo> custom_request_;
   HttpCache* cache_;
   HttpCache::ActiveEntry* entry_;
-  HttpTransaction* network_trans_;
+  scoped_ptr<HttpTransaction> network_trans_;
   CompletionCallback* callback_;  // consumer's callback
   HttpResponseInfo response_;
   HttpResponseInfo auth_response_;
@@ -310,10 +311,10 @@ class HttpCache::Transaction : public HttpTransaction,
   uint64 final_upload_progress_;
   CompletionCallbackImpl<Transaction> network_info_callback_;
   CompletionCallbackImpl<Transaction> network_read_callback_;
-  CompletionCallbackImpl<Transaction> cache_read_callback_;
+  scoped_refptr<CancelableCompletionCallback<Transaction> > cache_read_callback_;
 };
 
-void HttpCache::Transaction::Destroy() {
+HttpCache::Transaction::~Transaction() {
   if (entry_) {
     if (mode_ & WRITE) {
       // Assume that this is not a successful write.
@@ -325,14 +326,13 @@ void HttpCache::Transaction::Destroy() {
     cache_->RemovePendingTransaction(this);
   }
 
-  if (network_trans_)
-    network_trans_->Destroy();
+  // If there is an outstanding callback, mark it as cancelled so running it
+  // does nothing.
+  cache_read_callback_->Cancel();
 
   // We could still have a cache read in progress, so we just null the cache_
   // pointer to signal that we are dead.  See OnCacheReadCompleted.
   cache_ = NULL;
-
-  Release();
 }
 
 int HttpCache::Transaction::Start(const HttpRequestInfo* request,
@@ -435,7 +435,7 @@ int HttpCache::Transaction::Read(char* buf, int buf_len,
   switch (mode_) {
     case NONE:
     case WRITE:
-      DCHECK(network_trans_);
+      DCHECK(network_trans_.get());
       rv = network_trans_->Read(buf, buf_len, &network_read_callback_);
       read_buf_ = buf;
       if (rv >= 0)
@@ -443,14 +443,14 @@ int HttpCache::Transaction::Read(char* buf, int buf_len,
       break;
     case READ:
       DCHECK(entry_);
-      AddRef();  // Balanced in OnCacheReadCompleted
+      cache_read_callback_->AddRef();  // Balanced in OnCacheReadCompleted
       rv = entry_->disk_entry->ReadData(kResponseContentIndex, read_offset_,
-                                        buf, buf_len, &cache_read_callback_);
+                                        buf, buf_len, cache_read_callback_);
       read_buf_ = buf;
       if (rv >= 0) {
         OnCacheReadCompleted(rv);
       } else if (rv != ERR_IO_PENDING) {
-        Release();
+        cache_read_callback_->Release();
       }
       break;
     default:
@@ -471,7 +471,7 @@ const HttpResponseInfo* HttpCache::Transaction::GetResponseInfo() const {
 }
 
 LoadState HttpCache::Transaction::GetLoadState() const {
-  if (network_trans_)
+  if (network_trans_.get())
     return network_trans_->GetLoadState();
   if (entry_ || !request_)
     return LOAD_STATE_IDLE;
@@ -479,7 +479,7 @@ LoadState HttpCache::Transaction::GetLoadState() const {
 }
 
 uint64 HttpCache::Transaction::GetUploadProgress() const {
-  if (network_trans_)
+  if (network_trans_.get())
     return network_trans_->GetUploadProgress();
   return final_upload_progress_;
 }
@@ -669,10 +669,10 @@ int HttpCache::Transaction::BeginCacheValidation() {
 
 int HttpCache::Transaction::BeginNetworkRequest() {
   DCHECK(mode_ & WRITE || mode_ == NONE);
-  DCHECK(!network_trans_);
+  DCHECK(!network_trans_.get());
 
-  network_trans_ = cache_->network_layer_->CreateTransaction();
-  if (!network_trans_)
+  network_trans_.reset(cache_->network_layer_->CreateTransaction());
+  if (!network_trans_.get())
     return net::ERR_FAILED;
 
   int rv = network_trans_->Start(request_, &network_info_callback_);
@@ -683,7 +683,7 @@ int HttpCache::Transaction::BeginNetworkRequest() {
 
 int HttpCache::Transaction::RestartNetworkRequest() {
   DCHECK(mode_ & WRITE || mode_ == NONE);
-  DCHECK(network_trans_);
+  DCHECK(network_trans_.get());
 
   int rv = network_trans_->RestartIgnoringLastError(&network_info_callback_);
   if (rv != ERR_IO_PENDING)
@@ -695,7 +695,7 @@ int HttpCache::Transaction::RestartNetworkRequestWithAuth(
     const std::wstring& username,
     const std::wstring& password) {
   DCHECK(mode_ & WRITE || mode_ == NONE);
-  DCHECK(network_trans_);
+  DCHECK(network_trans_.get());
 
   int rv = network_trans_->RestartWithAuth(username, password,
                                            &network_info_callback_);
@@ -861,8 +861,7 @@ void HttpCache::Transaction::OnNetworkInfoAvailable(int result) {
             cache_->ConvertWriterToReader(entry_);
             // We no longer need the network transaction, so destroy it.
             final_upload_progress_ = network_trans_->GetUploadProgress();
-            network_trans_->Destroy();
-            network_trans_ = NULL;
+            network_trans_.reset();
             mode_ = READ;
           }
         } else {
@@ -901,18 +900,16 @@ void HttpCache::Transaction::OnNetworkReadCompleted(int result) {
 }
 
 void HttpCache::Transaction::OnCacheReadCompleted(int result) {
-  // If Destroy was called while waiting for this callback, then cache_ will be
-  // NULL.  In that case, we don't want to do anything but cleanup.
-  if (cache_) {
-    if (result > 0) {
-      read_offset_ += result;
-    } else if (result == 0) {  // end of file
-      cache_->DoneReadingFromEntry(entry_, this);
-      entry_ = NULL;
-    }
-    HandleResult(result);
+  DCHECK(cache_);
+  cache_read_callback_->Release();  // Balance the AddRef() from Start()
+
+  if (result > 0) {
+    read_offset_ += result;
+  } else if (result == 0) {  // end of file
+    cache_->DoneReadingFromEntry(entry_, this);
+    entry_ = NULL;
   }
-  Release();
+  HandleResult(result);
 }
 
 //-----------------------------------------------------------------------------
diff --git a/net/http/http_cache_unittest.cc b/net/http/http_cache_unittest.cc
index 648a889..b715aa6 100644
--- a/net/http/http_cache_unittest.cc
+++ b/net/http/http_cache_unittest.cc
@@ -259,8 +259,8 @@ void RunTransactionTest(net::HttpCache* cache,
 
   // write to the cache
 
-  net::HttpTransaction* trans = cache->CreateTransaction();
-  ASSERT_TRUE(trans);
+  scoped_ptr<net::HttpTransaction> trans(cache->CreateTransaction());
+  ASSERT_TRUE(trans.get());
 
   int rv = trans->Start(&request, &callback);
   if (rv == net::ERR_IO_PENDING)
@@ -270,9 +270,7 @@ void RunTransactionTest(net::HttpCache* cache,
   const net::HttpResponseInfo* response = trans->GetResponseInfo();
   ASSERT_TRUE(response);
 
-  ReadAndVerifyTransaction(trans, trans_info);
-
-  trans->Destroy();
+  ReadAndVerifyTransaction(trans.get(), trans_info);
 }
 
 }  // namespace
@@ -285,10 +283,9 @@ void RunTransactionTest(net::HttpCache* cache,
 TEST(HttpCache, CreateThenDestroy) {
   MockHttpCache cache;
 
-  net::HttpTransaction* trans = cache.http_cache()->CreateTransaction();
-  ASSERT_TRUE(trans);
-
-  trans->Destroy();
+  scoped_ptr<net::HttpTransaction> trans(
+      cache.http_cache()->CreateTransaction());
+  ASSERT_TRUE(trans.get());
 }
 
 TEST(HttpCache, SimpleGET) {
@@ -342,15 +339,16 @@ TEST(HttpCache, SimpleGET_LoadOnlyFromCache_Miss) {
   MockHttpRequest request(transaction);
   TestCompletionCallback callback;
 
-  net::HttpTransaction* trans = cache.http_cache()->CreateTransaction();
-  ASSERT_TRUE(trans);
+  scoped_ptr<net::HttpTransaction> trans(
+      cache.http_cache()->CreateTransaction());
+  ASSERT_TRUE(trans.get());
 
   int rv = trans->Start(&request, &callback);
   if (rv == net::ERR_IO_PENDING)
     rv = callback.WaitForResult();
   ASSERT_EQ(net::ERR_CACHE_MISS, rv);
 
-  trans->Destroy();
+  trans.reset();
 
   EXPECT_EQ(0, cache.network_layer()->transaction_count());
   EXPECT_EQ(0, cache.disk_cache()->open_count());
@@ -482,7 +480,7 @@ TEST(HttpCache, SimpleGET_LoadValidateCache_Implicit) {
 struct Context {
   int result;
   TestCompletionCallback callback;
-  net::HttpTransaction* trans;
+  scoped_ptr<net::HttpTransaction> trans;
 
   Context(net::HttpTransaction* t) : result(net::ERR_IO_PENDING), trans(t) {
   }
@@ -517,7 +515,7 @@ TEST(HttpCache, SimpleGET_ManyReaders) {
     Context* c = context_list[i];
     if (c->result == net::ERR_IO_PENDING)
       c->result = c->callback.WaitForResult();
-    ReadAndVerifyTransaction(c->trans, kSimpleGET_Transaction);
+    ReadAndVerifyTransaction(c->trans.get(), kSimpleGET_Transaction);
   }
 
   // we should not have had to re-open the disk entry
@@ -528,7 +526,6 @@ TEST(HttpCache, SimpleGET_ManyReaders) {
 
   for (int i = 0; i < kNumTransactions; ++i) {
     Context* c = context_list[i];
-    c->trans->Destroy();
     delete c;
   }
 }
@@ -564,7 +561,6 @@ TEST(HttpCache, SimpleGET_ManyWriters_CancelFirst) {
       c->result = c->callback.WaitForResult();
     // destroy only the first transaction
     if (i == 0) {
-      c->trans->Destroy();
       delete c;
       context_list[i] = NULL;
     }
@@ -573,7 +569,7 @@ TEST(HttpCache, SimpleGET_ManyWriters_CancelFirst) {
   // complete the rest of the transactions
   for (int i = 1; i < kNumTransactions; ++i) {
     Context* c = context_list[i];
-    ReadAndVerifyTransaction(c->trans, kSimpleGET_Transaction);
+    ReadAndVerifyTransaction(c->trans.get(), kSimpleGET_Transaction);
   }
 
   // we should have had to re-open the disk entry
@@ -584,7 +580,6 @@ TEST(HttpCache, SimpleGET_ManyWriters_CancelFirst) {
 
   for (int i = 1; i < kNumTransactions; ++i) {
     Context* c = context_list[i];
-    c->trans->Destroy();
     delete c;
   }
 }
@@ -598,7 +593,8 @@ TEST(HttpCache, SimpleGET_AbandonedCacheRead) {
   MockHttpRequest request(kSimpleGET_Transaction);
   TestCompletionCallback callback;
 
-  net::HttpTransaction* trans = cache.http_cache()->CreateTransaction();
+  scoped_ptr<net::HttpTransaction> trans(
+      cache.http_cache()->CreateTransaction());
   int rv = trans->Start(&request, &callback);
   if (rv == net::ERR_IO_PENDING)
     rv = callback.WaitForResult();
@@ -610,7 +606,7 @@ TEST(HttpCache, SimpleGET_AbandonedCacheRead) {
 
   // Test that destroying the transaction while it is reading from the cache
   // works properly.
-  trans->Destroy();
+  trans.reset();
 
   // Make sure we pump any pending events, which should include a call to
   // HttpCache::Transaction::OnCacheReadCompleted.
@@ -697,15 +693,16 @@ TEST(HttpCache, SimplePOST_LoadOnlyFromCache_Miss) {
   MockHttpRequest request(transaction);
   TestCompletionCallback callback;
 
-  net::HttpTransaction* trans = cache.http_cache()->CreateTransaction();
-  ASSERT_TRUE(trans);
+  scoped_ptr<net::HttpTransaction> trans(
+      cache.http_cache()->CreateTransaction());
+  ASSERT_TRUE(trans.get());
 
   int rv = trans->Start(&request, &callback);
   if (rv == net::ERR_IO_PENDING)
     rv = callback.WaitForResult();
   ASSERT_EQ(net::ERR_CACHE_MISS, rv);
 
-  trans->Destroy();
+  trans.reset();
 
   EXPECT_EQ(0, cache.network_layer()->transaction_count());
   EXPECT_EQ(0, cache.disk_cache()->open_count());
@@ -808,8 +805,9 @@ TEST(HttpCache, CachedRedirect) {
 
   // write to the cache
   {
-    net::HttpTransaction* trans = cache.http_cache()->CreateTransaction();
-    ASSERT_TRUE(trans);
+    scoped_ptr<net::HttpTransaction> trans(
+        cache.http_cache()->CreateTransaction());
+    ASSERT_TRUE(trans.get());
 
     int rv = trans->Start(&request, &callback);
     if (rv == net::ERR_IO_PENDING)
@@ -825,9 +823,8 @@ TEST(HttpCache, CachedRedirect) {
     info->headers->EnumerateHeader(NULL, "Location", &location);
     EXPECT_EQ(location, "http://www.bar.com/");
 
-    // now, destroy the transaction without actually reading the response body.
-    // we want to test that it is still getting cached.
-    trans->Destroy();
+    // Destroy transaction when going out of scope. We have not actually
+    // read the response body -- want to test that it is still getting cached.
   }
   EXPECT_EQ(1, cache.network_layer()->transaction_count());
   EXPECT_EQ(0, cache.disk_cache()->open_count());
@@ -835,8 +832,9 @@ TEST(HttpCache, CachedRedirect) {
 
   // read from the cache
   {
-    net::HttpTransaction* trans = cache.http_cache()->CreateTransaction();
-    ASSERT_TRUE(trans);
+    scoped_ptr<net::HttpTransaction> trans(
+        cache.http_cache()->CreateTransaction());
+    ASSERT_TRUE(trans.get());
 
     int rv = trans->Start(&request, &callback);
     if (rv == net::ERR_IO_PENDING)
@@ -852,9 +850,8 @@ TEST(HttpCache, CachedRedirect) {
     info->headers->EnumerateHeader(NULL, "Location", &location);
     EXPECT_EQ(location, "http://www.bar.com/");
 
-    // now, destroy the transaction without actually reading the response body.
-    // we want to test that it is still getting cached.
-    trans->Destroy();
+    // Destroy transaction when going out of scope. We have not actually
+    // read the response body -- want to test that it is still getting cached.
   }
   EXPECT_EQ(1, cache.network_layer()->transaction_count());
   EXPECT_EQ(1, cache.disk_cache()->open_count());
@@ -962,13 +959,12 @@ TEST(HttpCache, SimpleGET_SSLError) {
   MockHttpRequest request(transaction);
   TestCompletionCallback callback;
 
-  net::HttpTransaction* trans = cache.http_cache()->CreateTransaction();
-  ASSERT_TRUE(trans);
+  scoped_ptr<net::HttpTransaction> trans(
+      cache.http_cache()->CreateTransaction());
+  ASSERT_TRUE(trans.get());
 
   int rv = trans->Start(&request, &callback);
   if (rv == net::ERR_IO_PENDING)
     rv = callback.WaitForResult();
   ASSERT_EQ(net::ERR_CACHE_MISS, rv);
-
-  trans->Destroy();
 }
diff --git a/net/http/http_network_layer_unittest.cc b/net/http/http_network_layer_unittest.cc
index 7f99e54..33c93e2 100644
--- a/net/http/http_network_layer_unittest.cc
+++ b/net/http/http_network_layer_unittest.cc
@@ -23,25 +23,23 @@ class HttpNetworkLayerTest : public PlatformTest {
 TEST_F(HttpNetworkLayerTest, CreateAndDestroy) {
   net::HttpNetworkLayer factory(NULL);
 
-  net::HttpTransaction* trans = factory.CreateTransaction();
-  trans->Destroy();
+  scoped_ptr<net::HttpTransaction> trans(factory.CreateTransaction());
 }
 
 TEST_F(HttpNetworkLayerTest, Suspend) {
   net::HttpNetworkLayer factory(NULL);
 
-  net::HttpTransaction* trans = factory.CreateTransaction();
-  trans->Destroy();
+  scoped_ptr<net::HttpTransaction> trans(factory.CreateTransaction());
+  trans.reset();
 
   factory.Suspend(true);
 
-  trans = factory.CreateTransaction();
+  trans.reset(factory.CreateTransaction());
   ASSERT_TRUE(trans == NULL);
 
   factory.Suspend(false);
 
-  trans = factory.CreateTransaction();
-  trans->Destroy();
+  trans.reset(factory.CreateTransaction());
 }
 
 TEST_F(HttpNetworkLayerTest, GoogleGET) {
@@ -50,7 +48,7 @@ TEST_F(HttpNetworkLayerTest, GoogleGET) {
 
   TestCompletionCallback callback;
 
-  net::HttpTransaction* trans = factory.CreateTransaction();
+  scoped_ptr<net::HttpTransaction> trans(factory.CreateTransaction());
 
   net::HttpRequestInfo request_info;
   request_info.url = GURL("http://www.google.com/");
@@ -64,9 +62,7 @@ TEST_F(HttpNetworkLayerTest, GoogleGET) {
   EXPECT_EQ(net::OK, rv);
 
   std::string contents;
-  rv = ReadTransaction(trans, &contents);
+  rv = ReadTransaction(trans.get(), &contents);
   EXPECT_EQ(net::OK, rv);
-
-  trans->Destroy();
 }
 
diff --git a/net/http/http_network_transaction.cc b/net/http/http_network_transaction.cc
index 033d0d9..d0a26d0 100644
--- a/net/http/http_network_transaction.cc
+++ b/net/http/http_network_transaction.cc
@@ -62,10 +62,6 @@ HttpNetworkTransaction::HttpNetworkTransaction(HttpNetworkSession* session,
 #endif
 }
 
-void HttpNetworkTransaction::Destroy() {
-  delete this;
-}
-
 int HttpNetworkTransaction::Start(const HttpRequestInfo* request_info,
                                   CompletionCallback* callback) {
   request_ = request_info;
diff --git a/net/http/http_network_transaction.h b/net/http/http_network_transaction.h
index a075e06..7fd0b3c 100644
--- a/net/http/http_network_transaction.h
+++ b/net/http/http_network_transaction.h
@@ -31,8 +31,9 @@ class HttpNetworkTransaction : public HttpTransaction {
   HttpNetworkTransaction(HttpNetworkSession* session,
                          ClientSocketFactory* socket_factory);
 
+  virtual ~HttpNetworkTransaction();
+
   // HttpTransaction methods:
-  virtual void Destroy();
   virtual int Start(const HttpRequestInfo* request_info,
                     CompletionCallback* callback);
   virtual int RestartIgnoringLastError(CompletionCallback* callback);
@@ -45,7 +46,6 @@ class HttpNetworkTransaction : public HttpTransaction {
   virtual uint64 GetUploadProgress() const;
 
  private:
-  ~HttpNetworkTransaction();
   void BuildRequestHeaders();
   void BuildTunnelRequest();
   void DoCallback(int result);
diff --git a/net/http/http_network_transaction_unittest.cc b/net/http/http_network_transaction_unittest.cc
index 7b2f04f..45fda4b 100644
--- a/net/http/http_network_transaction_unittest.cc
+++ b/net/http/http_network_transaction_unittest.cc
@@ -234,8 +234,8 @@ struct SimpleGetHelperResult {
 SimpleGetHelperResult SimpleGetHelper(MockRead data_reads[]) {
   SimpleGetHelperResult out;
 
-  net::HttpTransaction* trans = new net::HttpNetworkTransaction(
-      CreateSession(), &mock_socket_factory);
+  scoped_ptr<net::HttpTransaction> trans(new net::HttpNetworkTransaction(
+      CreateSession(), &mock_socket_factory));
 
   net::HttpRequestInfo request;
   request.method = "GET";
@@ -253,10 +253,8 @@ SimpleGetHelperResult SimpleGetHelper(MockRead data_reads[]) {
   EXPECT_EQ(net::ERR_IO_PENDING, rv);
 
   out.rv = callback.WaitForResult();
-  if (out.rv != net::OK) {
-    trans->Destroy();
+  if (out.rv != net::OK)
     return out;
-  }
 
   const net::HttpResponseInfo* response = trans->GetResponseInfo();
   EXPECT_TRUE(response != NULL);
@@ -264,20 +262,17 @@ SimpleGetHelperResult SimpleGetHelper(MockRead data_reads[]) {
   EXPECT_TRUE(response->headers != NULL);
   out.status_line = response->headers->GetStatusLine();
 
-  rv = ReadTransaction(trans, &out.response_data);
+  rv = ReadTransaction(trans.get(), &out.response_data);
   EXPECT_EQ(net::OK, rv);
 
-  trans->Destroy();
-
   return out;
 }
 
 //-----------------------------------------------------------------------------
 
 TEST_F(HttpNetworkTransactionTest, Basic) {
-  net::HttpTransaction* trans = new net::HttpNetworkTransaction(
-      CreateSession(), &mock_socket_factory);
-  trans->Destroy();
+  scoped_ptr<net::HttpTransaction> trans(new net::HttpNetworkTransaction(
+      CreateSession(), &mock_socket_factory));
 }
 
 TEST_F(HttpNetworkTransactionTest, SimpleGET) {
@@ -403,8 +398,8 @@ TEST_F(HttpNetworkTransactionTest, ReuseConnection) {
   };
 
   for (int i = 0; i < 2; ++i) {
-    net::HttpTransaction* trans =
-        new net::HttpNetworkTransaction(session, &mock_socket_factory);
+    scoped_ptr<net::HttpTransaction> trans(
+        new net::HttpNetworkTransaction(session, &mock_socket_factory));
 
     net::HttpRequestInfo request;
     request.method = "GET";
@@ -426,17 +421,15 @@ TEST_F(HttpNetworkTransactionTest, ReuseConnection) {
     EXPECT_EQ("HTTP/1.1 200 OK", response->headers->GetStatusLine());
 
     std::string response_data;
-    rv = ReadTransaction(trans, &response_data);
+    rv = ReadTransaction(trans.get(), &response_data);
     EXPECT_EQ(net::OK, rv);
     EXPECT_EQ(kExpectedResponseData[i], response_data);
-
-    trans->Destroy();
   }
 }
 
 TEST_F(HttpNetworkTransactionTest, Ignores100) {
-  net::HttpTransaction* trans = new net::HttpNetworkTransaction(
-      CreateSession(), &mock_socket_factory);
+  scoped_ptr<net::HttpTransaction> trans(new net::HttpNetworkTransaction(
+      CreateSession(), &mock_socket_factory));
 
   net::HttpRequestInfo request;
   request.method = "POST";
@@ -471,11 +464,9 @@ TEST_F(HttpNetworkTransactionTest, Ignores100) {
   EXPECT_EQ("HTTP/1.0 200 OK", response->headers->GetStatusLine());
 
   std::string response_data;
-  rv = ReadTransaction(trans, &response_data);
+  rv = ReadTransaction(trans.get(), &response_data);
   EXPECT_EQ(net::OK, rv);
   EXPECT_EQ("hello world", response_data);
-
-  trans->Destroy();
 }
 
 // read_failure specifies a read failure that should cause the network
@@ -514,8 +505,8 @@ void HttpNetworkTransactionTest::KeepAliveConnectionResendRequestTest(
   for (int i = 0; i < 2; ++i) {
     TestCompletionCallback callback;
 
-    net::HttpTransaction* trans =
-        new net::HttpNetworkTransaction(session, &mock_socket_factory);
+    scoped_ptr<net::HttpTransaction> trans(
+        new net::HttpNetworkTransaction(session, &mock_socket_factory));
 
     int rv = trans->Start(&request, &callback);
     EXPECT_EQ(net::ERR_IO_PENDING, rv);
@@ -530,11 +521,9 @@ void HttpNetworkTransactionTest::KeepAliveConnectionResendRequestTest(
     EXPECT_EQ("HTTP/1.1 200 OK", response->headers->GetStatusLine());
 
     std::string response_data;
-    rv = ReadTransaction(trans, &response_data);
+    rv = ReadTransaction(trans.get(), &response_data);
     EXPECT_EQ(net::OK, rv);
     EXPECT_EQ(kExpectedResponseData[i], response_data);
-
-    trans->Destroy();
   }
 }
 
@@ -549,8 +538,8 @@ TEST_F(HttpNetworkTransactionTest, KeepAliveConnectionEOF) {
 }
 
 TEST_F(HttpNetworkTransactionTest, NonKeepAliveConnectionReset) {
-  net::HttpTransaction* trans = new net::HttpNetworkTransaction(
-      CreateSession(), &mock_socket_factory);
+  scoped_ptr<net::HttpTransaction> trans(new net::HttpNetworkTransaction(
+      CreateSession(), &mock_socket_factory));
 
   net::HttpRequestInfo request;
   request.method = "GET";
@@ -578,8 +567,6 @@ TEST_F(HttpNetworkTransactionTest, NonKeepAliveConnectionReset) {
 
   const net::HttpResponseInfo* response = trans->GetResponseInfo();
   EXPECT_TRUE(response == NULL);
-
-  trans->Destroy();
 }
 
 // What do various browsers do when the server closes a non-keepalive
@@ -605,8 +592,8 @@ TEST_F(HttpNetworkTransactionTest, NonKeepAliveConnectionEOF) {
 // Test the request-challenge-retry sequence for basic auth.
 // (basic auth is the easiest to mock, because it has no randomness).
 TEST_F(HttpNetworkTransactionTest, BasicAuth) {
-  net::HttpTransaction* trans = new net::HttpNetworkTransaction(
-      CreateSession(), &mock_socket_factory);
+  scoped_ptr<net::HttpTransaction> trans(new net::HttpNetworkTransaction(
+      CreateSession(), &mock_socket_factory));
 
   net::HttpRequestInfo request;
   request.method = "GET";
@@ -683,8 +670,6 @@ TEST_F(HttpNetworkTransactionTest, BasicAuth) {
   EXPECT_FALSE(response == NULL);
   EXPECT_TRUE(response->auth_challenge.get() == NULL);
   EXPECT_EQ(100, response->headers->GetContentLength());
-
-  trans->Destroy();
 }
 
 // Test the flow when both the proxy server AND origin server require
@@ -695,9 +680,9 @@ TEST_F(HttpNetworkTransactionTest, BasicAuthProxyThenServer) {
   proxy_info.UseNamedProxy("myproxy:70");
 
   // Configure against proxy server "myproxy:70".
-  net::HttpTransaction* trans = new net::HttpNetworkTransaction(
+  scoped_ptr<net::HttpTransaction> trans(new net::HttpNetworkTransaction(
       CreateSession(new net::ProxyResolverFixed(proxy_info)),
-      &mock_socket_factory);
+      &mock_socket_factory));
 
   net::HttpRequestInfo request;
   request.method = "GET";
@@ -816,6 +801,4 @@ TEST_F(HttpNetworkTransactionTest, BasicAuthProxyThenServer) {
   response = trans->GetResponseInfo();
   EXPECT_TRUE(response->auth_challenge.get() == NULL);
   EXPECT_EQ(100, response->headers->GetContentLength());
-
-  trans->Destroy();
 }
diff --git a/net/http/http_transaction.h b/net/http/http_transaction.h
index 3697a94..ba2cf29 100644
--- a/net/http/http_transaction.h
+++ b/net/http/http_transaction.h
@@ -18,10 +18,8 @@ class HttpResponseInfo;
 // answered.  Cookies are assumed to be managed by the caller.
 class HttpTransaction {
  public:
-  virtual ~HttpTransaction() {}
-
   // Stops any pending IO and destroys the transaction object.
-  virtual void Destroy() = 0;
+  virtual ~HttpTransaction() {}
 
   // Starts the HTTP transaction (i.e., sends the HTTP request).
   //
diff --git a/net/http/http_transaction_unittest.h b/net/http/http_transaction_unittest.h
index 91006a0..0b8a1c9 100644
--- a/net/http/http_transaction_unittest.h
+++ b/net/http/http_transaction_unittest.h
@@ -103,7 +103,6 @@ class TestTransactionConsumer : public CallbackRunner< Tuple1<int> > {
   }
 
   ~TestTransactionConsumer() {
-    trans_->Destroy();
   }
 
   void Start(const net::HttpRequestInfo* request) {
@@ -175,7 +174,7 @@ class TestTransactionConsumer : public CallbackRunner< Tuple1<int> > {
     DONE
   } state_;
 
-  net::HttpTransaction* trans_;
+  scoped_ptr<net::HttpTransaction> trans_;
   std::string content_;
   char read_buf_[1024];
   int error_;
@@ -196,10 +195,6 @@ class MockNetworkTransaction : public net::HttpTransaction {
       ALLOW_THIS_IN_INITIALIZER_LIST(task_factory_(this)), data_cursor_(0) {
   }
 
-  virtual void Destroy() {
-    delete this;
-  }
-
   virtual int Start(const net::HttpRequestInfo* request,
                     net::CompletionCallback* callback) {
     const MockTransaction* t = FindMockTransaction(request->url);
diff --git a/net/http/http_transaction_winhttp.cc b/net/http/http_transaction_winhttp.cc
index 239d93d..550f32a 100644
--- a/net/http/http_transaction_winhttp.cc
+++ b/net/http/http_transaction_winhttp.cc
@@ -825,10 +825,6 @@ HttpTransactionWinHttp::~HttpTransactionWinHttp() {
     session_->Release();
 }
 
-void HttpTransactionWinHttp::Destroy() {
-  delete this;
-}
-
 int HttpTransactionWinHttp::Start(const HttpRequestInfo* request_info,
                                   CompletionCallback* callback) {
   DCHECK(request_info);
diff --git a/net/http/http_transaction_winhttp.h b/net/http/http_transaction_winhttp.h
index 7168625..122aea3 100644
--- a/net/http/http_transaction_winhttp.h
+++ b/net/http/http_transaction_winhttp.h
@@ -50,8 +50,9 @@ class HttpTransactionWinHttp : public HttpTransaction {
     DISALLOW_EVIL_CONSTRUCTORS(Factory);
   };
 
+  virtual ~HttpTransactionWinHttp();
+
   // HttpTransaction methods:
-  virtual void Destroy();
   virtual int Start(const HttpRequestInfo*, CompletionCallback*);
   virtual int RestartIgnoringLastError(CompletionCallback*);
   virtual int RestartWithAuth(const std::wstring&,
@@ -80,7 +81,6 @@ class HttpTransactionWinHttp : public HttpTransaction {
   // Methods ------------------------------------------------------------------
 
   HttpTransactionWinHttp(Session* session, const ProxyInfo* info);
-  ~HttpTransactionWinHttp();
 
   void DoCallback(int rv);
   int ResolveProxy();
diff --git a/net/http/http_transaction_winhttp_unittest.cc b/net/http/http_transaction_winhttp_unittest.cc
index c71e0ed..cf462e5 100644
--- a/net/http/http_transaction_winhttp_unittest.cc
+++ b/net/http/http_transaction_winhttp_unittest.cc
@@ -9,32 +9,30 @@
 TEST(HttpTransactionWinHttp, CreateAndDestroy) {
   net::HttpTransactionWinHttp::Factory factory;
 
-  net::HttpTransaction* trans = factory.CreateTransaction();
-  trans->Destroy();
+  scoped_ptr<net::HttpTransaction> trans(factory.CreateTransaction());
 }
 
 TEST(HttpTransactionWinHttp, Suspend) {
   net::HttpTransactionWinHttp::Factory factory;
 
-  net::HttpTransaction* trans = factory.CreateTransaction();
-  trans->Destroy();
+  scoped_ptr<net::HttpTransaction> trans(factory.CreateTransaction());
+  trans.reset();
 
   factory.Suspend(true);
 
-  trans = factory.CreateTransaction();
+  trans.reset(factory.CreateTransaction());
   ASSERT_TRUE(trans == NULL);
 
   factory.Suspend(false);
 
-  trans = factory.CreateTransaction();
-  trans->Destroy();
+  trans.reset(factory.CreateTransaction());
 }
 
 TEST(HttpTransactionWinHttp, GoogleGET) {
   net::HttpTransactionWinHttp::Factory factory;
   TestCompletionCallback callback;
 
-  net::HttpTransaction* trans = factory.CreateTransaction();
+  scoped_ptr<net::HttpTransaction> trans(factory.CreateTransaction());
 
   net::HttpRequestInfo request_info;
   request_info.url = GURL("http://www.google.com/");
@@ -48,9 +46,7 @@ TEST(HttpTransactionWinHttp, GoogleGET) {
   EXPECT_EQ(net::OK, rv);
 
   std::string contents;
-  rv = ReadTransaction(trans, &contents);
+  rv = ReadTransaction(trans.get(), &contents);
   EXPECT_EQ(net::OK, rv);
-
-  trans->Destroy();
 }