Issue metadata
Sign in to add a comment
|
base:: should have a CallbackWithDefault. Mojo programmers could use it to simplify cleanup when connections break. |
||||||||||||||||||||||
Issue descriptionCurrently, mojo uses set_connection_error_handler as the mechanism by which programmers detect broken mojo connections and clean up. This leaves a burden of tracking and cleaning up individual in-flight (incomplete) operations to the programmer. In the case of mojo methods which return values, C++ programmers provide callbacks on the client side to receive those values. Mojo tracks them internally and deletes the callback objects if the connection dies. By defining a destructor for the callback object, we can hook Mojo's internal tracking. ScopedCallbackRunner https://codesearch.chromium.org/chromium/src/media/base/scoped_callback_runner.h?q=ScopedCallbackRunner&dr=CSs already does this in one way, by wrapping a callback and calling it with default args if the destructor runs before the wrapper's Run() method. This is suitable for many use cases, but not those in which i) we want to take special action if the connection fails ii) we don't want "call with default args" to mean connection failure. We propose base::CallbackWithDefault to replace the existing ScopedCallbackRunner, add base::CallbackWithCustomDelete as the most general case. These should return base::OnceCallback objects.
,
Oct 17 2017
,
Nov 21 2017
(+CC mmenke and myself who have talked about similar solutions too.)
,
Dec 21 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/af16e1187bb6300ccafe56aa4d8dfad1944ac656 commit af16e1187bb6300ccafe56aa4d8dfad1944ac656 Author: Nicholas Verne <nverne@chromium.org> Date: Thu Dec 21 00:42:54 2017 WrapCallbackWithDefaultInvokeIfNotRun and WrapCallbackWithDropHandler WrapCallbackWithDefaultInvokeIfNotRun renames media::base::ScopedCallbackRunner and moves it to mojo::. WrapCallbackWithDropHandler is function allowing users to specify the "drop" callback if needed. These callback helpers are intended for use on client side mojo calls. foo_ptr->MethodWithResponse(arg1, arg2, mojo::WrapCallbackWithDefaultInvokeIfNotRun( base::BindOnce(&Foo::OnResponse), default_arg))); Please see the related bug for more details. Bug: 775358 Change-Id: Ied9d0742a260954ee670a4d8b388053c32b2665b Reviewed-on: https://chromium-review.googlesource.com/724763 Reviewed-by: John Abd-El-Malek <jam@chromium.org> Reviewed-by: Ken Rockot <rockot@chromium.org> Reviewed-by: Sam McNally <sammc@chromium.org> Commit-Queue: John Abd-El-Malek <jam@chromium.org> Cr-Commit-Position: refs/heads/master@{#525536} [modify] https://crrev.com/af16e1187bb6300ccafe56aa4d8dfad1944ac656/content/browser/histogram_controller.cc [modify] https://crrev.com/af16e1187bb6300ccafe56aa4d8dfad1944ac656/mojo/public/cpp/bindings/BUILD.gn [add] https://crrev.com/af16e1187bb6300ccafe56aa4d8dfad1944ac656/mojo/public/cpp/bindings/callback_helpers.h [modify] https://crrev.com/af16e1187bb6300ccafe56aa4d8dfad1944ac656/mojo/public/cpp/bindings/tests/BUILD.gn [add] https://crrev.com/af16e1187bb6300ccafe56aa4d8dfad1944ac656/mojo/public/cpp/bindings/tests/callback_helpers_unittest.cc
,
Dec 21 2017
,
Jan 9 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e5a6661846ddacc04ac6624c9eed6e7353497ec9 commit e5a6661846ddacc04ac6624c9eed6e7353497ec9 Author: Nicholas Verne <nverne@chromium.org> Date: Tue Jan 09 21:07:52 2018 Delete scoped_callback_runner.h. The code was copied over to mojo/public/cpp/bindings/callback_helpers.h Bug: 775358 Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel Change-Id: I93d2b99cf4ff5795d43f81cf61b31db9dfb0de05 Reviewed-on: https://chromium-review.googlesource.com/855736 Reviewed-by: John Abd-El-Malek <jam@chromium.org> Commit-Queue: Nicholas Verne <nverne@chromium.org> Cr-Commit-Position: refs/heads/master@{#528107} [modify] https://crrev.com/e5a6661846ddacc04ac6624c9eed6e7353497ec9/chrome/browser/media/cdm_storage_id.cc [modify] https://crrev.com/e5a6661846ddacc04ac6624c9eed6e7353497ec9/content/browser/image_capture/image_capture_impl.cc [modify] https://crrev.com/e5a6661846ddacc04ac6624c9eed6e7353497ec9/content/browser/renderer_host/media/service_video_capture_provider.cc [modify] https://crrev.com/e5a6661846ddacc04ac6624c9eed6e7353497ec9/content/renderer/media/mojo_audio_output_ipc.cc [modify] https://crrev.com/e5a6661846ddacc04ac6624c9eed6e7353497ec9/extensions/browser/api/hid/hid_api.cc [modify] https://crrev.com/e5a6661846ddacc04ac6624c9eed6e7353497ec9/extensions/browser/api/hid/hid_device_manager.cc [modify] https://crrev.com/e5a6661846ddacc04ac6624c9eed6e7353497ec9/extensions/browser/api/serial/serial_connection.cc [delete] https://crrev.com/5e312a0a80f5358bdf5d75676ab4d4661238aad2/extensions/utility/scoped_callback_runner.h [modify] https://crrev.com/e5a6661846ddacc04ac6624c9eed6e7353497ec9/media/DEPS [modify] https://crrev.com/e5a6661846ddacc04ac6624c9eed6e7353497ec9/media/base/BUILD.gn [delete] https://crrev.com/5e312a0a80f5358bdf5d75676ab4d4661238aad2/media/base/scoped_callback_runner.h [delete] https://crrev.com/5e312a0a80f5358bdf5d75676ab4d4661238aad2/media/base/scoped_callback_runner_unittest.cc [modify] https://crrev.com/e5a6661846ddacc04ac6624c9eed6e7353497ec9/media/gpu/android/video_frame_factory_impl.cc [modify] https://crrev.com/e5a6661846ddacc04ac6624c9eed6e7353497ec9/media/mojo/clients/mojo_decryptor.cc [modify] https://crrev.com/e5a6661846ddacc04ac6624c9eed6e7353497ec9/media/mojo/services/mojo_cdm_file_io.cc [modify] https://crrev.com/e5a6661846ddacc04ac6624c9eed6e7353497ec9/media/mojo/services/mojo_cdm_helper.cc [modify] https://crrev.com/e5a6661846ddacc04ac6624c9eed6e7353497ec9/media/mojo/services/mojo_cdm_proxy.cc [modify] https://crrev.com/e5a6661846ddacc04ac6624c9eed6e7353497ec9/media/mojo/services/mojo_media_drm_storage.cc [modify] https://crrev.com/e5a6661846ddacc04ac6624c9eed6e7353497ec9/services/video_capture/device_media_to_mojo_adapter.cc [modify] https://crrev.com/e5a6661846ddacc04ac6624c9eed6e7353497ec9/services/video_capture/virtual_device_mojo_adapter.cc |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by dcheng@chromium.org
, Oct 17 2017