FWIW I think that requiring each client (i.e. chrome and ash) to explicitly declare which clients to initialize (e.g. by defining a DBusClientBundle) would be entirely reasonable. 

Maybe ash should just have its own version of DBusThreadManager?
DBusThreadManager is just a small amount of code for thread management + D-Bus client bundle, and I guess there is not much code to share between Chrome OS and ash (while D-Bus client classes can be reused by ash without modification).

Hmm. I need a way to tell chrome to skip initialization of the GSM_SMS and SMS services when chrome is running in mustash. So I think I need to provide support for partial initialization, which I could reuse in ash.

DBusThreadManager has a decent amount of logic to haandle real vs. fake implementations, we may as well share it imho, especiaally since we already have DBusClientBundle which should make it pretty easy to extend the class to be configurable by Chrome and by Ash.

Ash having its own version of DBusThreadManager, or ash just reusing Chrome's DBusThreadManager sounds good to me, but I'm not sure if it's good to make DBusThreadManager configurable.

As I wrote in https://codereview.chromium.org/2319783002/, existing code doesn't expect DBusThreadManager to return nullptr.
Shouldn't we just disable SMS code in Chrome when running with mash?

Commentary from the code review (which I might have to abandon):

So I need to do a couple things in mustash:

1) Disable SMS in chrome. In particular, I don't want chrome to respond to D-Bus
signals on the org.freedesktop.ModemManager.Modem.Gsm.SMS interface.

2) Disable a large number of services in ash. For example, I don't want ash to
respond to CrosDisks mount complete signals or cryptohome low-disk-space
signals.

It might work to initialize all the clients in both chrome and in ash, then rely
on the higher level wrappers not to use them (for example, rely on the fact that
no one calls DiskMountManager::Initialize() in ash, so it won't call
CrosDisksClient::SetMountCompletedHandler, so nothing will respond to the mount
completed event). However, it seems wasteful to initialize a bunch of clients
that are never used. It's also hard to me to determine by inspecting the code
that nothing will go wrong if ash receives a signal from a service it doesn't
use. My primary concern is that both chrome and ash will respond to a signal and
confuse the underlying service.

So I would prefer to skip initializing services I don't use.

<snip>

...we can utilize compilers, linkers, and DEPS
checker during build time to avoid accessing D-Bus clients from a wrong process.
I think it's much better than hitting random SIGSEGV at runtime.

Just to be clear, I don't think you need to worry about Chrome "responding" to SMS D-Bus signals. Signals are broadcast to every D-Bus client that's listening for them, so it'd probably be harmless for Chrome to receive the signals but not do anything in response to them. With that said, if the SMS service doesn't play nicely with multiple callers, it seems good to make it clear that Chrome shouldn't be using it by not instantiating the SMS client class in Chrome.

So I tried separating out DBusThreadManager vs. DBusThreadManagerAsh vs. DBusThreadManagerChrome. Unfortunately, there are a large number of places outside of //chrome that use services owned by the browser process and a large number of places outside of //ash that use services shared between the browser and ash processes. This means DBTMC can't live in //chrome and DBTMA can't live in //ash.

