Shift cookie system callbacks to OnceCallback to impedance match mojo.

Shift cookie system callbacks to OnceCallback to match mojo.

Mojo provides OnceCallbacks to services for notification of
completion of operations (which makes sense--operations
can't conceptually complete multiple times). Unfortunately,
CookieStore, while asynchronous, uses the default repeating
callbacks.  This CL shifts the CookieStore over to using
OnceCallbacks, so that a cookie service may be layered on
top of it.  This change is made simpler and smaller by the
fact that there is an automatic cast from a repeated Callback
to a OnceCallback, so consumers of the CookieStore
interface do not need to be converted.  However, all
subclasses of CookieStore do.

See https://docs.google.com/document/d/1jsaJAT-WbFS2ZwLEPkAgGcMTQ6MQLIQlyQfd1JeEupg/edit#heading=h.5ztx0u98smg8
for details on OnceCallback and conversions.  A couple of
things to note about conversions: 
* OnceCallbacks can't be copied, they need to be moved
  (std::move).
* Bind produces Callbacks, BindOnce produces
  OnceCallbacks.
* If you Bind a OnceCallback, that's similar to binding a
  unique_ptr<>; you have to use Passed() and it's not clear
  what happens if you call the result multiple times.
  BindOnce() Just Works, and correctly expresses the
  semantics (since if a repeating Callback actually used a
  OnceCallback, it's not clear what the repeating Callback
  would do on the next call).

BUG=721395
Review-Url: https://codereview.chromium.org/2937963003
Cr-Commit-Position: refs/heads/master@{#481573}
Committed: https://chromium.googlesource.com/chromium/src/+/7ac8171e75097750531a69f5eadf32b03f481f96

[Randy Smith (Not in Mondays), 2017-06-14 22:41:22]

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

[commit-bot: I haz the power, 2017-06-14 22:41:52]

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-06-14 22:50:08]

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

[commit-bot: I haz the power, 2017-06-14 22:50:09]

Dry run: Try jobs failed on following builders:
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[Randy Smith (Not in Mondays), 2017-06-15 21:11:32]

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

[commit-bot: I haz the power, 2017-06-15 21:11:57]

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-06-15 21:22:28]

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

[commit-bot: I haz the power, 2017-06-15 21:22:28]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[Randy Smith (Not in Mondays), 2017-06-15 22:45:47]

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

[commit-bot: I haz the power, 2017-06-15 22:46:22]

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-06-15 22:55:00]

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

[commit-bot: I haz the power, 2017-06-15 22:55:01]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)

[Randy Smith (Not in Mondays), 2017-06-16 14:00:17]

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

[commit-bot: I haz the power, 2017-06-16 14:00:26]

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-06-16 14:11:12]

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

[commit-bot: I haz the power, 2017-06-16 14:11:13]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)

[Randy Smith (Not in Mondays), 2017-06-17 00:13:39]

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

[commit-bot: I haz the power, 2017-06-17 00:14:06]

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-06-17 00:53:11]

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

[commit-bot: I haz the power, 2017-06-17 00:53:12]

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_...)

[Randy Smith (Not in Mondays), 2017-06-17 01:25:38]

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

[commit-bot: I haz the power, 2017-06-17 01:26:01]

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-06-17 01:26:02]

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

[commit-bot: I haz the power, 2017-06-17 01:26:03]

Dry run: CQ cannot get SignCLA result. Please try later.

[Randy Smith (Not in Mondays), 2017-06-17 01:27:43]

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

[commit-bot: I haz the power, 2017-06-17 01:27:58]

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-06-17 02:04:47]

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

[commit-bot: I haz the power, 2017-06-17 02:04:48]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)

[Randy Smith (Not in Mondays), 2017-06-17 02:38:50]

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

[commit-bot: I haz the power, 2017-06-17 02:39: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-06-17 03:12:58]

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

[commit-bot: I haz the power, 2017-06-17 03:12:59]

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_...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Randy Smith (Not in Mondays), 2017-06-17 13:00:36]

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

[commit-bot: I haz the power, 2017-06-17 13:00:46]

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-06-17 13:32:23]

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

[commit-bot: I haz the power, 2017-06-17 13:32:25]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)

[Randy Smith (Not in Mondays), 2017-06-17 13:37:40]

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

