New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 897022 link

Starred by 1 user

Issue metadata

Status: Untriaged
Owner: ----
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Callbacks on InterfaceRegistry::AddInterface can be called on the IO thread

Project Member Reported by haraken@google.com, Oct 19

Issue description

There are cases where callbacks on InterfaceRegistry::AddInterface can be called on the IO thread. People are adding CrossThreadBind to make it work.

e.g., https://chromium-review.googlesource.com/c/chromium/src/+/750562/3/third_party/WebKit/Source/controller/BlinkInitializer.cpp#121

This is not nice that many things (e.g., object allocations on Oilpan) are not supported on the IO thread.

Can we make sure that the callbacks are called on some task runner of the main thread?

I think this is the place that calls the callbacks on the IO thread:

https://cs.chromium.org/chromium/src/services/service_manager/public/cpp/interface_binder.h?gsn=AddInterface&g=0&l=62

 
One thing I don't quite remember: IIRC, if we call AddInterface on RFHI's BinderRegistry, is the resulting interface binder invoked on the UI thread task runner or on the IO thread task runner? I seem to recall it's invoked on the UI thread task runner, but I'm not sure how that works...
Right, so the BinderRegistry (it's no longer InterfaceRegistry, that's oooold) which we give to Blink initialization is ultimately added to a connection filter here[1], which means not *all* callbacks are *always* called on the IO thread.

This does seem unfortunate. Also unfortunate is that some callbacks must be called on the IO thread though, because we need to be able to handle them while the main thread is blocked. I assume those cases are all specific to content or chrome though, so we could always introduce a second BinderRegistry to give to blink only.

It probably is just something to keep in mind as we migrate to DocumentInterfaceBroker et al though.

[1] https://cs.chromium.org/chromium/src/content/renderer/render_thread_impl.cc?rcl=510accecfcfe640659c8cb01c5b78adbf8908bfd&l=807

Sign in to add a comment