Cleanup AppCacheQuotaClient using new Callback

Cleanup AppCacheQuotaClient using new Callback

BUG=None
TEST=AppCacheQuotaClientTest.PendingRequests


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

[tzik, 2011-10-11 13:13:13]

Hi.

This is another pass of Quota related callback refactoring.
(http://codereview.chromium.org/8070001/ .)
In this CL, I'm trying to replace {Usage,Origins,Delete}Request with callback as
awong suggests.

I believe this CL will not grow so huge.
Could you review this CL?

[michaeln, 2011-10-11 19:18:40]

this lgtm, although can you explain why the first snapshot didn't work, how it
introduced recursion that isn't seen in the current impl?

[tzik, 2011-10-12 01:58:23]

Sure.

http://codereview.chromium.org/8230012/diff/5/webkit/appcache/appcache_quota_...
File webkit/appcache/appcache_quota_client.cc (left):

http://codereview.chromium.org/8230012/diff/5/webkit/appcache/appcache_quota_...
webkit/appcache/appcache_quota_client.cc:162: service_delete_callback_);
In current implementation, we call service->DeleteAppCachesForOrigin() directly.
It causes delayed callback call.
# Actually, we should check storagy type and should not delete origin if storage
type is persistent.
OTOH in this CL, we call DeleteOriginData() here.
It calls the DidDeleteAppCacheForOrigin() immediately if the type is persistent.
In my first patchset, we didn't pop delete_request from pending_delete_requests_
here,
so re-entering DidDeleteAppCacheForOrigin() causes multiple processing for a
request.

[michaeln, 2011-10-12 06:36:54]

http://codereview.chromium.org/8230012/diff/5/webkit/appcache/appcache_quota_...
File webkit/appcache/appcache_quota_client.cc (left):

http://codereview.chromium.org/8230012/diff/5/webkit/appcache/appcache_quota_...
webkit/appcache/appcache_quota_client.cc:162: service_delete_callback_);
On 2011/10/12 01:58:23, tzik wrote:
> In current implementation, we call service->DeleteAppCachesForOrigin()
directly...

Got it... so this also fixes a logic bug!

[awong, 2011-10-12 07:28:06]

LGTM for the callback usage.

As for the logic change on quota delete, I think I understand it and that it
looks right.  But am only 80% certain I understand the code, so I'll leave it
that to you two. :)

[tzik, 2011-10-12 14:59:41]

Thanks a lot.
If no problem, I'll put it in CQ after 8070001 lands.

[michaeln, 2011-10-12 18:46:12]

yup... lgtm... thnx!

[commit-bot: I haz the power, 2011-10-19 03:35:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tzik@chromium.org/8230012/12001

[commit-bot: I haz the power, 2011-10-19 05:40:46]

Change committed as 106233