Enable use_once_callback for //content/common/*.mojom

Enable use_once_callback for //content/common/*.mojom

This CL flips use_once_callback flag in //content/common/BUILD.gn, and
fixes all compile failure after that. After this CL, mojo-generated code
starts using OnceCallback instead of Callback for asynchronous result handling.

BUG=554299
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2824193002
Cr-Commit-Position: refs/heads/master@{#466920}
Committed: https://chromium.googlesource.com/chromium/src/+/c200756ff65784f2151383ea803d177d6a9767a6

[tzik, 2017-04-18 11:49:59]

Description was changed from

==========
Enable use_once_callback for //content/common/*.mojom

BUG=
==========

to

==========
Enable use_once_callback for //content/common/*.mojom

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[tzik, 2017-04-18 11:54:31]

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

[commit-bot: I haz the power, 2017-04-18 11:54:47]

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

[tzik, 2017-04-18 12:19:07]

Description was changed from

==========
Enable use_once_callback for //content/common/*.mojom

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Enable use_once_callback for //content/common/*.mojom

This CL flips use_once_callback flag in //content/common/BUILD.gn, and
fixes all compile failure after that.

After this CL, mojo-generated code starts using OnceCallback instead of
Callback for asynchronous result handling.

BUG=554299
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[tzik, 2017-04-18 12:22:59]

tzik@chromium.org changed reviewers:
+ kinuko@chromium.org, nhiroki@chromium.org

[tzik, 2017-04-18 12:27:15]

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

[tzik, 2017-04-18 12:29:13]

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

[tzik, 2017-04-18 12:38:06]

Description was changed from

==========
Enable use_once_callback for //content/common/*.mojom

This CL flips use_once_callback flag in //content/common/BUILD.gn, and
fixes all compile failure after that.

After this CL, mojo-generated code starts using OnceCallback instead of
Callback for asynchronous result handling.

BUG=554299
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Enable use_once_callback for //content/common/*.mojom

This CL flips use_once_callback flag in //content/common/BUILD.gn, and
fixes all compile failure after that. After this CL, mojo-generated code
starts using OnceCallback instead of Callback for asynchronous result handling.

BUG=554299
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[tzik, 2017-04-18 12:48:07]

PTAL to:
nhiroki: Service Worker stuff. //content/*/service_worker
xhwang: //content/browser/renderer_host/media, //content/renderer/media.
mmenke: //content/browser/loader and //content/network
kinuko: Other //content

[commit-bot: I haz the power, 2017-04-18 12:52:15]

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

[commit-bot: I haz the power, 2017-04-18 12:52:15]

Dry run: Try jobs failed on following builders:
  linux_site_isolation on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_site_isol...)

[tzik, 2017-04-18 15:05:48]

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

[commit-bot: I haz the power, 2017-04-18 15:06:18]

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-18 15:10:30]

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

[commit-bot: I haz the power, 2017-04-18 15:10:31]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[mmenke, 2017-04-18 15:29:05]

On 2017/04/18 12:48:07, tzik wrote:
> PTAL to:
> nhiroki: Service Worker stuff. //content/*/service_worker
> xhwang: //content/browser/renderer_host/media, //content/renderer/media.
> mmenke: //content/browser/loader and //content/network
> kinuko: Other //content

Have you considered if this is the most intuitive way to go about this?  The
difference between "SyncLoadCallback callback" and "const SyncLoadCallback&
callback" is rather non-obvious (In that this is like unique_ptr passing, as
opposed to passing pretty much any other data type).

[mmenke, 2017-04-18 15:30:38]

On 2017/04/18 15:29:05, mmenke wrote:
> On 2017/04/18 12:48:07, tzik wrote:
> > PTAL to:
> > nhiroki: Service Worker stuff. //content/*/service_worker
> > xhwang: //content/browser/renderer_host/media, //content/renderer/media.
> > mmenke: //content/browser/loader and //content/network
> > kinuko: Other //content
> 
> Have you considered if this is the most intuitive way to go about this?  The
> difference between "SyncLoadCallback callback" and "const SyncLoadCallback&
> callback" is rather non-obvious (In that this is like unique_ptr passing, as
> opposed to passing pretty much any other data type).

