So Erik and I did some digging and looks like the thread where we receive messages is actually bound here:

https://cs.chromium.org/chromium/src/content/browser/service_manager/common_browser_interfaces.cc?rcl=da392f3cf28ff8d32d5f2a9821c8cece183365ec&l=40

not sure how easy changing that will be.

+chrisha / oysteine, I wonder if that is going to change with https://chromium-review.googlesource.com/c/chromium/src/+/628658 and how?

Embedded Mojo services in the browser run on the main thread by default, yes: https://cs.chromium.org/chromium/src/services/service_manager/embedder/embedded_service_info.h?type=cs&sq=package:chromium&l=22

Passing in another task runner to use should be trivial enough here: https://cs.chromium.org/chromium/src/content/browser/service_manager/service_manager_context.cc?type=cs&sq=package:chromium&l=345


Giving the service its own dedicated thread would probably be overkill, but maybe we can use the luckyluke sequenced worker pool stuff? I'm not sure how this interfaces with mojo.

Other services nearby (e.g., https://cs.chromium.org/chromium/src/content/browser/service_manager/service_manager_context.cc?rcl=a67f5458794fb21bb5e33511544dc6cce158e838&l=338) use the IO thread, that seems a safer (w.r.t. jank) choice.
Any reason / objection against moving the resource coordinator there?

In theory even better would be getting a task_runner from base::Scheduler (LuckyLuke) as you say. But I don't see any precedent for that and I am honestly done with trying out unprecedented stuff with mojo for this quarter.

I had a quick try and of course, it doesn't just work like we'd expected:
I tried this:
https://chromium-review.googlesource.com/c/chromium/src/+/662784/1/content/browser/service_manager/service_manager_context.cc

but the messages are still received on the Main thread :/
+rockot, what is the way to make it so that our service lives on another thread (say, IO)?

That is the correct thing to do. What messages are you seeing dispatched on the main thread? Keep in mind, where the service runs (i.e. where the service_manager::mojom::Service control messages are received) and where your interfaces are actually bound (which is completely at your discretion in the service impl) are two separate things.

RE #5: I added logging to CoordinatorImpl::RequestGlobalMemoryDump, which implements this mojom RPC (https://cs.chromium.org/chromium/src/services/resource_coordinator/public/interfaces/memory_instrumentation/memory_instrumentation.mojom?rcl=e89c48a3666bc7f9bd130c9da14c7f0ca9db5168&l=211)

and I see the event coming from the main thread. What am I doing wrong?

See logging and change in the updated patchset of https://chromium-review.googlesource.com/c/chromium/src/+/662784

Well, you don't appear to be doing anything wrong, and it looks like the service impl just binds incoming interface requests on its own thread.

Here's the code that actually runs your service Create(): https://cs.chromium.org/chromium/src/services/service_manager/embedder/embedded_instance_manager.cc?rcl=af21999b4dc14b078fbf55712dedf5cdb03ee679&l=80

It's fairly straigtforward to verify that if your service is running at all, it must be because an EmbeddedInstanceManager was constructed over your resource_coordinator_info with the task_runner field set. I don't see any bugs that would cause this to happen, but maybe you can dig a little deeper with this info?

Um, what about this?

https://cs.chromium.org/chromium/src/content/browser/browser_main_loop.cc?rcl=af21999b4dc14b078fbf55712dedf5cdb03ee679&l=1449

That is clearly creating a CoordinatorImpl directly on the main thread,
complete outside the context of the resource_coordinator service.

Hold on, at this point I am a bit confused. What is the thing that decides on which task runner the messages will be received?

1) The task runner on which CoordinatorImpl (which in turn creates the base mojom::Coordinator) is created (your link in #8)
2) The task_runner property passed to EmbeddedServiceInfo (see #6)

And how are those two things related?

Every binding endpoint is bound to a sequence. Typically this is a mojo::Binding for impls.

In the case of CoordinatorImpl, it's the BindingSet here: https://cs.chromium.org/chromium/src/services/resource_coordinator/memory_instrumentation/coordinator_impl.h?rcl=81fe9724b9aa939bea960363c3db0863cb11a822&l=146

BindingSet maintains a set of Binding objects, each implicitly bound to the sequence on which the BindingSet calls AddBinding.

So e.g. whatever sequence this is called on, is the sequence the
CoordinatorImpl will receive messages on:
https://cs.chromium.org/chromium/src/services/resource_coordinator/memory_instrumentation/coordinator_impl.cc?rcl=21c870a1afd2ec5efe4f70ea80cb2e2dc800d566&l=124

I see it hooked up here:
https://cs.chromium.org/chromium/src/content/browser/service_manager/common_browser_interfaces.cc?l=27&gs=cpp%253Acontent%253A%253A%253Canonymous-namespace%253E%253A%253ABindMemoryCoordinatorRequest(mojo%253A%253AInterfaceRequest%253Cmemory_instrumentation%253A%253Amojom%253A%253ACoordinator%253E%252C%2Bconst%2Bservice_manager%253A%253ABindSourceInfo%2B%2526)%2540chromium%252F..%252F..%252Fcontent%252Fbrowser%252Fservice_manager%252Fcommon_browser_interfaces.cc%257Cdef&gsn=BindMemoryCoordinatorRequest&ct=xref_usages

And again, this has *nothing to do* with ServiceManagerContext, or
EmbeddedServiceInfo, or the resource_coordinator service. This is a one-off
instance of CoordinatorImpl that is registered with the "content_browser"
service and is bound on the main thread.

As for why that exists at all, I have no idea. Seems like clients should
just be connecting to resource_coordinator.

I suspect something like ChildThreadImpl or RenderThreadImpl must ask for
the interface directly from content_browser.

As a corollary to all this, if you connect to resource_coordinator directly and get this interface, with your patch applied, I'm confident that you'll see messages being dispatched on the IO thread. You'll also see them on the UI thread until you get rid of this other one-off Coordinator impl.

Confused; I thought we were talking about the situation after https://chromium-review.googlesource.com/c/chromium/src/+/628658 lands?

Oh... sorry, I was only talking about primiano's small patch. With that
patch applied as well, the problem should be completely addressed.

I just patched both Primiano's and the other big CL in. Looks like with both, it the trace events from Primiano's CL happen on the IO thread which I guess is as expected?

Yes

primiano@ I've verified that landing your CL fixes this issue so assigning you so you can take a look at some point!