Do not leak when iterator outlives disk cache backend.

Do not leak when iterator outlives disk cache backend.

The blockfile cache was previously allocating a Rankings::Iterator
object in its enumeration code which was not being freed if the
backend was destroyed before the iterator. This change ensures
that no memory is leaked if an iterator outlives its backend.

R=rvargas@chromium.org,pasko@chromium.org
BUG=413644, 416059

[gavinp, 2014-09-19 18:18:33]

It turns out that the blockfile cache has the same bug (or design/API issue)
that caused Josh sadness earlier. This patch, which is downstream of the reland
of the earlier disk cache work, attempts to fix that.

I tried hard to avoid the temptation to refactor the code as I made these
changes; I did replace most of the void* and void** with the existing
Ranking::Iterator type.

[gavinp, 2014-09-19 18:18:49]

On 2014/09/19 18:18:33, gavinp wrote:
> It turns out that the blockfile cache has the same bug (or design/API issue)
> that caused Josh sadness earlier. This patch, which is downstream of the
reland
> of the earlier disk cache work, attempts to fix that.
> 
> I tried hard to avoid the temptation to refactor the code as I made these
> changes; I did replace most of the void* and void** with the existing
> Ranking::Iterator type.

And here's a link to the upstream patch:
https://codereview.chromium.org/583283002/

[rvargas (doing something else), 2014-09-20 01:41:58]

lgtm

I should have mentioned before that this backend was not freeing that memory
either.

https://codereview.chromium.org/583293002/diff/20001/net/disk_cache/blockfile...
File net/disk_cache/blockfile/backend_impl.cc (right):

https://codereview.chromium.org/583293002/diff/20001/net/disk_cache/blockfile...
net/disk_cache/blockfile/backend_impl.cc:1259:
background_queue_->EndEnumeration(iterator_.Pass());
Technically, all that was needed was iterator_.release() here and the part to
eliminate the iterator destructor.

[gavinp, 2014-09-22 16:43:27]

Thanks for the review. I'm closing this issue, because I've combined this code
with https://codereview.chromium.org/583283002/ (the earlier CL), since it was
reverted because of the bug this fixes.

The combined issue is https://codereview.chromium.org/588243002/ , and I'll land
it TBR you.

https://codereview.chromium.org/583293002/diff/20001/net/disk_cache/blockfile...
File net/disk_cache/blockfile/backend_impl.cc (right):

https://codereview.chromium.org/583293002/diff/20001/net/disk_cache/blockfile...
net/disk_cache/blockfile/backend_impl.cc:1259:
background_queue_->EndEnumeration(iterator_.Pass());
On 2014/09/20 01:41:58, rvargas wrote:
> Technically, all that was needed was iterator_.release() here and the part to
> eliminate the iterator destructor.

Absolutely. The rest of the changes amount to adding type safety to moving the
void* around.