Also worth noting //content/network doesn't have an OWNERS file currently?  I'm
happy to review it, as I am familiar with it, you'll just need need an OWNERS
signoff (As I don't think you're a content owner)

[xhwang, 2017-04-18 16:41:33]

//content/browser/renderer_host/media and //content/renderer/media lgtm

Do you plan to update media/mojo/interfaces/*.mojom as well?

[tzik, 2017-04-18 22:59:57]

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

[commit-bot: I haz the power, 2017-04-18 23:00:40]

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 00:06:50]

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

[commit-bot: I haz the power, 2017-04-19 00:06:51]

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

[nhiroki, 2017-04-19 01:55:40]

service worker lgtm

[tzik, 2017-04-19 03:30:54]

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

[commit-bot: I haz the power, 2017-04-19 03:31:09]

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

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

[commit-bot: I haz the power, 2017-04-19 05:32:53]

Dry run: This issue passed the CQ dry run.

[kinuko, 2017-04-19 07:10:03]

The change appears rather subtle, but assuming that we're going to have compile
error if we try to write like before. lgtm

[tzik, 2017-04-19 14:11:27]

On 2017/04/18 15:29:05, mmenke wrote:
> On 2017/04/18 12:48:07, tzik wrote:
> > PTAL to:
> > nhiroki: Service Worker stuff. //content/*/service_worker
> > xhwang: //content/browser/renderer_host/media, //content/renderer/media.
> > mmenke: //content/browser/loader and //content/network
> > kinuko: Other //content
> 
> Have you considered if this is the most intuitive way to go about this?  The
> difference between "SyncLoadCallback callback" and "const SyncLoadCallback&
> callback" is rather non-obvious (In that this is like unique_ptr passing, as
> opposed to passing pretty much any other data type).

Yes, I believe this is the best way.
A motivation to use OnceCallback is its unique owner ship without the reference
count.
And that requires pass-by-value or pass-by-rvalue-reference, as std::unique_ptr
does.

[tzik, 2017-04-19 14:16:28]

On 2017/04/18 15:30:38, mmenke wrote:
> On 2017/04/18 15:29:05, mmenke wrote:
> > On 2017/04/18 12:48:07, tzik wrote:
> > > PTAL to:
> > > nhiroki: Service Worker stuff. //content/*/service_worker
> > > xhwang: //content/browser/renderer_host/media, //content/renderer/media.
> > > mmenke: //content/browser/loader and //content/network
> > > kinuko: Other //content
> > 
> > Have you considered if this is the most intuitive way to go about this?  The
> > difference between "SyncLoadCallback callback" and "const SyncLoadCallback&
> > callback" is rather non-obvious (In that this is like unique_ptr passing, as
> > opposed to passing pretty much any other data type).
> 
> Also worth noting //content/network doesn't have an OWNERS file currently? 
I'm
> happy to review it, as I am familiar with it, you'll just need need an OWNERS
> signoff (As I don't think you're a content owner)

I didn't aware that. Does Net team have a plan to add it? (Just adding
file://net/OWNERS probably works well?)

[tzik, 2017-04-20 06:25:44]

