Converted remaining non-factory NewRunnable* calls to use base::Bind

Converted remaining non-factory NewRunnable* calls to use base::Bind

BUG=105561
TEST=unit_tests


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

[Rick Campbell, 2011-11-28 17:59:09]

Please take a look

[rlarocque, 2011-11-28 19:51:02]

I don't know enough about the differences in memory management between the
NewRunnableMethod and Bind systems to comment on the threading issues that may
have been introduced by this change.  

If there is no difference, then I could approve this change once my comment has
been addressed.  Otherwise I'd prefer to wait for Fred's review.

http://codereview.chromium.org/8726001/diff/1/chrome/browser/sync/glue/non_fr...
File chrome/browser/sync/glue/non_frontend_data_type_controller_unittest.cc
(right):

http://codereview.chromium.org/8726001/diff/1/chrome/browser/sync/glue/non_fr...
chrome/browser/sync/glue/non_frontend_data_type_controller_unittest.cc:388:
non_frontend_dtc_.get());
non_frontend_dtc_.get() is the "this" argument, right?  Shouldn't it be the
second argument to Bind, rather than the last?

[akalin, 2011-11-28 20:28:38]

I think the classes changed are ref-counted so this change is okay in general

http://codereview.chromium.org/8726001/diff/1/chrome/browser/sync/glue/http_b...
File chrome/browser/sync/glue/http_bridge_unittest.cc (right):

http://codereview.chromium.org/8726001/diff/1/chrome/browser/sync/glue/http_b...
chrome/browser/sync/glue/http_bridge_unittest.cc:140: MessageLoop::current(),
this));
args should be swapped here too, no?

http://codereview.chromium.org/8726001/diff/1/chrome/browser/sync/glue/non_fr...
File chrome/browser/sync/glue/non_frontend_data_type_controller_unittest.cc
(right):

http://codereview.chromium.org/8726001/diff/1/chrome/browser/sync/glue/non_fr...
chrome/browser/sync/glue/non_frontend_data_type_controller_unittest.cc:388:
non_frontend_dtc_.get());
On 2011/11/28 19:51:03, rlarocque wrote:
> non_frontend_dtc_.get() is the "this" argument, right?  Shouldn't it be the
> second argument to Bind, rather than the last?  

Yeah, how did this compile?

[Rick Campbell, 2011-11-29 17:26:13]

Please take another look

http://codereview.chromium.org/8726001/diff/1/chrome/browser/sync/glue/http_b...
File chrome/browser/sync/glue/http_bridge_unittest.cc (right):

http://codereview.chromium.org/8726001/diff/1/chrome/browser/sync/glue/http_b...
chrome/browser/sync/glue/http_bridge_unittest.cc:140: MessageLoop::current(),
this));
On 2011/11/28 20:28:39, akalin wrote:
> args should be swapped here too, no?

Don't think so.  Generally NRM needs to move args, NRF does not.

http://codereview.chromium.org/8726001/diff/1/chrome/browser/sync/glue/non_fr...
File chrome/browser/sync/glue/non_frontend_data_type_controller_unittest.cc
(right):

http://codereview.chromium.org/8726001/diff/1/chrome/browser/sync/glue/non_fr...
chrome/browser/sync/glue/non_frontend_data_type_controller_unittest.cc:388:
non_frontend_dtc_.get());
On 2011/11/28 20:28:39, akalin wrote:
> On 2011/11/28 19:51:03, rlarocque wrote:
> > non_frontend_dtc_.get() is the "this" argument, right?  Shouldn't it be the
> > second argument to Bind, rather than the last?  
> 
> Yeah, how did this compile?

Yeah, it didn't.  It was a one-last-change thing, sigh.  Fixed.

[akalin, 2011-11-29 17:35:01]

LGTM

http://codereview.chromium.org/8726001/diff/2002/chrome/browser/sync/glue/htt...
File chrome/browser/sync/glue/http_bridge_unittest.cc (right):

http://codereview.chromium.org/8726001/diff/2002/chrome/browser/sync/glue/htt...
chrome/browser/sync/glue/http_bridge_unittest.cc:282: &HttpBridgeTest::Abort,
http_bridge));
line up with (

http://codereview.chromium.org/8726001/diff/2002/chrome/browser/sync/glue/non...
File chrome/browser/sync/glue/non_frontend_data_type_controller_unittest.cc
(right):

http://codereview.chromium.org/8726001/diff/2002/chrome/browser/sync/glue/non...
chrome/browser/sync/glue/non_frontend_data_type_controller_unittest.cc:77:
this));
this might now fit on previous line

http://codereview.chromium.org/8726001/diff/2002/chrome/browser/sync/glue/non...
chrome/browser/sync/glue/non_frontend_data_type_controller_unittest.cc:84:
this));
this might now fit on previous line

http://codereview.chromium.org/8726001/diff/2002/chrome/browser/sync/glue/non...
chrome/browser/sync/glue/non_frontend_data_type_controller_unittest.cc:184:
&done));
this might now fit on previous line

[Rick Campbell, 2011-11-30 18:37:22]

Nits fixed.  Committing.  Thanks!

http://codereview.chromium.org/8726001/diff/2002/chrome/browser/sync/glue/htt...
File chrome/browser/sync/glue/http_bridge_unittest.cc (right):

http://codereview.chromium.org/8726001/diff/2002/chrome/browser/sync/glue/htt...
chrome/browser/sync/glue/http_bridge_unittest.cc:282: &HttpBridgeTest::Abort,
http_bridge));
On 2011/11/29 17:35:01, akalin wrote:
> line up with (

Done.  Also put Bind call all on same line.

http://codereview.chromium.org/8726001/diff/2002/chrome/browser/sync/glue/non...
File chrome/browser/sync/glue/non_frontend_data_type_controller_unittest.cc
(right):

http://codereview.chromium.org/8726001/diff/2002/chrome/browser/sync/glue/non...
chrome/browser/sync/glue/non_frontend_data_type_controller_unittest.cc:77:
this));
On 2011/11/29 17:35:01, akalin wrote:
> this might now fit on previous line

Done.

http://codereview.chromium.org/8726001/diff/2002/chrome/browser/sync/glue/non...
chrome/browser/sync/glue/non_frontend_data_type_controller_unittest.cc:84:
this));
On 2011/11/29 17:35:01, akalin wrote:
> this might now fit on previous line

Done.

http://codereview.chromium.org/8726001/diff/2002/chrome/browser/sync/glue/non...
chrome/browser/sync/glue/non_frontend_data_type_controller_unittest.cc:184:
&done));
On 2011/11/29 17:35:01, akalin wrote:
> this might now fit on previous line

Done.

[commit-bot: I haz the power, 2011-11-30 18:37:46]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rickcam@chromium.org/8726001/2002

[commit-bot: I haz the power, 2011-11-30 18:40:20]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rickcam@chromium.org/8726001/9001

[commit-bot: I haz the power, 2011-11-30 19:57:07]

Change committed as 112270