I chatted with derat@ (since stevenjb@ is OOO and it's Saturday for hashimoto@). We think a better solution might be to follow the pattern of //content/browser vs. //content/renderer vs. //content/common and have a per-process split in //chromeos. That lets both the dbus client code and higher level "handler" code live in a directory by process, and makes the DEPS more clear.

I'm going to try that in a new CL.

Thanks, utilizing DEPS while keeping everything under //chromeos sounds practical to me.
It can get complicated when we are to split Chrome into more than 2 processes (i.e. Chrome + Ash + <Another Chrome component in a new process>), but
it shouldn't matter for this mash work.

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

commit 2b590999a25934d555ca191a2ad2f3e802a86203
Author: jamescook <jamescook@chromium.org>
Date: Wed Sep 14 15:28:45 2016

chromeos: Refactor D-Bus client type enum and stub vs. fake naming

This is needed for other //chromeos/dbus refactoring for mus+ash.

* Rename stub to fake in DBusThreadManager and DBusClientBundle for consistency
with the Fake*Client implementations
* Extract DBusClientType into its own file
* Move --dbus-unstub-clients parsing into the same file so that the client name
strings are near the enum
* Add some simple unit tests for DBusThreadManager initialization

No functional changes.

BUG= 644414 
TEST=chromeos_unittests
TBR=peter@chromium.org

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

[modify] https://crrev.com/2b590999a25934d555ca191a2ad2f3e802a86203/ash/mus/window_manager_application.cc
[modify] https://crrev.com/2b590999a25934d555ca191a2ad2f3e802a86203/ash/test/ash_test_helper.cc
[modify] https://crrev.com/2b590999a25934d555ca191a2ad2f3e802a86203/chrome/browser/chromeos/chrome_browser_main_chromeos.cc
[modify] https://crrev.com/2b590999a25934d555ca191a2ad2f3e802a86203/chrome/browser/chromeos/login/chrome_restart_request.cc
[modify] https://crrev.com/2b590999a25934d555ca191a2ad2f3e802a86203/chrome/browser/chromeos/policy/device_cloud_policy_manager_chromeos_unittest.cc
[modify] https://crrev.com/2b590999a25934d555ca191a2ad2f3e802a86203/chrome/browser/extensions/api/easy_unlock_private/easy_unlock_private_api_chromeos_unittest.cc
[modify] https://crrev.com/2b590999a25934d555ca191a2ad2f3e802a86203/chrome/browser/metrics/chromeos_metrics_provider_unittest.cc
[modify] https://crrev.com/2b590999a25934d555ca191a2ad2f3e802a86203/chrome/test/base/testing_io_thread_state.cc
[modify] https://crrev.com/2b590999a25934d555ca191a2ad2f3e802a86203/chrome/test/base/view_event_test_platform_part_chromeos.cc
[modify] https://crrev.com/2b590999a25934d555ca191a2ad2f3e802a86203/chromeos/chromeos.gyp
[modify] https://crrev.com/2b590999a25934d555ca191a2ad2f3e802a86203/chromeos/chromeos_switches.cc
[modify] https://crrev.com/2b590999a25934d555ca191a2ad2f3e802a86203/chromeos/chromeos_switches.h
[modify] https://crrev.com/2b590999a25934d555ca191a2ad2f3e802a86203/chromeos/dbus/cros_disks_client.cc
[modify] https://crrev.com/2b590999a25934d555ca191a2ad2f3e802a86203/chromeos/dbus/cros_disks_client.h
[modify] https://crrev.com/2b590999a25934d555ca191a2ad2f3e802a86203/chromeos/dbus/dbus_client.h
[modify] https://crrev.com/2b590999a25934d555ca191a2ad2f3e802a86203/chromeos/dbus/dbus_client_bundle.cc
[modify] https://crrev.com/2b590999a25934d555ca191a2ad2f3e802a86203/chromeos/dbus/dbus_client_bundle.h
[modify] https://crrev.com/2b590999a25934d555ca191a2ad2f3e802a86203/chromeos/dbus/dbus_client_bundle_unittest.cc
[delete] https://crrev.com/f19502008401dc093629f3ca082f82edbc47e475/chromeos/dbus/dbus_client_implementation_type.h
[add] https://crrev.com/2b590999a25934d555ca191a2ad2f3e802a86203/chromeos/dbus/dbus_client_types.cc
[add] https://crrev.com/2b590999a25934d555ca191a2ad2f3e802a86203/chromeos/dbus/dbus_client_types.h
[modify] https://crrev.com/2b590999a25934d555ca191a2ad2f3e802a86203/chromeos/dbus/dbus_thread_manager.cc
[modify] https://crrev.com/2b590999a25934d555ca191a2ad2f3e802a86203/chromeos/dbus/dbus_thread_manager.h
[add] https://crrev.com/2b590999a25934d555ca191a2ad2f3e802a86203/chromeos/dbus/dbus_thread_manager_unittest.cc
[modify] https://crrev.com/2b590999a25934d555ca191a2ad2f3e802a86203/chromeos/dbus/power_manager_client.cc
[modify] https://crrev.com/2b590999a25934d555ca191a2ad2f3e802a86203/chromeos/dbus/power_manager_client.h
[modify] https://crrev.com/2b590999a25934d555ca191a2ad2f3e802a86203/chromeos/dbus/session_manager_client.cc
[modify] https://crrev.com/2b590999a25934d555ca191a2ad2f3e802a86203/chromeos/dbus/session_manager_client.h
[modify] https://crrev.com/2b590999a25934d555ca191a2ad2f3e802a86203/chromeos/dbus/update_engine_client.cc
[modify] https://crrev.com/2b590999a25934d555ca191a2ad2f3e802a86203/chromeos/dbus/update_engine_client.h
[modify] https://crrev.com/2b590999a25934d555ca191a2ad2f3e802a86203/content/shell/browser/shell_browser_main_parts.cc
[modify] https://crrev.com/2b590999a25934d555ca191a2ad2f3e802a86203/extensions/shell/browser/shell_browser_main_parts.cc

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

commit dd6183891740c725a872def6aaf07e51d4f2d723
Author: jamescook <jamescook@chromium.org>
Date: Mon Sep 19 16:42:08 2016

chromeos: Refactor D-Bus client creation for ash and browser processes

In mustash we need to have some D-Bus clients in the ash window manager
process and some in the browser process. In traditional ash we need everything
in the browser process. Allow per-process initialization of subsets of clients.

* Split ownership of clients into DBusClientsBrowser, DBusClientsAsh and
DBusClientsCommon
* Make DBusThreadManager::Initialize() take a process enum. This isn't great,
but see code review comments
* Remove unnecessary setters from DBusThreadManagerSetter
* Use ash/DEPS to restrict which clients can be used in //ash
* Only initialize ash clients in ash_unittests
* When running in mustash, limit which clients are initialized in the ash
and browser processes

This is a transitional step toward cleaner multi-process initialization of
clients as discussed in go/chromeos-dbus-clients option (G)

BUG= 644414 ,647367
TEST=chromeos_unittests, ash_unittests

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

[modify] https://crrev.com/dd6183891740c725a872def6aaf07e51d4f2d723/ash/DEPS
[modify] https://crrev.com/dd6183891740c725a872def6aaf07e51d4f2d723/ash/mus/window_manager_application.cc
[modify] https://crrev.com/dd6183891740c725a872def6aaf07e51d4f2d723/ash/shell/content/client/shell_browser_main_parts.cc
[modify] https://crrev.com/dd6183891740c725a872def6aaf07e51d4f2d723/ash/test/ash_test_helper.cc
[modify] https://crrev.com/dd6183891740c725a872def6aaf07e51d4f2d723/chrome/browser/chromeos/chrome_browser_main_chromeos.cc
[modify] https://crrev.com/dd6183891740c725a872def6aaf07e51d4f2d723/chromeos/chromeos.gyp
[modify] https://crrev.com/dd6183891740c725a872def6aaf07e51d4f2d723/chromeos/dbus/dbus_client.h
[delete] https://crrev.com/da9e827b14a9f37c7488893768dd910698cdecd5/chromeos/dbus/dbus_client_bundle.h
[modify] https://crrev.com/dd6183891740c725a872def6aaf07e51d4f2d723/chromeos/dbus/dbus_client_bundle_unittest.cc
[add] https://crrev.com/dd6183891740c725a872def6aaf07e51d4f2d723/chromeos/dbus/dbus_clients_browser.cc
[add] https://crrev.com/dd6183891740c725a872def6aaf07e51d4f2d723/chromeos/dbus/dbus_clients_browser.h
[rename] https://crrev.com/dd6183891740c725a872def6aaf07e51d4f2d723/chromeos/dbus/dbus_clients_common.cc
[add] https://crrev.com/dd6183891740c725a872def6aaf07e51d4f2d723/chromeos/dbus/dbus_clients_common.h
[modify] https://crrev.com/dd6183891740c725a872def6aaf07e51d4f2d723/chromeos/dbus/dbus_thread_manager.cc
[modify] https://crrev.com/dd6183891740c725a872def6aaf07e51d4f2d723/chromeos/dbus/dbus_thread_manager.h
[modify] https://crrev.com/dd6183891740c725a872def6aaf07e51d4f2d723/chromeos/dbus/dbus_thread_manager_unittest.cc

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

commit 9be05a4b2ac43b54230f6c27711882176c377788
Author: jamescook <jamescook@chromium.org>
Date: Mon Sep 19 19:09:49 2016

chromeos; Remove unused switch --dbus-unstub-clients / --dbus-real-clients

These are unused in the chromium and chromiumos source trees and no one on
the mailing list is actually using them. Remove them significantly simplifies
D-Bus client initialization -- only the D-Bus thread creation needs to stay
centralized.

* Remove DBusClientTypes and the string representations of them
* Delete the parser for the switch
* Initialize the client bundles as either all real or all fake
* Remove a bunch of Init/Create methods from DBusThreadManager since there
are fewer conditional cases

BUG=647367, 644414 
TEST=chromeos_unittests, ash_unittests, unit_tests
TBR=tbarzic@chromium.org for easy_unlock

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

[modify] https://crrev.com/9be05a4b2ac43b54230f6c27711882176c377788/ash/mus/window_manager_application.cc
[modify] https://crrev.com/9be05a4b2ac43b54230f6c27711882176c377788/ash/test/ash_test_helper.cc
[modify] https://crrev.com/9be05a4b2ac43b54230f6c27711882176c377788/chrome/browser/chromeos/chrome_browser_main_chromeos.cc
[modify] https://crrev.com/9be05a4b2ac43b54230f6c27711882176c377788/chrome/browser/chromeos/login/chrome_restart_request.cc
[modify] https://crrev.com/9be05a4b2ac43b54230f6c27711882176c377788/chrome/browser/chromeos/policy/device_cloud_policy_manager_chromeos_unittest.cc
[modify] https://crrev.com/9be05a4b2ac43b54230f6c27711882176c377788/chrome/browser/extensions/api/easy_unlock_private/easy_unlock_private_api_chromeos_unittest.cc
[modify] https://crrev.com/9be05a4b2ac43b54230f6c27711882176c377788/chrome/test/base/testing_io_thread_state.cc
[modify] https://crrev.com/9be05a4b2ac43b54230f6c27711882176c377788/chrome/test/base/view_event_test_platform_part_chromeos.cc
[modify] https://crrev.com/9be05a4b2ac43b54230f6c27711882176c377788/chromeos/chromeos.gyp
[modify] https://crrev.com/9be05a4b2ac43b54230f6c27711882176c377788/chromeos/chromeos_switches.cc
[modify] https://crrev.com/9be05a4b2ac43b54230f6c27711882176c377788/chromeos/chromeos_switches.h
[modify] https://crrev.com/9be05a4b2ac43b54230f6c27711882176c377788/chromeos/dbus/cros_disks_client.h
[delete] https://crrev.com/8406717810569b784d50d9a907ac306586b5f6aa/chromeos/dbus/dbus_client_bundle_unittest.cc
[add] https://crrev.com/9be05a4b2ac43b54230f6c27711882176c377788/chromeos/dbus/dbus_client_implementation_type.h
[delete] https://crrev.com/8406717810569b784d50d9a907ac306586b5f6aa/chromeos/dbus/dbus_client_types.cc
[delete] https://crrev.com/8406717810569b784d50d9a907ac306586b5f6aa/chromeos/dbus/dbus_client_types.h
[modify] https://crrev.com/9be05a4b2ac43b54230f6c27711882176c377788/chromeos/dbus/dbus_clients_browser.cc
[modify] https://crrev.com/9be05a4b2ac43b54230f6c27711882176c377788/chromeos/dbus/dbus_clients_browser.h
[modify] https://crrev.com/9be05a4b2ac43b54230f6c27711882176c377788/chromeos/dbus/dbus_clients_common.cc
[modify] https://crrev.com/9be05a4b2ac43b54230f6c27711882176c377788/chromeos/dbus/dbus_clients_common.h
[modify] https://crrev.com/9be05a4b2ac43b54230f6c27711882176c377788/chromeos/dbus/dbus_thread_manager.cc
[modify] https://crrev.com/9be05a4b2ac43b54230f6c27711882176c377788/chromeos/dbus/dbus_thread_manager.h
[modify] https://crrev.com/9be05a4b2ac43b54230f6c27711882176c377788/chromeos/dbus/dbus_thread_manager_unittest.cc
[modify] https://crrev.com/9be05a4b2ac43b54230f6c27711882176c377788/chromeos/dbus/power_manager_client.h
[modify] https://crrev.com/9be05a4b2ac43b54230f6c27711882176c377788/chromeos/dbus/session_manager_client.h
[modify] https://crrev.com/9be05a4b2ac43b54230f6c27711882176c377788/chromeos/dbus/update_engine_client.h
[modify] https://crrev.com/9be05a4b2ac43b54230f6c27711882176c377788/content/shell/browser/shell_browser_main_parts.cc
[modify] https://crrev.com/9be05a4b2ac43b54230f6c27711882176c377788/extensions/shell/browser/shell_browser_main_parts.cc

On hold for a bit to work on ash <-> chrome communication. Will pick up again soon when working on networking UI.

Done enough for Q1. Back in the pile.

Can't we say this is done?