[commit-bot: I haz the power, 2017-06-17 13:37:42]

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-06-17 14:08:23]

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

[commit-bot: I haz the power, 2017-06-17 14:08:24]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Randy Smith (Not in Mondays), 2017-06-17 14:47:13]

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

[commit-bot: I haz the power, 2017-06-17 14:47:25]

Dry run: CQ is trying da patch.

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

[Randy Smith (Not in Mondays), 2017-06-17 16:10:25]

Description was changed from

==========
Shift cookie system callbacks to OnceCallback to impedance match mojo.

BUG=721395
==========

to

==========
Shift cookie system callbacks to OnceCallback to impedance match mojo.

Mojo provides OnceCallbacks to services for notification of completion of
operations (which makes sense--operations can't conceptually complete multiple
times).  Unfortunately, CookieStore, while asynchronous, uses the default
repeating callbacks.  This CL shifts the CookieStore over to using
OnceCallbacks, so that a cookie service may be layered on top of it.  This
change is made simpler and smaller by the fact that there is an automatic cast
from a repeated Callback to a OnceCallback, so consumers of the CookieStore
interface do not need to be converted.  However, all subclasses of CookieStore
do.

See
https://docs.google.com/document/d/1jsaJAT-WbFS2ZwLEPkAgGcMTQ6MQLIQlyQfd1JeEu...
for details on OnceCallback and conversions.  A couple of things to note about
conversions: 
* OnceCallbacks can't be copied, they need to be moved (std::move).
* Bind produces Callbacks, BindOnce produces OnceCallbacks.
* If you Bind a OnceCallback, that's similar to binding a unique_ptr<>; you have
to use Passed() and it's not clear what happens if you call the result multiple
times.  BindOnce() Just Works, and correctly expresses the semantics (since if a
repeating Callback actually used a OnceCallback, it's not clear what the
repeating Callback would do on the next call).


BUG=721395
==========

[commit-bot: I haz the power, 2017-06-17 16:17:11]

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

[commit-bot: I haz the power, 2017-06-17 16:17:12]

Dry run: This issue passed the CQ dry run.

[Randy Smith (Not in Mondays), 2017-06-17 16:20:38]

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

[commit-bot: I haz the power, 2017-06-17 16:20:41]

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-06-17 17:13:22]

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

[commit-bot: I haz the power, 2017-06-17 17:13:23]

Dry run: Try jobs failed on following builders:
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Randy Smith (Not in Mondays), 2017-06-17 18:42:15]

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

[commit-bot: I haz the power, 2017-06-17 18:42:23]

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-06-17 18:51:12]

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

[commit-bot: I haz the power, 2017-06-17 18:51:13]

Dry run: Try jobs failed on following builders:
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[Randy Smith (Not in Mondays), 2017-06-17 19:02:43]

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

[commit-bot: I haz the power, 2017-06-17 19:02:52]

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-06-17 21:39:05]

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

[commit-bot: I haz the power, 2017-06-17 21:39:07]

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

[tzik, 2017-06-19 02:18:13]

On 2017/06/17 21:39:07, commit-bot: I haz the power wrote:
> Dry run: Try jobs failed on following builders:
>   linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

non owner LGTM.

[Ryan Sleevi, 2017-06-19 07:28:11]

Description was changed from

==========
Shift cookie system callbacks to OnceCallback to impedance match mojo.

Mojo provides OnceCallbacks to services for notification of completion of
operations (which makes sense--operations can't conceptually complete multiple
times).  Unfortunately, CookieStore, while asynchronous, uses the default
repeating callbacks.  This CL shifts the CookieStore over to using
OnceCallbacks, so that a cookie service may be layered on top of it.  This
change is made simpler and smaller by the fact that there is an automatic cast
from a repeated Callback to a OnceCallback, so consumers of the CookieStore
interface do not need to be converted.  However, all subclasses of CookieStore
do.

See
https://docs.google.com/document/d/1jsaJAT-WbFS2ZwLEPkAgGcMTQ6MQLIQlyQfd1JeEu...
for details on OnceCallback and conversions.  A couple of things to note about
conversions: 
* OnceCallbacks can't be copied, they need to be moved (std::move).
* Bind produces Callbacks, BindOnce produces OnceCallbacks.
* If you Bind a OnceCallback, that's similar to binding a unique_ptr<>; you have
to use Passed() and it's not clear what happens if you call the result multiple
times.  BindOnce() Just Works, and correctly expresses the semantics (since if a
repeating Callback actually used a OnceCallback, it's not clear what the
repeating Callback would do on the next call).


