base::Bind usage

base::Bind: Cleanups in webkit/appcache
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=107812

[michaeln, 2011-10-27 00:13:00]

ptal

[James Hawkins, 2011-10-27 03:30:32]

FYI, please make sure to check the spreadsheet at [1] and ask current assigness
if they have CLs already prepared.  I thankfully didn't have anything for this
yet, but it was next on my list :-/

[1]
https://docs.google.com/a/chromium.org/spreadsheet/ccc?key=0AlcDRdDbqkaadEFXQ...

http://codereview.chromium.org/8399019/diff/9/webkit/appcache/appcache_respon...
File webkit/appcache/appcache_response.cc (right):

http://codereview.chromium.org/8399019/diff/9/webkit/appcache/appcache_respon...
webkit/appcache/appcache_response.cc:92:
MessageLoop::current()->PostTask(FROM_HERE,
nit: Parameters must start on the same column.  In general, I've been doing:

PostTask(
    FROM_HERE, blahblah,
    base::Bind(...));

http://codereview.chromium.org/8399019/diff/9/webkit/appcache/appcache_servic...
File webkit/appcache/appcache_service.cc (right):

http://codereview.chromium.org/8399019/diff/9/webkit/appcache/appcache_servic...
webkit/appcache/appcache_service.cc:54: base::Bind(&DeferredCallCallback,
callback_, rv));
Nit: Looks like we can save a line and put this parameter on the previous line.

http://codereview.chromium.org/8399019/diff/9/webkit/appcache/appcache_servic...
webkit/appcache/appcache_service.cc:59: static void
DeferredCallCallback(net::OldCompletionCallback* callback, int rv) {
Nit: This was wrong from before, but this breaks 80 cols.

http://codereview.chromium.org/8399019/diff/9/webkit/appcache/appcache_servic...
File webkit/appcache/appcache_service_unittest.cc (right):

http://codereview.chromium.org/8399019/diff/9/webkit/appcache/appcache_servic...
webkit/appcache/appcache_service_unittest.cc:71: weak_factory_.GetWeakPtr(),
result));
Nit: For both function calls, line up parameters on the same column (per-call).

[James Hawkins, 2011-10-27 03:31:06]

Please expand the CL description to be more descriptive.  Including a summary is
ideal.

[michaeln, 2011-10-27 19:20:01]

http://codereview.chromium.org/8399019/diff/9/webkit/appcache/appcache_respon...
File webkit/appcache/appcache_response.cc (right):

http://codereview.chromium.org/8399019/diff/9/webkit/appcache/appcache_respon...
webkit/appcache/appcache_response.cc:92:
MessageLoop::current()->PostTask(FROM_HERE,
On 2011/10/27 03:30:32, James Hawkins wrote:
> nit: Parameters must start on the same column.  In general, I've been doing:
> 
> PostTask(
>     FROM_HERE, blahblah,
>     base::Bind(...));

Done.

http://codereview.chromium.org/8399019/diff/9/webkit/appcache/appcache_servic...
File webkit/appcache/appcache_service.cc (right):

http://codereview.chromium.org/8399019/diff/9/webkit/appcache/appcache_servic...
webkit/appcache/appcache_service.cc:54: base::Bind(&DeferredCallCallback,
callback_, rv));
On 2011/10/27 03:30:32, James Hawkins wrote:
> Nit: Looks like we can save a line and put this parameter on the previous
line.

Done.

http://codereview.chromium.org/8399019/diff/9/webkit/appcache/appcache_servic...
webkit/appcache/appcache_service.cc:59: static void
DeferredCallCallback(net::OldCompletionCallback* callback, int rv) {
On 2011/10/27 03:30:32, James Hawkins wrote:
> Nit: This was wrong from before, but this breaks 80 cols.

Done.

http://codereview.chromium.org/8399019/diff/9/webkit/appcache/appcache_servic...
File webkit/appcache/appcache_service_unittest.cc (right):

http://codereview.chromium.org/8399019/diff/9/webkit/appcache/appcache_servic...
webkit/appcache/appcache_service_unittest.cc:71: weak_factory_.GetWeakPtr(),
result));
On 2011/10/27 03:30:32, James Hawkins wrote:
> Nit: For both function calls, line up parameters on the same column
(per-call).

Done.

[michaeln, 2011-10-27 21:09:08]

> FYI, please make sure to check the spreadsheet at [1] and ask current
assigness
> if they have CLs already prepared.  I thankfully didn't have anything for this
> yet, but it was next on my list :-/

Sorry, i didn't think to check the spreadsheet i didn't see any CLs pending for
it so i went for it.

[James Hawkins, 2011-10-27 21:40:43]

No worries, it worked out this time :-)

LGTM