Rewrite base::Bind to base::BindOnce with base_bind_rewriters in //chrome/browser/ui

Rewrite base::Bind to base::BindOnce with base_bind_rewriters in //chrome/browser/ui

This is a generated CL by a clang refactoring tool at //tools/clang/base_bind_rewriters.
The tool rewrites base::Bind to base::BindOnce where the resulting Callback
is immediately converted to OnceCallback.
E.g.:
  base::PostTask(FROM_HERE, base::Bind(&Foo));
is rewritten to:
  base::PostTask(FROM_HERE, base::BindOnce(&Foo));

In terms of the behavior change after the rewrite, the OnceCallback
implementation moves out the bound argument to the target function, unlike
Callback.

BUG=554299
Review-Url: https://codereview.chromium.org/2824363002
Cr-Commit-Position: refs/heads/master@{#466254}
Committed: https://chromium.googlesource.com/chromium/src/+/22036ccf431c0d88976766f309d098448d325f31

[tzik, 2017-04-19 06:52:50]

The CQ bit was checked by tzik@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-19 06:53:03]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-19 07:35:52]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-19 07:35:53]

Dry run: This issue passed the CQ dry run.

[tzik, 2017-04-19 13:29:08]

tzik@chromium.org changed reviewers:
+ msw@chromium.org

[tzik, 2017-04-19 13:29:09]

PTAL

[msw, 2017-04-19 17:38:12]

A couple questions:

(1) How do we know that these specific bound callbacks are never copied or run
more than once? (does the tool somehow discern that? am I, as the reviewer,
supposed to know or determine that for each changed bind call? or are those
cases just supposed to be caught by errors/crashes in testing/production?)

(2) Is there something I should be looking for in this CL beyond checking that
there aren't any glaring errors in the Bind->BindOnce changes? (do you have any
tips for reviewers in these CLs? how thorough should they be? are you just
looking for spot checking and a rubber stamp? it'd be nice to include some
instructions like that in the copy-pasta CL description)

[tzik, 2017-04-20 04:17:16]

> A couple questions:
> 
> (1) How do we know that these specific bound callbacks are never copied or run
> more than once? (does the tool somehow discern that? am I, as the reviewer,
> supposed to know or determine that for each changed bind call? or are those
> cases just supposed to be caught by errors/crashes in testing/production?)

As it's a move-only type, OnceCallback is not copied if the code compiles
successfully.
We have no compile time check to ensure the callback runs only once.
Use-after-move causes a null pointer access crash, or a CHECK failure if it's
posted via a PostTask.
So we have to check manually that it's not used after passed via std::move().
However, for this CL, I just changed temporary objects that are discarded soon,
so there should be no additional use-after-move.

> 
> (2) Is there something I should be looking for in this CL beyond checking that
> there aren't any glaring errors in the Bind->BindOnce changes? (do you have
any
> tips for reviewers in these CLs? how thorough should they be? are you just
> looking for spot checking and a rubber stamp? it'd be nice to include some
> instructions like that in the copy-pasta CL description)

I believe just a sanity check is enough. It has mostly no behavior change.
A subtle behavior change is: If you bind a function whose parameter is
pass-by-value, the parameter was constructed by copy in the old code, while in
the new code, it will be constructed by move. Though I think there's no code
that depends on the difference.

[msw, 2017-04-20 18:43:30]

partly-stampy lgtm

[tzik, 2017-04-21 03:22:14]

The CQ bit was checked by tzik@chromium.org

[commit-bot: I haz the power, 2017-04-21 03:22:34]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-21 04:08:21]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1492744934474200, "parent_rev":
"5fdf6911aa0ae645b60fdbf58e501d550f86fbba", "commit_rev":
"22036ccf431c0d88976766f309d098448d325f31"}

[commit-bot: I haz the power, 2017-04-21 04:09:22]

Description was changed from

==========
Rewrite base::Bind to base::BindOnce with base_bind_rewriters in
//chrome/browser/ui

This is a generated CL by a clang refactoring tool at
//tools/clang/base_bind_rewriters.
The tool rewrites base::Bind to base::BindOnce where the resulting Callback
is immediately converted to OnceCallback.
E.g.:
  base::PostTask(FROM_HERE, base::Bind(&Foo));
is rewritten to:
  base::PostTask(FROM_HERE, base::BindOnce(&Foo));

In terms of the behavior change after the rewrite, the OnceCallback
implementation moves out the bound argument to the target function, unlike
Callback.

BUG=554299
==========

to

==========
Rewrite base::Bind to base::BindOnce with base_bind_rewriters in
//chrome/browser/ui

This is a generated CL by a clang refactoring tool at
//tools/clang/base_bind_rewriters.
The tool rewrites base::Bind to base::BindOnce where the resulting Callback
is immediately converted to OnceCallback.
E.g.:
  base::PostTask(FROM_HERE, base::Bind(&Foo));
is rewritten to:
  base::PostTask(FROM_HERE, base::BindOnce(&Foo));

In terms of the behavior change after the rewrite, the OnceCallback
implementation moves out the bound argument to the target function, unlike
Callback.

BUG=554299

Review-Url: https://codereview.chromium.org/2824363002
Cr-Commit-Position: refs/heads/master@{#466254}
Committed:
https://chromium.googlesource.com/chromium/src/+/22036ccf431c0d88976766f309d0...
==========

[commit-bot: I haz the power, 2017-04-21 04:09:23]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/22036ccf431c0d88976766f309d0...