Change the API of disk_cache::CreateCacheBackend to use scoped_ptr

Change the API of disk_cache::CreateCacheBackend to use scoped_ptr

 Using scoped_ptr instead of pointer allows the API to show the transfert of
ownership and ensure that the caller will holds it.

R=rvargas@chromium.org
TBR=bradchen@chromium.org,apatrick@chromium.org,darin@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=215008

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

I've never been a fan of forcing consumers do something on a given way, but I
guess this is the new black so why not.

https://codereview.chromium.org/20737002/diff/26001/net/disk_cache/backend_un...
File net/disk_cache/backend_unittest.cc (right):

https://codereview.chromium.org/20737002/diff/26001/net/disk_cache/backend_un...
net/disk_cache/backend_unittest.cc:59: }
nit: remove {}

https://codereview.chromium.org/20737002/diff/26001/net/disk_cache/backend_un...
net/disk_cache/backend_unittest.cc:524: ASSERT_TRUE(backend.get() == NULL);
nit: false(get())

https://codereview.chromium.org/20737002/diff/26001/net/disk_cache/backend_un...
net/disk_cache/backend_unittest.cc:1762:
cache.reset(static_cast<disk_cache::BackendImpl*>(base_cache.release()));
nit: remove this line

https://codereview.chromium.org/20737002/diff/26001/net/disk_cache/cache_crea...
File net/disk_cache/cache_creator.cc (right):

https://codereview.chromium.org/20737002/diff/26001/net/disk_cache/cache_crea...
net/disk_cache/cache_creator.cc:112: new
disk_cache::TracingCacheBackend(created_cache_.release()));
TracingCacheBackend should probably change to keep up with the new paradigm.

https://codereview.chromium.org/20737002/diff/26001/net/disk_cache/disk_cache...
File net/disk_cache/disk_cache_test_base.cc (right):

https://codereview.chromium.org/20737002/diff/26001/net/disk_cache/disk_cache...
net/disk_cache/disk_cache_test_base.cc:246: ASSERT_TRUE(NULL != cache_.get());
nit: (cache_.get())

https://codereview.chromium.org/20737002/diff/26001/net/disk_cache/disk_cache...
net/disk_cache/disk_cache_test_base.cc:297: ASSERT_TRUE(NULL != cache_.get());
ditto

https://codereview.chromium.org/20737002/diff/26001/net/disk_cache/entry_unit...
File net/disk_cache/entry_unittest.cc (right):

https://codereview.chromium.org/20737002/diff/26001/net/disk_cache/entry_unit...
net/disk_cache/entry_unittest.cc:2028: explicit
SparseTestCompletionCallback(scoped_ptr<disk_cache::Backend>* cache)
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.

https://codereview.chromium.org/20737002/diff/26001/net/http/http_cache.cc
File net/http/http_cache.cc (right):

https://codereview.chromium.org/20737002/diff/26001/net/http/http_cache.cc#ne...
net/http/http_cache.cc:1122: disk_cache_ = backend.Pass();
pending_op->backend.Pass() and remove |backend|

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

https://codereview.chromium.org/20737002/diff/26001/net/disk_cache/backend_un...
File net/disk_cache/backend_unittest.cc (right):

https://codereview.chromium.org/20737002/diff/26001/net/disk_cache/backend_un...
net/disk_cache/backend_unittest.cc:59: }
On 2013/07/26 21:05:01, rvargas wrote:
> nit: remove {}

Done.

https://codereview.chromium.org/20737002/diff/26001/net/disk_cache/backend_un...
net/disk_cache/backend_unittest.cc:524: ASSERT_TRUE(backend.get() == NULL);
On 2013/07/26 21:05:01, rvargas wrote:
> nit: false(get())

Done.

https://codereview.chromium.org/20737002/diff/26001/net/disk_cache/backend_un...
net/disk_cache/backend_unittest.cc:1762:
cache.reset(static_cast<disk_cache::BackendImpl*>(base_cache.release()));
On 2013/07/26 21:05:01, rvargas wrote:
> nit: remove this line

Done.

https://codereview.chromium.org/20737002/diff/26001/net/disk_cache/cache_crea...
File net/disk_cache/cache_creator.cc (right):

https://codereview.chromium.org/20737002/diff/26001/net/disk_cache/cache_crea...
net/disk_cache/cache_creator.cc:112: new
disk_cache::TracingCacheBackend(created_cache_.release()));
On 2013/07/26 21:05:01, rvargas wrote:
> TracingCacheBackend should probably change to keep up with the new paradigm.

Done.

https://codereview.chromium.org/20737002/diff/26001/net/disk_cache/disk_cache...
File net/disk_cache/disk_cache_test_base.cc (right):

https://codereview.chromium.org/20737002/diff/26001/net/disk_cache/disk_cache...
net/disk_cache/disk_cache_test_base.cc:246: ASSERT_TRUE(NULL != cache_.get());
On 2013/07/26 21:05:01, rvargas wrote:
> nit: (cache_.get())

Done.

https://codereview.chromium.org/20737002/diff/26001/net/disk_cache/disk_cache...
net/disk_cache/disk_cache_test_base.cc:297: ASSERT_TRUE(NULL != cache_.get());
On 2013/07/26 21:05:01, rvargas wrote:
> ditto

Done.

https://codereview.chromium.org/20737002/diff/26001/net/disk_cache/entry_unit...
File net/disk_cache/entry_unittest.cc (right):

https://codereview.chromium.org/20737002/diff/26001/net/disk_cache/entry_unit...
net/disk_cache/entry_unittest.cc:2028: explicit
SparseTestCompletionCallback(scoped_ptr<disk_cache::Backend>* cache)
 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?

https://codereview.chromium.org/20737002/diff/26001/net/http/http_cache.cc
File net/http/http_cache.cc (right):

https://codereview.chromium.org/20737002/diff/26001/net/http/http_cache.cc#ne...
net/http/http_cache.cc:1122: disk_cache_ = backend.Pass();
On 2013/07/26 21:05:01, rvargas wrote:
> pending_op->backend.Pass() and remove |backend|

Done.

[rvargas (doing something else), 2013-07-29 23:57:27]

LGTM, Thanks.

[qsr, 2013-07-30 07:17:10]

+darin for OWNERS

[qsr, 2013-07-31 09:05:45]

On 2013/07/30 07:17:10, qsr wrote:
> +darin for OWNERS

 Darin: gentle ping?

[qsr, 2013-08-01 07:58:07]

+bradchen@chromium.org for chrome/browser/nacl_host
+apatrick@chromium.org for chrome/browser/gpu

[commit-bot: I haz the power, 2013-08-01 07:58:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/qsr@chromium.org/20737002/46001

[commit-bot: I haz the power, 2013-08-01 08:21:27]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_clang.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_clan...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-08-01 08:29:46]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/qsr@chromium.org/20737002/107002

[commit-bot: I haz the power, 2013-08-01 11:57:58]

Change committed as 215008