I can only speak to the powerd (PowerManagerClient) side of this. I think that most of it would still work with instances of the class in both Chrome and ash, but the HandlePowerButtonAcknowledgment calls should only be made by one of the two (ash?). kSuspendDelayDescription should be "ash" instead of "chrome" when instantiated in ash. I'm not sure whether both processes will need to continue registering suspend delays, but it seems likely that they might (e.g. ash for display configuration, Chrome for locking the screen (or is that in ash?) and freezing renderers).

I think that every class in chromeos/dbus/ will need this level of scrutiny. :-(

Unfortunately I that we will need to scrutinize each chromeos/dbus service, which will not be trivial.

Many (most?) DBus services will be accessed by both the Status Area (which presumably will be in Ash) and Settings (which will presumably be in Chrome, or someday-maybe a separate app, but a separate executable in either case).

For Networking, there *shouldn't* be any problem running two independent NetworkStateHandler layers since we rely on Shill as the authoritative model, but we do cache some state data, which *should* get updated with any Shill change, but we obviously haven't tested that.

NOTE: At minimum this will double the networking related DBus traffic, which is not trivial, especially while scanning (i.e. actively updating the list of visible networks when networking UI is visible).

I also agree that Option 2 will buy us little if anything. Using mojo instead of dbus won't necessarily fix any caching or other state related problems we may have.

That said, I also have no idea whether or not Shill will support multiple clients (i.e. Chrome + Ash). I would expect that to "just work" but I don't actually know.

+rkc who has an entirely different plan in his app_shell design doc (go/chrome-appshell), including a transitional state where system services move into app_shell, then move into a separate system services process (option 2).

He mentioned to me a couple days ago that he anticipated problems if more than one client talked to bluez, and that the failure modes would be subtle, though I don't recall the details.

Hmm. I would be concerned about the labor involved in creating (in particular) new/additional bluetooth and networking communication layers. 

It is certainly doable. For networking and bluetooth we have greatly improved the extension APIs (in .idl format) for Settings, which could theoretically be used to create Mojo interfaces, which would be great.

However, from experience such a conversion is going to take a lot of time and effort.

The plan described in the AppShell design is long term and we do expect it be a considerable amount of work. It is the correct solution though. "Eventually", system services should be in their own process(es), and should be accessed via well defined mojo interfaces.

That being said, since the Chrome/Ash process split is more immediate, we may have to figure out a shorter term solution.

My concern is more than just DBus. A lot of Bluetooth code itself has been written with the assumption that it has exclusive access to the adapter. I am uncertain how much code will break, but I would be very nervous about letting two different sets of BlueZ clients speak to BlueZ. My concern is that if bugs exist in this scenario, they could often be very hard to find due to them manifesting exclusively in race conditions. I don't think BlueZ itself has ever been tested with more than one client using it.


There could be simpler options such as defining a minimal Mojo interface for Ash to use that Chrome provides that will allow Ash to do what it needs to with Bluetooth. Even if we put in some throwaway work, I would strongly recommend that we keep all the code that speaks to BlueZ only in one process.

+satorux

One more thing to consider: Chrome itself acts as a D-Bus service under the name "org.chromium.LibCrosService" to expose methods and send signals. The same D-Bus service name cannot be used by multiple processes. In other words, the D-Bus service should be provided by a single process. 

#8: That's a good point. I thought that all that code lived inside chrome/browser/, but much of it is actually in chromeos/dbus/ and will probably fail if all of that code is instantiated by two different processes.

jamescook: https://cs.chromium.org/chromium/src/chromeos/dbus/services/cros_dbus_service.h?q=LibCrosService&sq=package:chromium&dr=C&l=25 is where the D-Bus service code in Chrome is located.

If Chrome browser wants ability to quit in mus+ash, the D-Bus service may need to be provided by something else.

Note to self: The Chrome D-Bus service currently provides:

* Proxy resolution (e.g. for update engine)
* Liveness checking (is chrome UI thread responsive)
* Display power on/off/dim
* Console service (release the display so the console can take it)

In the long term we want chrome to quit, but in the near term many services will still come from chrome.

 Issue 606400  has been merged into this issue.

Per an internal email thread with adlr, derat, puneet, zel, rockot:

* mojo in chrome is still under heavy active development. The public bindings API is not yet stable enough to start exporting it to use in other system services. Eventually it will be, but we don't want to convert system services to mojo right now. We'll stick with D-Bus.
* Regardless of whether the services speak mojo or D-Bus, some of them will need to be changed to support multiple clients.

I'll start a design doc and ask people familiar with each service to fill in details.

The system service I have the most intimate knowledge of is BlueZ. In its context, I have a few observations. Since I don't have the context or details of the discussion, I'll try to interpret the points based on a few assumptions.


"The public bindings API is not yet stable enough to start exporting it to use in other system services."

This seems to imply that the plan was to use Mojo to communicate with system services? If so, with BlueZ, this won't work, I would say, ever. Not without completely re-writing it. That or we would need to switch to a Bluetooth stack that we completely own.

"* Regardless of whether the services speak mojo or D-Bus, some of them will need to be changed to support multiple clients"

Even if we made BlueZ work with multiple clients, we'll be venturing into a territory which is virtually unexplored, with heavily untested and code with which we have severe quality concerns already.


Other system services may or may not have such severe issues with switching to a multi-client mode. Owners of those services would have much more context.

For BlueZ we might need to keep the existing D-Bus client in the chrome browser process, then create a minimal mojo service around it that allows mojo:ash to get what it needs.

For other daemons I hope to avoid creating wrappers. We'll see as we look deeper.

FWIW, I actually like the idea of creating well thought out mojo APIs ("wrappers") for DBus services that are accessed from both Ash and Chrome (e.g. networking and bluetooth, likely others as well). My reasons are:

1. It is an opportunity to create well considered client focused APIs that can be accessed from C++ or JS, in Chrome, Ash, or even Arc.

2. The mojo service that makes DBus calls could live in whatever process makes the most sense and could (in theory) be moved from one to another (or even to the services themselves, e.g. Shill) without changing the clients.

We already have, effectively, a C++ wrapper in Chrome for networking (Network*Handler). We even have an extension API (networkingPrivate), based on a well considered spec ('ONC'), that covers about 90% of what the NetworkHandler classes provide. Converting the extension API to Mojo should (theoretically) be straightforward.

We also already have a pair of bluetooth extension APis ('bluetooth' and 'bluetoothPrivate'). I suspect that extending these will be more work, but we are looking at a fair bit of work regardless.

Based on my experience with maintaining ash::SystemTrayDelegate, I would very much prefer to avoid introducing any chrome <-> ash specific delegates for this, even using mojo.

i.e. if we create a "minimal mojo service" for ash hosed by chrome, I would suggest doing so with a fully defined bluetooth configuration API in mind. Implementing this incrementally makes lots of sense, but eventually I would expect this service to run in Ash instead of Chrome (or in a dedicated service). It would be great to build the API with that in mind from the start.

There's a tradeoff in complexity:

1) Mojo wrapper around a D-Bus service in chrome: Find all call sites to D-Bus code in ash and convert them to speak mojo to chrome. This will be an async "delegate" over IPC. Chrome's service implementation will need to handle multiple clients (itself and ash). It also means another wrapper we have to maintain.

