Change the API of disk_cache::CreateCacheBackend to use scoped_ptr

net/disk_cache/entry_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 "base/basictypes.h"
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/file_util.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
@@ skipping 2007 matching lines @@
   InitCache();
   DoomSparseEntry();
 }
 
 // A CompletionCallback wrapper that deletes the cache from within the callback.
 // The way a CompletionCallback works means that all tasks (even new ones)
 // are executed by the message loop before returning to the caller so the only
 // way to simulate a race is to execute what we want on the callback.
 class SparseTestCompletionCallback: public net::TestCompletionCallback {
  public:
-  explicit SparseTestCompletionCallback(disk_cache::Backend* cache)
+  explicit SparseTestCompletionCallback(scoped_ptr<disk_cache::Backend>* cache)

[rvargas (doing something else), 2013/07/26 21:05:01]

nit: this is kind of ugly :(. The argument suggest transfer of ownership (in
fact as an out argument), but there is no real transfer of ownership even though
this class will delete the cache later on.

Isn't it better to leave this class using plain pointers and make the test do
the ugly dance of coping with the cache being deleted at some point by someone
else?. At least while reading the test what's going on will be explicit.

[qsr, 2013/07/29 08:26:28]

 Given that this is a pointer to the scoped_ptr, and not a scoped_ptr, this is
not really a transfer of ownership... But I'm fine changing this if you prefer.

 Given what this class is doing, everything is ugly. I changed it by really
passing ownership of the backend to this class, and just keeping a pointer to it
beforehand. Is that fine with you?

       : cache_(cache) {
   }
 
  private:
   virtual void SetResult(int result) OVERRIDE {
-    delete cache_;
+    cache_->reset();
     TestCompletionCallback::SetResult(result);
   }
 
-  disk_cache::Backend* cache_;
+  scoped_ptr<disk_cache::Backend>* cache_;
   DISALLOW_COPY_AND_ASSIGN(SparseTestCompletionCallback);
 };
 
 // Tests that we don't crash when the backend is deleted while we are working
 // deleting the sub-entries of a sparse entry.
 TEST_F(DiskCacheEntryTest, DoomSparseEntry2) {
   UseCurrentThread();
   InitCache();
   std::string key("the key");
   disk_cache::Entry* entry;
   ASSERT_EQ(net::OK, CreateEntry(key, &entry));
 
   const int kSize = 4 * 1024;
   scoped_refptr<net::IOBuffer> buf(new net::IOBuffer(kSize));
   CacheTestFillBuffer(buf->data(), kSize, false);
 
   int64 offset = 1024;
   // Write to a bunch of ranges.
   for (int i = 0; i < 12; i++) {
     EXPECT_EQ(kSize,
               entry->WriteSparseData(
                   offset, buf.get(), kSize, net::CompletionCallback()));
     offset *= 4;
   }
   EXPECT_EQ(9, cache_->GetEntryCount());
 
   entry->Close();
-  SparseTestCompletionCallback cb(cache_);
+  SparseTestCompletionCallback cb(&cache_);
   int rv = cache_->DoomEntry(key, cb.callback());
   EXPECT_EQ(net::ERR_IO_PENDING, rv);
   EXPECT_EQ(net::OK, cb.WaitForResult());
-
-  // TearDown will attempt to delete the cache_.
-  cache_ = NULL;
 }
 
 void DiskCacheEntryTest::PartialSparseEntry() {
   std::string key("the first key");
   disk_cache::Entry* entry;
   ASSERT_EQ(net::OK, CreateEntry(key, &entry));
 
   // We should be able to deal with IO that is not aligned to the block size
   // of a sparse entry, at least to write a big range without leaving holes.
   const int kSize = 4 * 1024;
@@ skipping 1316 matching lines @@
 
   // Check that we are not leaking.
   ASSERT_NE(entry, null);
   EXPECT_TRUE(
       static_cast<disk_cache::SimpleEntryImpl*>(entry)->HasOneRef());
   entry->Close();
   entry = NULL;
 }
 
 #endif  // defined(OS_POSIX)