BUG=721395
==========

to

==========
Shift cookie system callbacks to OnceCallback to impedance match mojo.

Mojo provides OnceCallbacks to services for notification of completion of
operations
(which makes sense--operations can't conceptually complete multiple times). 
Unfortunately, CookieStore, while asynchronous, uses the default repeating
callbacks. 
This CL shifts the CookieStore over to using OnceCallbacks, so that a cookie
service
may be layered on top of it.  This change is made simpler and smaller by the
fact that
there is an automatic cast from a repeated Callback to a OnceCallback, so
consumers
of the CookieStore interface do not need to be converted.  However, all
subclasses of
CookieStore do.

See
https://docs.google.com/document/d/1jsaJAT-WbFS2ZwLEPkAgGcMTQ6MQLIQlyQfd1JeEu...
for
details on OnceCallback and conversions.  A couple of things to note about
conversions: 
* OnceCallbacks can't be copied, they need to be moved (std::move).
* Bind produces Callbacks, BindOnce produces OnceCallbacks.
* If you Bind a OnceCallback, that's similar to binding a unique_ptr<>; you have
to use
Passed() and it's not clear what happens if you call the result multiple times. 
BindOnce() Just Works, and correctly expresses the semantics (since if a
repeating
Callback actually used a OnceCallback, it's not clear what the repeating
Callback
would do on the next call).

BUG=721395
==========

[Ryan Sleevi, 2017-06-19 07:28:31]

Description was changed from

==========
Shift cookie system callbacks to OnceCallback to impedance match mojo.

Mojo provides OnceCallbacks to services for notification of completion of
operations
(which makes sense--operations can't conceptually complete multiple times). 
Unfortunately, CookieStore, while asynchronous, uses the default repeating
callbacks. 
This CL shifts the CookieStore over to using OnceCallbacks, so that a cookie
service
may be layered on top of it.  This change is made simpler and smaller by the
fact that
there is an automatic cast from a repeated Callback to a OnceCallback, so
consumers
of the CookieStore interface do not need to be converted.  However, all
subclasses of
CookieStore do.

See
https://docs.google.com/document/d/1jsaJAT-WbFS2ZwLEPkAgGcMTQ6MQLIQlyQfd1JeEu...
for
details on OnceCallback and conversions.  A couple of things to note about
conversions: 
* OnceCallbacks can't be copied, they need to be moved (std::move).
* Bind produces Callbacks, BindOnce produces OnceCallbacks.
* If you Bind a OnceCallback, that's similar to binding a unique_ptr<>; you have
to use
Passed() and it's not clear what happens if you call the result multiple times. 
BindOnce() Just Works, and correctly expresses the semantics (since if a
repeating
Callback actually used a OnceCallback, it's not clear what the repeating
Callback
would do on the next call).

BUG=721395
==========

to

==========
Shift cookie system callbacks to OnceCallback to impedance match mojo.

Mojo provides OnceCallbacks to services for notification of completion of
operations
(which makes sense--operations can't conceptually complete multiple times). 
Unfortunately, CookieStore, while asynchronous, uses the default repeating
callbacks. 
This CL shifts the CookieStore over to using OnceCallbacks, so that a cookie
service
may be layered on top of it.  This change is made simpler and smaller by the
fact that
there is an automatic cast from a repeated Callback to a OnceCallback, so
consumers
of the CookieStore interface do not need to be converted.  However, all
subclasses of
CookieStore do.

See
https://docs.google.com/document/d/1jsaJAT-WbFS2ZwLEPkAgGcMTQ6MQLIQlyQfd1JeEu...
for details on OnceCallback and conversions.  A couple of things to note about
conversions: 
* OnceCallbacks can't be copied, they need to be moved (std::move).
* Bind produces Callbacks, BindOnce produces OnceCallbacks.
* If you Bind a OnceCallback, that's similar to binding a unique_ptr<>; you have
to use
Passed() and it's not clear what happens if you call the result multiple times. 
BindOnce() Just Works, and correctly expresses the semantics (since if a
repeating
Callback actually used a OnceCallback, it's not clear what the repeating
Callback
would do on the next call).

BUG=721395
==========

[Ryan Sleevi, 2017-06-19 07:30:35]

Description was changed from

==========
Shift cookie system callbacks to OnceCallback to impedance match mojo.

Mojo provides OnceCallbacks to services for notification of completion of
operations
(which makes sense--operations can't conceptually complete multiple times). 
Unfortunately, CookieStore, while asynchronous, uses the default repeating
callbacks. 
This CL shifts the CookieStore over to using OnceCallbacks, so that a cookie
service
may be layered on top of it.  This change is made simpler and smaller by the
fact that
there is an automatic cast from a repeated Callback to a OnceCallback, so
consumers
of the CookieStore interface do not need to be converted.  However, all
subclasses of
CookieStore do.

See
https://docs.google.com/document/d/1jsaJAT-WbFS2ZwLEPkAgGcMTQ6MQLIQlyQfd1JeEu...
for details on OnceCallback and conversions.  A couple of things to note about
conversions: 
* OnceCallbacks can't be copied, they need to be moved (std::move).
* Bind produces Callbacks, BindOnce produces OnceCallbacks.
* If you Bind a OnceCallback, that's similar to binding a unique_ptr<>; you have
to use
Passed() and it's not clear what happens if you call the result multiple times. 
BindOnce() Just Works, and correctly expresses the semantics (since if a
repeating
Callback actually used a OnceCallback, it's not clear what the repeating
Callback
would do on the next call).

BUG=721395
==========

to

==========
Shift cookie system callbacks to OnceCallback to match mojo.

Mojo provides OnceCallbacks to services for notification of
completion of operations (which makes sense--operations
can't conceptually complete multiple times). Unfortunately,
CookieStore, while asynchronous, uses the default repeating
callbacks.  This CL shifts the CookieStore over to using
OnceCallbacks, so that a cookie service may be layered on
top of it.  This change is made simpler and smaller by the
fact that there is an automatic cast from a repeated Callback
to a OnceCallback, so consumers of the CookieStore
interface do not need to be converted.  However, all
subclasses of CookieStore do.

See
https://docs.google.com/document/d/1jsaJAT-WbFS2ZwLEPkAgGcMTQ6MQLIQlyQfd1JeEu...
for details on OnceCallback and conversions.  A couple of
things to note about conversions: 
* OnceCallbacks can't be copied, they need to be moved
   (std::move).
* Bind produces Callbacks, BindOnce produces
   OnceCallbacks.
* If you Bind a OnceCallback, that's similar to binding a
   unique_ptr<>; you have to use Passed() and it's not clear
   what happens if you call the result multiple times.
   BindOnce() Just Works, and correctly expresses the
   semantics (since if a repeating Callback actually used a
   OnceCallback, it's not clear what the repeating Callback
   would do on the next call).

BUG=721395
==========

[Ryan Sleevi, 2017-06-19 07:30:57]

Description was changed from

==========
Shift cookie system callbacks to OnceCallback to match mojo.

Mojo provides OnceCallbacks to services for notification of
completion of operations (which makes sense--operations
can't conceptually complete multiple times). Unfortunately,
CookieStore, while asynchronous, uses the default repeating
callbacks.  This CL shifts the CookieStore over to using
OnceCallbacks, so that a cookie service may be layered on
top of it.  This change is made simpler and smaller by the
fact that there is an automatic cast from a repeated Callback
to a OnceCallback, so consumers of the CookieStore
interface do not need to be converted.  However, all
subclasses of CookieStore do.

See
https://docs.google.com/document/d/1jsaJAT-WbFS2ZwLEPkAgGcMTQ6MQLIQlyQfd1JeEu...
for details on OnceCallback and conversions.  A couple of
things to note about conversions: 
* OnceCallbacks can't be copied, they need to be moved
   (std::move).
* Bind produces Callbacks, BindOnce produces
   OnceCallbacks.
* If you Bind a OnceCallback, that's similar to binding a
   unique_ptr<>; you have to use Passed() and it's not clear
   what happens if you call the result multiple times.
   BindOnce() Just Works, and correctly expresses the
   semantics (since if a repeating Callback actually used a
   OnceCallback, it's not clear what the repeating Callback
   would do on the next call).

BUG=721395
==========

to

==========
Shift cookie system callbacks to OnceCallback to match mojo.

Mojo provides OnceCallbacks to services for notification of
completion of operations (which makes sense--operations
can't conceptually complete multiple times). Unfortunately,
CookieStore, while asynchronous, uses the default repeating
callbacks.  This CL shifts the CookieStore over to using
OnceCallbacks, so that a cookie service may be layered on
top of it.  This change is made simpler and smaller by the
fact that there is an automatic cast from a repeated Callback
to a OnceCallback, so consumers of the CookieStore
interface do not need to be converted.  However, all
subclasses of CookieStore do.

See
https://docs.google.com/document/d/1jsaJAT-WbFS2ZwLEPkAgGcMTQ6MQLIQlyQfd1JeEu...
for details on OnceCallback and conversions.  A couple of
things to note about conversions: 
* OnceCallbacks can't be copied, they need to be moved
  (std::move).
* Bind produces Callbacks, BindOnce produces
  OnceCallbacks.
* If you Bind a OnceCallback, that's similar to binding a
  unique_ptr<>; you have to use Passed() and it's not clear
  what happens if you call the result multiple times.
  BindOnce() Just Works, and correctly expresses the
  semantics (since if a repeating Callback actually used a
  OnceCallback, it's not clear what the repeating Callback
  would do on the next call).

BUG=721395
==========

[Ryan Sleevi, 2017-06-19 07:34:05]

rsleevi@chromium.org changed reviewers:
+ rsleevi@chromium.org

[Ryan Sleevi, 2017-06-19 07:34:06]

LGTM.

I tried to refrain from noting too many odd things in this CL, but one thing I
noted while tracing the callbacks (to try to make sure I understood the lifetime
issues), I noticed a frequent use of the anti-pattern if (!callback.is_null())
in the code.

While that's generally outside the remit of this CL (and thus worth refraining
from), I think it's worth revisiting the necessity of those. It's generally a
'red-flag', since things like "bound to a weak ptr" don't necessarily affect
.is_null(), and with your transition to OnceCallback/OnceClosure, should also be
a stronger guarantee of "this callback has not been run and left in an undefined
state". So I believe that _after_ this change, you can remove most (all?) of
those checks while still maintaining the same invariants, since the .Run() will
guarantee the once-ness of the callback, and it will overall simplify the logic
and code.

https://codereview.chromium.org/2937963003/diff/240001/chrome/browser/browsin...
File chrome/browser/browsing_data/browsing_data_cookie_helper.h (right):

https://codereview.chromium.org/2937963003/diff/240001/chrome/browser/browsin...
chrome/browser/browsing_data/browsing_data_cookie_helper.h:34: using
FetchCallback = base::Callback<void(const net::CookieList&)>;
Is there a reason not to change this to a OnceCallback for alignment?

[kinuko, 2017-06-19 10:52:08]

Drive-by (just saw this on net-dev and took a look to learn some)

https://codereview.chromium.org/2937963003/diff/240001/net/cookies/cookie_mon...
File net/cookies/cookie_monster.cc (right):

https://codereview.chromium.org/2937963003/diff/240001/net/cookies/cookie_mon...
net/cookies/cookie_monster.cc:942: new SetAllCookiesTask(this, list,
std::move(callback));
I wonder if (eventually) these all CookieMonsterTasks could also be move-only /
retained by unique_ptr (looks like the common usage is to push back into
pending_tasks_ and eventually just run it, though I could be missing various
things)

[Randy Smith (Not in Mondays), 2017-06-21 17:54:53]

Thanks, all!

A quick meta-comment, which is that I think a lot of time could be sunk into
cleaning up the cookie code and related problems.  I've been moderately
ruthlessly trying to avoid that (I say "moderately" because of
https://codereview.chromium.org/2930893002 and
https://codereview.chromium.org/2929923002, but I'm resistant to getting pulled
further down the rabbit hole).  So I have no argument with fixing the .is_null()
anti-pattern, or the cookie task structure, or the BrowsingDataRemover
FetchCallback usage, but I'd like to say a bit more laser focussed on
servicification than I think getting distracted by all that allows.  Sorry :-J. 
Patches welcome?  :-}

https://codereview.chromium.org/2937963003/diff/240001/chrome/browser/browsin...
File chrome/browser/browsing_data/browsing_data_cookie_helper.h (right):

https://codereview.chromium.org/2937963003/diff/240001/chrome/browser/browsin...
chrome/browser/browsing_data/browsing_data_cookie_helper.h:34: using
FetchCallback = base::Callback<void(const net::CookieList&)>;
On 2017/06/19 07:34:06, Ryan Sleevi (slow through 6-27 wrote:
> Is there a reason not to change this to a OnceCallback for alignment?

Yes, though ideal would be to fix it.  The MockBrowsingDataCookieHelper inherits
from this class and uses FetchCallback, and can call the stored callback
multiple times, resulting in test errors.  See
https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium...
(look at the unit tests).

https://codereview.chromium.org/2937963003/diff/240001/net/cookies/cookie_mon...
File net/cookies/cookie_monster.cc (right):

https://codereview.chromium.org/2937963003/diff/240001/net/cookies/cookie_mon...
net/cookies/cookie_monster.cc:942: new SetAllCookiesTask(this, list,
std::move(callback));
On 2017/06/19 10:52:07, kinuko wrote:
> I wonder if (eventually) these all CookieMonsterTasks could also be move-only
/
> retained by unique_ptr (looks like the common usage is to push back into
> pending_tasks_ and eventually just run it, though I could be missing various
> things)

Yes, I think that would be a good design.  I'd also like to think about whether
they can all be abstracted away by just using callbacks; there's a metric tone
of boilerplate around the CookieMonsterTasks it would be nice to find a way to
get rid of.

[Randy Smith (Not in Mondays), 2017-06-21 18:00:08]

rdsmith@chromium.org changed reviewers:
+ alexclarke@chromium.org, droger@chromium.org, jam@chromium.org,
mkwst@chromium.org, rdevlin.cronin@chromium.org, sgurun@chromium.org

[Randy Smith (Not in Mondays), 2017-06-21 18:00:09]

Now seeking owners stamps:

* android_webview: sgurun@
* chrome/browser/browsing_data: mkwst@
* chrome/browser/extensions/api/cookies: rdevlin.cronin@
* content/browser/net: jam@ (Sorry, I appear to need a content owner for that
directory, which is weird).
* headless: alexclarke@
* ios: droger@

Thanks!

The only part of the above that's got any subtly to it at all is browsing_data;
see the 
https://codereview.chromium.org/2937963003/diff/240001/chrome/browser/browsin...
for context on that, though it's not *particularly* subtle.

[Mike West, 2017-06-21 18:15:26]

Thanks, this is valuable cleanup.

chrome/browser/browsing_data and chrome/browser/extensions/api/cookies LGTM.

I agree with kinuko@'s suggestion above, BTW. Moving to a set of unique_ptrs for
tasks seems like an improvement over what we have now.

[Devlin, 2017-06-21 18:17:14]

On 2017/06/21 18:00:09, Randy Smith (Not in Mondays) wrote:
> Now seeking owners stamps:
> * chrome/browser/extensions/api/cookies: rdevlin.cronin@

chrome/browser/extensions/api/cookies lgtm

[sgurun-gerrit only, 2017-06-21 21:27:24]

On 2017/06/21 18:17:14, Devlin wrote:
> On 2017/06/21 18:00:09, Randy Smith (Not in Mondays) wrote:
> > Now seeking owners stamps:
> > * chrome/browser/extensions/api/cookies: rdevlin.cronin@
> 
> chrome/browser/extensions/api/cookies lgtm

aw lgtm

[alex clarke (OOO till 29th), 2017-06-21 21:38:37]

headless/ LGTM

[kinuko, 2017-06-22 00:28:18]

I can stamp for content/browser/net then :) (don't we have owners file there?) -
lgtm

[Randy Smith (Not in Mondays), 2017-06-22 15:30:04]

On 2017/06/22 00:28:18, kinuko wrote:
> I can stamp for content/browser/net then :) (don't we have owners file there?)
-
> lgtm

Thanks Kinuko (and everyone else who'se responded so far)!  John, you're off the
hook.

We did a reset of OWNERS in most of {chrome,content}/*/net to point at the //net
OWNERS file, but I think there was some objection around
content/browser/net--felt like it was specialized stuff that wasn't particularly
well known in the //net owners set.  But if we've got to go all the way up to
//content OWNERS, maybe we should revisit that decision.

[Randy Smith (Not in Mondays), 2017-06-22 15:30:23]

rdsmith@chromium.org changed reviewers:
- jam@chromium.org

[droger, 2017-06-22 15:37:10]

lgtm

[Randy Smith (Not in Mondays), 2017-06-22 15:43:36]

The CQ bit was checked by rdsmith@chromium.org

[commit-bot: I haz the power, 2017-06-22 15:43:53]

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-06-22 17:09:57]

CQ is committing da patch.

Bot data: {"patchset_id": 240001, "attempt_start_ts": 1498146216519270,
"parent_rev": "407b4fb9d9633db6317f8eb3cd9e29a5abd73951", "commit_rev":
"7ac8171e75097750531a69f5eadf32b03f481f96"}

[commit-bot: I haz the power, 2017-06-22 17:10:13]

Description was changed from

==========
Shift cookie system callbacks to OnceCallback to match mojo.

Mojo provides OnceCallbacks to services for notification of
completion of operations (which makes sense--operations
can't conceptually complete multiple times). Unfortunately,
CookieStore, while asynchronous, uses the default repeating
callbacks.  This CL shifts the CookieStore over to using
OnceCallbacks, so that a cookie service may be layered on
top of it.  This change is made simpler and smaller by the
fact that there is an automatic cast from a repeated Callback
to a OnceCallback, so consumers of the CookieStore
interface do not need to be converted.  However, all
subclasses of CookieStore do.

See
https://docs.google.com/document/d/1jsaJAT-WbFS2ZwLEPkAgGcMTQ6MQLIQlyQfd1JeEu...
for details on OnceCallback and conversions.  A couple of
things to note about conversions: 
* OnceCallbacks can't be copied, they need to be moved
  (std::move).
* Bind produces Callbacks, BindOnce produces
  OnceCallbacks.
* If you Bind a OnceCallback, that's similar to binding a
  unique_ptr<>; you have to use Passed() and it's not clear
  what happens if you call the result multiple times.
  BindOnce() Just Works, and correctly expresses the
  semantics (since if a repeating Callback actually used a
  OnceCallback, it's not clear what the repeating Callback
  would do on the next call).

BUG=721395
==========

to

==========
Shift cookie system callbacks to OnceCallback to match mojo.

Mojo provides OnceCallbacks to services for notification of
completion of operations (which makes sense--operations
can't conceptually complete multiple times). Unfortunately,
CookieStore, while asynchronous, uses the default repeating
callbacks.  This CL shifts the CookieStore over to using
OnceCallbacks, so that a cookie service may be layered on
top of it.  This change is made simpler and smaller by the
fact that there is an automatic cast from a repeated Callback
to a OnceCallback, so consumers of the CookieStore
interface do not need to be converted.  However, all
subclasses of CookieStore do.

See
https://docs.google.com/document/d/1jsaJAT-WbFS2ZwLEPkAgGcMTQ6MQLIQlyQfd1JeEu...
for details on OnceCallback and conversions.  A couple of
things to note about conversions: 
* OnceCallbacks can't be copied, they need to be moved
  (std::move).
* Bind produces Callbacks, BindOnce produces
  OnceCallbacks.
* If you Bind a OnceCallback, that's similar to binding a
  unique_ptr<>; you have to use Passed() and it's not clear
  what happens if you call the result multiple times.
  BindOnce() Just Works, and correctly expresses the
  semantics (since if a repeating Callback actually used a
  OnceCallback, it's not clear what the repeating Callback
  would do on the next call).

BUG=721395

Review-Url: https://codereview.chromium.org/2937963003
Cr-Commit-Position: refs/heads/master@{#481573}
Committed:
https://chromium.googlesource.com/chromium/src/+/7ac8171e75097750531a69f5eadf...
==========

[commit-bot: I haz the power, 2017-06-22 17:10:15]

Committed patchset #13 (id:240001) as
https://chromium.googlesource.com/chromium/src/+/7ac8171e75097750531a69f5eadf...