Allow SequencedTaskRunner argument in InterfacePtr::Bind(). |
||
Issue description
Today, InterfacePtr::Bind() accepts a SingleThreadTaskRunner argument to use to dispatch callbacks and error notifications. This is useful to bind an interface to a specific task queue of the Blink scheduler. If someone wants to bind an interface to a SequencedTaskRunner which isn't a SingleThreadTaskRunner, they have to leave the argument nullptr and rely on the default behavior of using SequencedTaskRunnerHandle::Get().
This is inconvenient for classes that support running on a SequencedTaskRunner *or* a SingleThreadTaskRunner and want to bind an interface. Simplified example:
class Foo {
public:
// Foo sometimes runs on a Blink scheduler task queue, in which case a
// SingleThreadTaskRunner will be passed as argument. But it can also
// run on a TaskScheduler SequencedTaskRunner.
Foo(scoped_refptr<SequencedTaskRunner> task_runner)
: task_runner_(std::move(task_runner)) {}
void DoSomething() {
task_runner_->PostTask(FROM_HERE, ...);
}
void BindSomething() {
// Impossible! Because |task_runner_| is not a SingleThreadTaskRunner.
// Foo shouldn't have to know whether it is running on a SingleThreadTaskRunner
// or a SequencedTaskRunner.
interface_.Bind(..., task_runner_);
}
private:
MyInterfacePtr interface_;
scoped_refptr<SequencedTaskRunner> task_runner_;
}
Real example: https://cs.chromium.org/chromium/src/content/renderer/loader/url_loader_client_impl.cc?l=228&rcl=aed427b076ba209b24b49a72ce9d7c4637984029
ResourceDispatcher could very well run a SequencedTaskRunner without this restriction. https://cs.chromium.org/chromium/src/content/renderer/loader/resource_dispatcher.cc?l=507&rcl=ad8bf3d60f97558c2f8a15db74d7ded3d16224eb
Because it makes it harder to have classes that can run both on a SingleThreadTaskRunner and a SequencedTaskRunner, InterfacePtr::Bind() should not have a SingleThreadTaskRunner argument. It should have a SequencedTaskRunner argument.
,
May 17 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6ac0dc16d307483b88be911c791d3fccfb8516f4 commit 6ac0dc16d307483b88be911c791d3fccfb8516f4 Author: Francois Doray <fdoray@chromium.org> Date: Thu May 17 17:13:19 2018 Allow SequencedTaskRunner argument in InterfacePtr::Bind(). Discussion at: https://groups.google.com/a/chromium.org/d/msg/chromium-mojo/PxJTGC-xZmQ/Jx04J27RAwAJ Today, InterfacePtr::Bind() accepts a SingleThreadTaskRunner argument to use to dispatch callbacks and error notifications. This is useful to bind an interface to a specific task queue of the Blink scheduler. If someone wants to bind an interface to a SequencedTaskRunner which isn't a SingleThreadTaskRunner, they have to leave the argument nullptr and rely on the default behavior of using SequencedTaskRunnerHandle::Get(). This is inconvenient for classes that support running on a SequencedTaskRunner *or* a SingleThreadTaskRunner and want to bind an interface. Simplified example: class Foo { public: // Foo sometimes runs on a Blink scheduler task queue, in which case a // SingleThreadTaskRunner will be passed as argument. But it can also // run on a TaskScheduler SequencedTaskRunner. Foo(scoped_refptr<SequencedTaskRunner> task_runner) : task_runner_(std::move(task_runner)) {} void DoSomething() { task_runner_->PostTask(FROM_HERE, ...); } void BindSomething() { // Impossible! Because |task_runner_| is not a SingleThreadTaskRunner. // Foo shouldn't have to know whether it is running on a // SingleThreadTaskRunner or a SequencedTaskRunner. interface_.Bind(..., task_runner_); } private: MyInterfacePtr interface_; scoped_refptr<SequencedTaskRunner> task_runner_; } This CL fixes the issue by changing the argument type of InterfacePtr::Bind() and AssociatedInterfacePtr::Bind() to SequencedTaskRunner. Bug: 844039 Change-Id: Ie0febeccaf002299e5a771d3ac885fb9cd18db8b Reviewed-on: https://chromium-review.googlesource.com/1064311 Reviewed-by: Ken Rockot <rockot@chromium.org> Commit-Queue: François Doray <fdoray@chromium.org> Cr-Commit-Position: refs/heads/master@{#559581} [modify] https://crrev.com/6ac0dc16d307483b88be911c791d3fccfb8516f4/mojo/public/cpp/bindings/associated_interface_ptr.h [modify] https://crrev.com/6ac0dc16d307483b88be911c791d3fccfb8516f4/mojo/public/cpp/bindings/interface_ptr.h [modify] https://crrev.com/6ac0dc16d307483b88be911c791d3fccfb8516f4/mojo/public/cpp/bindings/lib/associated_interface_ptr_state.cc [modify] https://crrev.com/6ac0dc16d307483b88be911c791d3fccfb8516f4/mojo/public/cpp/bindings/lib/associated_interface_ptr_state.h [modify] https://crrev.com/6ac0dc16d307483b88be911c791d3fccfb8516f4/mojo/public/cpp/bindings/lib/interface_ptr_state.cc [modify] https://crrev.com/6ac0dc16d307483b88be911c791d3fccfb8516f4/mojo/public/cpp/bindings/lib/interface_ptr_state.h [modify] https://crrev.com/6ac0dc16d307483b88be911c791d3fccfb8516f4/mojo/public/cpp/bindings/lib/task_runner_helper.cc [modify] https://crrev.com/6ac0dc16d307483b88be911c791d3fccfb8516f4/mojo/public/cpp/bindings/lib/task_runner_helper.h
,
May 18 2018
|
||
►
Sign in to add a comment |
||
Comment 1 by fdoray@chromium.org
, May 17 2018