Use OnceCallback on Mojo interfaces in //ash

Use OnceCallback on Mojo interfaces in //ash
    
This CL flips `use_once_calback` flag on the Mojo code generator, and
fixes all compile errors after it. After this CL, Mojo interfaces in
//ash starts using base::OnceCallback instead of base::Callback on its
return value handling.

The migration recipe was:
 - Convert pass-by-ref callback objects to pass-by-value.
 - Use std::move() to forward it to other consumer, or to invoke it
   with Callback::Run().
 - Handle wherever copies are required manually.
 - Check if the conversion doesn't change the semantics. As the transfer
   and invocation clobber the callback object, care about use-after-move.
   It's considered safe to consume almost scoped-out callback.

Specifically to //ash, the change in //ash/test/test_session_state_animator.cc
is non-trivial. This CL adds a BarrierClosure here.

BUG=714018
Review-Url: https://codereview.chromium.org/2867673004
Cr-Commit-Position: refs/heads/master@{#470902}
Committed: https://chromium.googlesource.com/chromium/src/+/5788c3d4d26446572f032baa70ba82fdf39e789d

[tzik, 2017-05-08 06:39:17]

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

[commit-bot: I haz the power, 2017-05-08 06:39:51]

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-05-08 08:51:52]

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

[commit-bot: I haz the power, 2017-05-08 08:51:54]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[tzik, 2017-05-09 05:28:24]

Description was changed from

==========
Use OnceCallback on Mojo interfaces in //ash
==========

to

==========
Use OnceCallback on Mojo interfaces in //ash
    
This CL flips `use_once_calback` flag on the Mojo code generator, and
fixes all compile errors after it. After this CL, Mojo interfaces in
//ash starts using base::OnceCallback instead of base::Callback on its
return value handling.

The migration recipe was:
 - Convert pass-by-ref callback objects to pass-by-value.
 - Use std::move() to forward it to other consumer, or to invoke it
   with Callback::Run().
 - Handle wherever copies are required manually.
 - Check if the conversion doesn't change the semantics. As the transfer
   and invocation clobber the callback object, care about use-after-move.
   It's considered safe to consume almost scoped-out callback.

Specifically to //ash, the change in //ash/test/test_session_state_animator.cc
is non-trivial. This CL adds a BarrierClosure here.
==========

[tzik, 2017-05-09 05:33:56]

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

[commit-bot: I haz the power, 2017-05-09 05:34:32]

Dry run: CQ is trying da patch.

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

[tzik, 2017-05-09 05:36:15]

Description was changed from

==========
Use OnceCallback on Mojo interfaces in //ash
    
This CL flips `use_once_calback` flag on the Mojo code generator, and
fixes all compile errors after it. After this CL, Mojo interfaces in
//ash starts using base::OnceCallback instead of base::Callback on its
return value handling.

The migration recipe was:
 - Convert pass-by-ref callback objects to pass-by-value.
 - Use std::move() to forward it to other consumer, or to invoke it
   with Callback::Run().
 - Handle wherever copies are required manually.
 - Check if the conversion doesn't change the semantics. As the transfer
   and invocation clobber the callback object, care about use-after-move.
   It's considered safe to consume almost scoped-out callback.

Specifically to //ash, the change in //ash/test/test_session_state_animator.cc
is non-trivial. This CL adds a BarrierClosure here.
==========

to

==========
Use OnceCallback on Mojo interfaces in //ash
    
This CL flips `use_once_calback` flag on the Mojo code generator, and
fixes all compile errors after it. After this CL, Mojo interfaces in
//ash starts using base::OnceCallback instead of base::Callback on its
return value handling.

The migration recipe was:
 - Convert pass-by-ref callback objects to pass-by-value.
 - Use std::move() to forward it to other consumer, or to invoke it
   with Callback::Run().
 - Handle wherever copies are required manually.
 - Check if the conversion doesn't change the semantics. As the transfer
   and invocation clobber the callback object, care about use-after-move.
   It's considered safe to consume almost scoped-out callback.

Specifically to //ash, the change in //ash/test/test_session_state_animator.cc
is non-trivial. This CL adds a BarrierClosure here.

BUG=714018
==========

[tzik, 2017-05-09 05:36:15]

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

[tzik, 2017-05-09 05:36:33]

PTAL

[oshima, 2017-05-09 06:15:31]

oshima@chromium.org changed reviewers:
+ jamescook@chromium.org

[oshima, 2017-05-09 06:15:31]

-> jamescook@

[commit-bot: I haz the power, 2017-05-09 08:09:56]

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

[commit-bot: I haz the power, 2017-05-09 08:09:57]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[James Cook, 2017-05-09 18:30:41]

LGTM with nits

https://codereview.chromium.org/2867673004/diff/20001/ash/shelf/app_list_shel...
File ash/shelf/app_list_shelf_item_delegate.cc (right):

https://codereview.chromium.org/2867673004/diff/20001/ash/shelf/app_list_shel...
ash/shelf/app_list_shelf_item_delegate.cc:51:
std::move(callback).Run(SHELF_ACTION_APP_LIST_SHOWN, base::nullopt);
Aside: I know this decision has already been made, but I find the fact that you
have to std::move(callback) to run it to be difficult to read.

