Http cache: It turns out that the cache destructor...

Http cache: It turns out that the cache destructor
may be called from within one of the notifications
of "backend construction done". This CL makes sure
that this scenario is handled properly by invoking
the callbacks one at a time, posting a task before
moving on to the next one.

BUG=52090
TEST=netunittests


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=56964

[eroman, 2010-08-20 18:15:23]

lgtm

http://codereview.chromium.org/3173030/diff/1/2
File net/http/http_cache.cc (right):

http://codereview.chromium.org/3173030/diff/1/2#newcode1020
net/http/http_cache.cc:1020: if (backend_factory_.get()) {
Can you add a comment explaining this check is because we re-call
OnBackendCreated() for each pending operation?

http://codereview.chromium.org/3173030/diff/1/2#newcode1038
net/http/http_cache.cc:1038: result, pending_op));
Won't this leak pending_op if HttpCache gets deleted before the callback is run?

[rvargas (doing something else), 2010-08-20 18:47:38]

Thanks.

http://codereview.chromium.org/3173030/diff/1/2
File net/http/http_cache.cc (right):

http://codereview.chromium.org/3173030/diff/1/2#newcode1020
net/http/http_cache.cc:1020: if (backend_factory_.get()) {
On 2010/08/20 18:15:23, eroman wrote:
> Can you add a comment explaining this check is because we re-call
> OnBackendCreated() for each pending operation?

Added:
// We may end up calling OnBackendCreated multiple times if we have pending
// work items. The first call saves the backend and releases the factory,
// and the last call clears building_backend_.

http://codereview.chromium.org/3173030/diff/1/2#newcode1038
net/http/http_cache.cc:1038: result, pending_op));
On 2010/08/20 18:15:23, eroman wrote:
> Won't this leak pending_op if HttpCache gets deleted before the callback is
run?

No, it doesn't. pending_op is part of a list that is deleted by the destructor
(DeletePendingOp removes the operation from that list).

[eroman, 2010-08-20 18:56:37]

http://codereview.chromium.org/3173030/diff/1/2
File net/http/http_cache.cc (right):

http://codereview.chromium.org/3173030/diff/1/2#newcode1038
net/http/http_cache.cc:1038: result, pending_op));
On 2010/08/20 18:47:38, rvargas wrote:
> On 2010/08/20 18:15:23, eroman wrote:
> > Won't this leak pending_op if HttpCache gets deleted before the callback is
> run?
> 
> No, it doesn't. pending_op is part of a list that is deleted by the destructor
> (DeletePendingOp removes the operation from that list).

Ah my bad. LGTM