Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2753953003:
Remove RunMixin from base::Callback implementation (Closed)
|
Created:
3 years, 9 months ago by tzik Modified:
3 years, 9 months ago Reviewers:
dcheng CC:
chromium-reviews, asanka, jsbell+serviceworker_chromium.org, posciak+watch_chromium.org, ntp-dev+reviews_chromium.org, vmpstr+watch_chromium.org, extensions-reviews_chromium.org, serviceworker-reviews, jam, kinuko+serviceworker, nhiroki, chromium-apps-reviews_chromium.org, feature-media-reviews_chromium.org, darin-cc_chromium.org, michaeln, horo+watch_chromium.org, noyau+watch_chromium.org, kinuko+watch, shimazu+serviceworker_chromium.org, blink-worker-reviews_chromium.org, michaelpg Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionRemove RunMixin from base::Callback implementation
RunMixin had provided to define rvalue-qualified Callback::Run for
OnceCallback without overloading Callback::Run. This is needed since some
user took the method pointer of Callback::Run.
After landing all depends-on CLs, the method pointer of Callback::Run will
go away and we will be able to remove RunMixin. In addition, this CL adds
an overload of Callback::Run for rvalue-qualified RepeatingCallback that
nullify the callback itself, so that `std::move(callback).Run()` is more
consistent to OnceCallback and RepeatingCallback.
Review-Url: https://codereview.chromium.org/2753953003
Cr-Commit-Position: refs/heads/master@{#458340}
Committed: https://chromium.googlesource.com/chromium/src/+/ecb1b24070b8cbf7958f05ecad9373b979e212b9
Patch Set 1 #Patch Set 2 : . #Patch Set 3 : . #Patch Set 4 : rebase #Patch Set 5 : +test #Patch Set 6 : rebase #Messages
Total messages: 29 (24 generated)
The CQ bit was checked by tzik@chromium.org to run a CQ dry run
The CQ bit was checked by tzik@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
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...)
The CQ bit was checked by tzik@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by tzik@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by tzik@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Remove RunMixin from base::Callback implementation BUG= ========== to ========== Remove RunMixin from base::Callback implementation 123456789012345678901234567890123456789012345678901234567890123456789012 RunMixin had provided to define rvalue-qualified Callback::Run for OnceCallback without overloading Callback::Run. This is needed since some user took the method pointer of Callback::Run. After landing all depends-on CLs, the method pointer of Callback::Run will go away and we will be able to remove RunMixin. In addition, this CL adds an overload of Callback::Run for rvalue-qualified RepeatingCallback that nullify the callback itself, so that `std::move(callback).Run()` is more consistent to OnceCallback and RepeatingCallback. ==========
Description was changed from ========== Remove RunMixin from base::Callback implementation 123456789012345678901234567890123456789012345678901234567890123456789012 RunMixin had provided to define rvalue-qualified Callback::Run for OnceCallback without overloading Callback::Run. This is needed since some user took the method pointer of Callback::Run. After landing all depends-on CLs, the method pointer of Callback::Run will go away and we will be able to remove RunMixin. In addition, this CL adds an overload of Callback::Run for rvalue-qualified RepeatingCallback that nullify the callback itself, so that `std::move(callback).Run()` is more consistent to OnceCallback and RepeatingCallback. ========== to ========== Remove RunMixin from base::Callback implementation RunMixin had provided to define rvalue-qualified Callback::Run for OnceCallback without overloading Callback::Run. This is needed since some user took the method pointer of Callback::Run. After landing all depends-on CLs, the method pointer of Callback::Run will go away and we will be able to remove RunMixin. In addition, this CL adds an overload of Callback::Run for rvalue-qualified RepeatingCallback that nullify the callback itself, so that `std::move(callback).Run()` is more consistent to OnceCallback and RepeatingCallback. ==========
tzik@chromium.org changed reviewers: + dcheng@chromium.org
PTAL
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
LGTM Did you check that the nocompile tests all still pass?
The CQ bit was checked by tzik@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
On 2017/03/17 18:31:26, dcheng wrote: > LGTM > > Did you check that the nocompile tests all still pass? Yes, it passes. And, nocompile tests seems to be enabled by default for some platforms including linux. https://cs.chromium.org/chromium/src/build/nocompile.gni?type=cs&q=nocompile_...
The CQ bit was checked by tzik@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from dcheng@chromium.org Link to the patchset: https://codereview.chromium.org/2753953003/#ps100001 (title: "rebase")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 100001, "attempt_start_ts": 1490080583949350,
"parent_rev": "a626eb843a373043835437074f4c7de51baf00f4", "commit_rev":
"ecb1b24070b8cbf7958f05ecad9373b979e212b9"}
Message was sent while issue was closed.
Description was changed from ========== Remove RunMixin from base::Callback implementation RunMixin had provided to define rvalue-qualified Callback::Run for OnceCallback without overloading Callback::Run. This is needed since some user took the method pointer of Callback::Run. After landing all depends-on CLs, the method pointer of Callback::Run will go away and we will be able to remove RunMixin. In addition, this CL adds an overload of Callback::Run for rvalue-qualified RepeatingCallback that nullify the callback itself, so that `std::move(callback).Run()` is more consistent to OnceCallback and RepeatingCallback. ========== to ========== Remove RunMixin from base::Callback implementation RunMixin had provided to define rvalue-qualified Callback::Run for OnceCallback without overloading Callback::Run. This is needed since some user took the method pointer of Callback::Run. After landing all depends-on CLs, the method pointer of Callback::Run will go away and we will be able to remove RunMixin. In addition, this CL adds an overload of Callback::Run for rvalue-qualified RepeatingCallback that nullify the callback itself, so that `std::move(callback).Run()` is more consistent to OnceCallback and RepeatingCallback. Review-Url: https://codereview.chromium.org/2753953003 Cr-Commit-Position: refs/heads/master@{#458340} Committed: https://chromium.googlesource.com/chromium/src/+/ecb1b24070b8cbf7958f05ecad93... ==========
Message was sent while issue was closed.
Committed patchset #6 (id:100001) as https://chromium.googlesource.com/chromium/src/+/ecb1b24070b8cbf7958f05ecad93... |