2) Change system service to support multiple D-Bus clients. This implies fewer changes in Chrome, more changes in the service, but we don't have to pay for a new wrapper.

I think we'll have to make that decision per-service. I will look at the code and talk to you guys with more details.

I'm hopeful that most system services written for Chrome OS are already happy to have multiple D-Bus clients. The general model in the ones that I've worked with is to use signals to announce events and changes (multiple clients can listen for these) and accept method calls to perform actions (which I guess could be tricky if those actions are performed asynchronously and oughtn't overlap).

Recommendations are at the bottom of design doc go/dbus-in-mustash (This is the doc I sent around in the internal threads).

I'll give it a day or two for comments, then start filing bugs for individual work items.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2ffdb659830b0b1a5875f2357c2f8c5b8beacc6a

commit 2ffdb659830b0b1a5875f2357c2f8c5b8beacc6a
Author: jamescook <jamescook@chromium.org>
Date: Tue Sep 20 18:36:00 2016

mustash: Restrict //ash/DEPS to services available in ash process

Only a subset of D-Bus clients and services are availabe in the mustash ash
process. Use DEPS to restrict access to the ones that are allowed.

BUG=629707
TEST=presubmit

Review-Url: https://codereview.chromium.org/2352973003
Cr-Commit-Position: refs/heads/master@{#419824}

[modify] https://crrev.com/2ffdb659830b0b1a5875f2357c2f8c5b8beacc6a/ash/DEPS
[modify] https://crrev.com/2ffdb659830b0b1a5875f2357c2f8c5b8beacc6a/ash/common/system/chromeos/DEPS
[modify] https://crrev.com/2ffdb659830b0b1a5875f2357c2f8c5b8beacc6a/ash/system/DEPS
[modify] https://crrev.com/2ffdb659830b0b1a5875f2357c2f8c5b8beacc6a/ash/system/chromeos/DEPS

Available, just to make it clear people can pick up subtasks.

Bug scrub.