base::Bind() conversion for chrome/browser/search_engines

base::Bind() conversion for chrome/browser/search_engines

BUG=none
TEST=compiles and passes tests


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

[dcheng, 2011-11-16 02:17:05]

No one else uses TaskQueue so I just deleted it from base. I replaced it with 
std::vector<base::Closure>, but I'm not sure if std::deque is preferred here.
Also adding willchan to make sure it's kosher to store base::Closure by value in
a STL container (from my reading of how the new Callback system works, I believe
this is the correct way to do it).

[sky, 2011-11-16 04:42:17]

http://codereview.chromium.org/8570022/diff/4001/chrome/browser/search_engine...
File chrome/browser/search_engines/search_provider_install_data.cc (right):

http://codereview.chromium.org/8570022/diff/4001/chrome/browser/search_engine...
chrome/browser/search_engines/search_provider_install_data.cc:307:
closure_queue_.clear();
I've feel better if you created a vector on the stack, swapped the two, and used
the one on the stack for sending notification. Also, isn't there some for_each
magic that can streamline this?

[willchan no longer on Chromium, 2011-11-16 05:43:26]

base/ LGTM, and yes, std::vector of Closures work fine. There's one subtle
difference though in the change. In the old TaskQueue::Run(), the task would get
deleted after it ran. In your new code, the Closures only get removed from the
vector in the vector.clear() call at the end, after all Closures have been
Run(). Not a huge difference, but it's possible for this to cause behavioral
changes, since when Closures get destroyed, they destroy the bound arguments.

[dcheng, 2011-11-16 10:00:39]

http://codereview.chromium.org/8570022/diff/4001/chrome/browser/search_engine...
File chrome/browser/search_engines/search_provider_install_data.cc (right):

http://codereview.chromium.org/8570022/diff/4001/chrome/browser/search_engine...
chrome/browser/search_engines/search_provider_install_data.cc:307:
closure_queue_.clear();
On 2011/11/16 04:42:17, sky wrote:
> I've feel better if you created a vector on the stack, swapped the two, and
used
> the one on the stack for sending notification. Also, isn't there some for_each
> magic that can streamline this?

Done for both, but any particular reason for preferring swap()? I don't think
it's threadsafe either way without synchronization.

[sky, 2011-11-16 16:40:41]

LGTM

I like the swap approach because you don't have to worry about someone trying to
modify the vector out from under you. Say someone you're notified invokes a
method back on the service that alters the vector. I don't think it's possible,
but better to be safe.

[commit-bot: I haz the power, 2011-11-16 18:47:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dcheng@chromium.org/8570022/6001

[commit-bot: I haz the power, 2011-11-16 19:50:42]

Change committed as 110342