On 2017/04/18 16:41:33, xhwang_slow wrote:
> //content/browser/renderer_host/media and //content/renderer/media lgtm
> 
> Do you plan to update media/mojo/interfaces/*.mojom as well?

Well, yes, but I will ask chromium-dev for volunteer if it turns out there's too
much to do for me.

[mmenke, 2017-04-20 15:21:38]

On 2017/04/20 06:25:44, tzik wrote:
> On 2017/04/18 16:41:33, xhwang_slow wrote:
> > //content/browser/renderer_host/media and //content/renderer/media lgtm
> > 
> > Do you plan to update media/mojo/interfaces/*.mojom as well?
> 
> Well, yes, but I will ask chromium-dev for volunteer if it turns out there's
too
> much to do for me.

Sorry for the delay - I'll try to get to this today, but we found out no one has
been on network bug triage for the last 10 days, due to a tool failure, and I'm
playing catchup, so may not be until tomorrow.

[mmenke, 2017-04-21 16:25:58]

LGTM, just pointing out one other place BindOnce could optionally be used,
though the change would propogate down through a number of files.  LGTM, either
way.

https://codereview.chromium.org/2824193002/diff/60001/content/browser/loader/...
File content/browser/loader/url_loader_factory_impl.cc (right):

https://codereview.chromium.org/2824193002/diff/60001/content/browser/loader/...
content/browser/loader/url_loader_factory_impl.cc:100:
base::Bind(&DispatchSyncLoadResult, base::Passed(&callback)));
You could use BindOnce here, too, and make OnSyncLoadWithMojo take a
non-const-ref callback, and work your way down a bit.

[tzik, 2017-04-24 09:17:11]

https://codereview.chromium.org/2824193002/diff/60001/content/browser/loader/...
File content/browser/loader/url_loader_factory_impl.cc (right):

https://codereview.chromium.org/2824193002/diff/60001/content/browser/loader/...
content/browser/loader/url_loader_factory_impl.cc:100:
base::Bind(&DispatchSyncLoadResult, base::Passed(&callback)));
On 2017/04/21 16:25:57, mmenke wrote:
> You could use BindOnce here, too, and make OnSyncLoadWithMojo take a
> non-const-ref callback, and work your way down a bit.

Let me do it in a separate CL.
# This should hopefully be handled by a tool based refactoring later. But doing
it manually for now.

[tzik, 2017-04-24 09:17:20]

The CQ bit was checked by tzik@chromium.org

[tzik, 2017-04-24 09:17:21]

The patchset sent to the CQ was uploaded after l-g-t-m from xhwang@chromium.org,
nhiroki@chromium.org
Link to the patchset: https://codereview.chromium.org/2824193002/#ps60001
(title: "push_messaging fix")

[commit-bot: I haz the power, 2017-04-24 09:17:33]

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-24 09:21:17]

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

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

Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[tzik, 2017-04-24 11:11:38]

The CQ bit was checked by tzik@chromium.org

[tzik, 2017-04-24 11:11:38]

The patchset sent to the CQ was uploaded after l-g-t-m from kinuko@chromium.org,
xhwang@chromium.org, nhiroki@chromium.org, mmenke@chromium.org
Link to the patchset: https://codereview.chromium.org/2824193002/#ps80001
(title: "rebase")

[commit-bot: I haz the power, 2017-04-24 11:11:50]

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-24 14:08:24]

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

[commit-bot: I haz the power, 2017-04-24 14:08:26]

Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[tzik, 2017-04-24 18:15:15]

The CQ bit was checked by tzik@chromium.org

[commit-bot: I haz the power, 2017-04-24 18:15:55]

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-24 19:28:35]

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

[commit-bot: I haz the power, 2017-04-24 19:28:37]

Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[tzik, 2017-04-25 06:05:47]

The CQ bit was checked by tzik@chromium.org

[tzik, 2017-04-25 06:05:48]

The patchset sent to the CQ was uploaded after l-g-t-m from kinuko@chromium.org,
xhwang@chromium.org, nhiroki@chromium.org, mmenke@chromium.org
Link to the patchset: https://codereview.chromium.org/2824193002/#ps100001
(title: "rebase")

[commit-bot: I haz the power, 2017-04-25 06:06:04]

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-25 07:48:28]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1493100347888800,
"parent_rev": "b907da9ed5c5e71b804165c0c6cb43c9c318ce4c", "commit_rev":
"c200756ff65784f2151383ea803d177d6a9767a6"}

[commit-bot: I haz the power, 2017-04-25 07:48:44]

Description was changed from

==========
Enable use_once_callback for //content/common/*.mojom

This CL flips use_once_callback flag in //content/common/BUILD.gn, and
fixes all compile failure after that. After this CL, mojo-generated code
starts using OnceCallback instead of Callback for asynchronous result handling.

BUG=554299
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Enable use_once_callback for //content/common/*.mojom

This CL flips use_once_callback flag in //content/common/BUILD.gn, and
fixes all compile failure after that. After this CL, mojo-generated code
starts using OnceCallback instead of Callback for asynchronous result handling.

BUG=554299
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2824193002
Cr-Commit-Position: refs/heads/master@{#466920}
Committed:
https://chromium.googlesource.com/chromium/src/+/c200756ff65784f2151383ea803d...
==========

[commit-bot: I haz the power, 2017-04-25 07:48:46]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/c200756ff65784f2151383ea803d...