For the record, the CL to add a media::ScopedCallbackRunner to do what's proposed in this thread is landed in https://chromium-review.googlesource.com/c/562751/On Fri, Apr 7, 2017 at 2:23 AM, Jan van Oort <j...@kivu.tech> wrote:A bit late, but yes - this makes sense. I've had similar cases in my own code. Solution: use a shutdown hook. Which is essentially what you're doing here.On Wed, Mar 29, 2017 at 12:53 AM, Xiaohan Wang (王消寒) <xhw...@chromium.org> wrote:--Hello,There are cases where a caller always expect a callback (OnceCallback) to run to proceed to the next step. However, the callback might be dropped somewhere and never ran. For example, a callback is stored in a RunLoop and got dropped during tear down.One more realistic example is when making a mojo call like this:
void Foo::Bar(ResultCB result_cb) {
foo_ptr_->Bar(base::Bind(&Foo::OnResult, weak_ptr_, base::Passed(&result_cb)));
}If a connection error happened, the callback will be dropped without ever running (correct me if I am wrong).To work around this issue, currently I have to store a callback such that when connection error happens, we can run all stored callbacks:void Foo::Bar(ResultCB result_cb) {result_cb_ = std::move(result_cb);
foo_ptr_->Bar(base::Bind(&Foo::OnResult, weak_ptr_));
}void Foo::OnConnectonError() {if (result_cb_)std::move(result_cb_).Run(false);
}However, this makes code ugly and harder to maintain.I wonder whether it makes sense to introduce a RunOnDestruction() wrapper such that when a callback is destructed without ever running, the callback will be run with some default parameter automatically. So the above code will become:void Foo::Bar(ResultCB result_cb) {
foo_ptr_->Bar(RunOnDestruction(base::Bind(&Foo::OnResult, weak_ptr_, base::Passed(&result_cb)),false // default parameter when running callback during destruction));
}I created a prototype CL to show the implementation with some tests. I also tried this with mojo bindings to demo that this does solve the mojo example above. The CL is for POC only and is not ready for full review.So, do all of these make sense? If we all agree on the problem and solution, I can polish/update my CL for review.Best,Xiaohan
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAF1j9YMQNiiPD0J51Grkbr%2B5tUUmaL-ZoUY617VVqLC%3Dz4foyQ%40mail.gmail.com.
--
You received this message because you are subscribed to the Google Groups "chromium-mojo" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-mojo+unsubscribe@chromium.org.
To post to this group, send email to chromi...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-mojo/040eb23c-6d38-48a6-a024-539b9651c73f%40chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CA%2BapAgGVtrzsnUkgMd1mcYZSWC5_THV-kweZiB_usnd0OVZtsg%40mail.gmail.com.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-moj...@chromium.org.
To post to this group, send email to chromi...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-mojo/040eb23c-6d38-48a6-a024-539b9651c73f%40chromium.org.
--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CA%2BapAgGVtrzsnUkgMd1mcYZSWC5_THV-kweZiB_usnd0OVZtsg%40mail.gmail.com.
--
You received this message because you are subscribed to the Google Groups "chromium-mojo" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-moj...@chromium.org.
To post to this group, send email to chromi...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-mojo/CALekkJcMFaUQz%3DFnM9%2BP0RSOJFN0oEyqNoAZqDd5XK43Z%3DoTSA%40mail.gmail.com.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-mojo+unsubscribe@chromium.org.
To post to this group, send email to chromi...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-mojo/040eb23c-6d38-48a6-a024-539b9651c73f%40chromium.org.
--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.--To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CA%2BapAgGVtrzsnUkgMd1mcYZSWC5_THV-kweZiB_usnd0OVZtsg%40mail.gmail.com.
You received this message because you are subscribed to the Google Groups "chromium-mojo" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-mojo+unsubscribe@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-mojo/CALekkJcMFaUQz%3DFnM9%2BP0RSOJFN0oEyqNoAZqDd5XK43Z%3DoTSA%40mail.gmail.com.
--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAFK_eqT%3DVPFgtzMctnSQOivuL0F3Y8wTspsDaDv9HCSzFo2aMA%40mail.gmail.com.
Another +1 to moving it to base. This would be quite useful for an out-of-tree project I'm working on that uses chromium's base/ for utility classes and mojo/ for IPC.I don't think sequence-affinity should hold up the move. The most common way to use this is to wrap a Mojo result callback, which already has to be executed on the IPC sequence. So adding ScopedCallbackRunner doesn't change the sequence-affinity of the callback, and whatever method the calling code is using to ensure the callback runs on the right sequence will still apply to the wrapped callback.Finding a good callback wrapper for sequence-affinity solves a different problem and should be done on a different layer.I'm not sure about the name, though - it's tempting to treat this as an analogue of ScopedClosureRunner, but they have quite different semantics.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-moj...@chromium.org.
To post to this group, send email to chromi...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-mojo/040eb23c-6d38-48a6-a024-539b9651c73f%40chromium.org.
--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.--To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CA%2BapAgGVtrzsnUkgMd1mcYZSWC5_THV-kweZiB_usnd0OVZtsg%40mail.gmail.com.
You received this message because you are subscribed to the Google Groups "chromium-mojo" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-moj...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-mojo/CALekkJcMFaUQz%3DFnM9%2BP0RSOJFN0oEyqNoAZqDd5XK43Z%3DoTSA%40mail.gmail.com.
--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAFK_eqT%3DVPFgtzMctnSQOivuL0F3Y8wTspsDaDv9HCSzFo2aMA%40mail.gmail.com.
--
You received this message because you are subscribed to the Google Groups "chromium-mojo" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-moj...@chromium.org.
To post to this group, send email to chromi...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-mojo/CAH%3DT95T5vd_9%2B-m6AkCCqYFK%2BjGXqZg_G1TBbJfKqdd2tKEOZA%40mail.gmail.com.
On Wed, Aug 16, 2017 at 10:19 AM Joe Mason <joenot...@chromium.org> wrote:Another +1 to moving it to base. This would be quite useful for an out-of-tree project I'm working on that uses chromium's base/ for utility classes and mojo/ for IPC.I don't think sequence-affinity should hold up the move. The most common way to use this is to wrap a Mojo result callback, which already has to be executed on the IPC sequence. So adding ScopedCallbackRunner doesn't change the sequence-affinity of the callback, and whatever method the calling code is using to ensure the callback runs on the right sequence will still apply to the wrapped callback.Finding a good callback wrapper for sequence-affinity solves a different problem and should be done on a different layer.I'm not sure about the name, though - it's tempting to treat this as an analogue of ScopedClosureRunner, but they have quite different semantics.I'm not opposed to ScopedCallbackRunner. This seems fine.However, I'd prefer leaving sequence affinity out of it. I've been opposed to moving BindToCurrentLoop into //base, since it captures (often non-obvious) global state into a callback. Building a general way to encode sequence affinity into callbacks is problematic as well: it's been my experience in code reviews that this sort of thing can lead to confusing code. Where possible, separating the code into sequence-affine objects and then posting callbacks between them is usually easier to understand (and doesn't incur the cost of creating a trampoline callback just to jump to another task runner).The concerns around Mojo are interesting; how often do we expect to use ScopedCallbackRunner in conjunction with Mojo? Given that Mojo already does the right thing for interface method dispatching, how hard would it be for Mojo to dispatch to the corresponding task runner when disposing these objects?
Daniel
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-mojo+unsubscribe@chromium.org.
To post to this group, send email to chromi...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-mojo/040eb23c-6d38-48a6-a024-539b9651c73f%40chromium.org.
--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.--To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CA%2BapAgGVtrzsnUkgMd1mcYZSWC5_THV-kweZiB_usnd0OVZtsg%40mail.gmail.com.
You received this message because you are subscribed to the Google Groups "chromium-mojo" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-mojo+unsubscribe@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-mojo/CALekkJcMFaUQz%3DFnM9%2BP0RSOJFN0oEyqNoAZqDd5XK43Z%3DoTSA%40mail.gmail.com.
--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
--To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAFK_eqT%3DVPFgtzMctnSQOivuL0F3Y8wTspsDaDv9HCSzFo2aMA%40mail.gmail.com.
You received this message because you are subscribed to the Google Groups "chromium-mojo" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-mojo+unsubscribe@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-mojo/CA%2BapAgHbFBBCnNEJBriZuQv4ZVpBgL6yjDO6nnP2QaEzgsVoQw%40mail.gmail.com.
Daniel
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-moj...@chromium.org.
To post to this group, send email to chromi...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-mojo/040eb23c-6d38-48a6-a024-539b9651c73f%40chromium.org.
--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.--To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CA%2BapAgGVtrzsnUkgMd1mcYZSWC5_THV-kweZiB_usnd0OVZtsg%40mail.gmail.com.
You received this message because you are subscribed to the Google Groups "chromium-mojo" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-moj...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-mojo/CALekkJcMFaUQz%3DFnM9%2BP0RSOJFN0oEyqNoAZqDd5XK43Z%3DoTSA%40mail.gmail.com.
--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
--To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAFK_eqT%3DVPFgtzMctnSQOivuL0F3Y8wTspsDaDv9HCSzFo2aMA%40mail.gmail.com.
You received this message because you are subscribed to the Google Groups "chromium-mojo" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-moj...@chromium.org.
To post to this group, send email to chromi...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-mojo/CAH%3DT95T5vd_9%2B-m6AkCCqYFK%2BjGXqZg_G1TBbJfKqdd2tKEOZA%40mail.gmail.com.
--
You received this message because you are subscribed to the Google Groups "chromium-mojo" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-moj...@chromium.org.
I see there's now two copies of this class checked into chromium in (//extensions/utility/ and //media/base/), plus I have another copy of it in an internal project that uses //base and //mojo. So if there are no objections, I'll go ahead and make a patch to move it into //base.