However, I'm glad to see there's a nice static assert telling you what to do
when you accidentally type callback.Run() without std::move(). Kudos to whoever
added that.

https://codereview.chromium.org/2867673004/diff/20001/ash/test/test_session_s...
File ash/test/test_session_state_animator.cc (right):

https://codereview.chromium.org/2867673004/diff/20001/ash/test/test_session_s...
ash/test/test_session_state_animator.cc:219: int count = 0;
nit: |container_count| or similar

https://codereview.chromium.org/2867673004/diff/20001/ash/test/test_session_s...
ash/test/test_session_state_animator.cc:220: for (size_t i = 0; i <
arraysize(kAllContainers); ++i)
nit: {}

[tzik, 2017-05-11 08:08:01]

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

[commit-bot: I haz the power, 2017-05-11 08:08:36]

Dry run: CQ is trying da patch.

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

[tzik, 2017-05-11 08:15:21]

https://codereview.chromium.org/2867673004/diff/20001/ash/shelf/app_list_shel...
File ash/shelf/app_list_shelf_item_delegate.cc (right):

https://codereview.chromium.org/2867673004/diff/20001/ash/shelf/app_list_shel...
ash/shelf/app_list_shelf_item_delegate.cc:51:
std::move(callback).Run(SHELF_ACTION_APP_LIST_SHOWN, base::nullopt);
On 2017/05/09 18:30:40, James Cook (sheriff - slow) wrote:
> Aside: I know this decision has already been made, but I find the fact that
you
> have to std::move(callback) to run it to be difficult to read.
> 
> However, I'm glad to see there's a nice static assert telling you what to do
> when you accidentally type callback.Run() without std::move(). Kudos to
whoever
> added that.

Yes, it's very helpful. Kudos goes to dcheng.

https://codereview.chromium.org/2867673004/diff/20001/ash/test/test_session_s...
File ash/test/test_session_state_animator.cc (right):

https://codereview.chromium.org/2867673004/diff/20001/ash/test/test_session_s...
ash/test/test_session_state_animator.cc:219: int count = 0;
On 2017/05/09 18:30:40, James Cook (sheriff - slow) wrote:
> nit: |container_count| or similar

Done.

https://codereview.chromium.org/2867673004/diff/20001/ash/test/test_session_s...
ash/test/test_session_state_animator.cc:220: for (size_t i = 0; i <
arraysize(kAllContainers); ++i)
On 2017/05/09 18:30:40, James Cook (sheriff - slow) wrote:
> nit: {}

Done.

[tzik, 2017-05-11 08:15:32]

The CQ bit was checked by tzik@chromium.org

[tzik, 2017-05-11 08:15:33]

The patchset sent to the CQ was uploaded after l-g-t-m from
jamescook@chromium.org
Link to the patchset: https://codereview.chromium.org/2867673004/#ps60001
(title: "count -> container_count")

[commit-bot: I haz the power, 2017-05-11 08:15:57]

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-05-11 10:24:16]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1494490532837860,
"parent_rev": "2f57af188a5c2c4ac67aa6ed198a37e39cb7f6d6", "commit_rev":
"5788c3d4d26446572f032baa70ba82fdf39e789d"}

[commit-bot: I haz the power, 2017-05-11 10:24:29]

Description was changed from

==========
Use OnceCallback on Mojo interfaces in //ash
    
This CL flips `use_once_calback` flag on the Mojo code generator, and
fixes all compile errors after it. After this CL, Mojo interfaces in
//ash starts using base::OnceCallback instead of base::Callback on its
return value handling.

The migration recipe was:
 - Convert pass-by-ref callback objects to pass-by-value.
 - Use std::move() to forward it to other consumer, or to invoke it
   with Callback::Run().
 - Handle wherever copies are required manually.
 - Check if the conversion doesn't change the semantics. As the transfer
   and invocation clobber the callback object, care about use-after-move.
   It's considered safe to consume almost scoped-out callback.

Specifically to //ash, the change in //ash/test/test_session_state_animator.cc
is non-trivial. This CL adds a BarrierClosure here.

BUG=714018
==========

to

==========
Use OnceCallback on Mojo interfaces in //ash
    
This CL flips `use_once_calback` flag on the Mojo code generator, and
fixes all compile errors after it. After this CL, Mojo interfaces in
//ash starts using base::OnceCallback instead of base::Callback on its
return value handling.

The migration recipe was:
 - Convert pass-by-ref callback objects to pass-by-value.
 - Use std::move() to forward it to other consumer, or to invoke it
   with Callback::Run().
 - Handle wherever copies are required manually.
 - Check if the conversion doesn't change the semantics. As the transfer
   and invocation clobber the callback object, care about use-after-move.
   It's considered safe to consume almost scoped-out callback.

Specifically to //ash, the change in //ash/test/test_session_state_animator.cc
is non-trivial. This CL adds a BarrierClosure here.

BUG=714018

Review-Url: https://codereview.chromium.org/2867673004
Cr-Commit-Position: refs/heads/master@{#470902}
Committed:
https://chromium.googlesource.com/chromium/src/+/5788c3d4d26446572f032baa70ba...
==========

[commit-bot: I haz the power, 2017-05-11 10:24:31]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/5788c3d4d26446572f032baa70ba...