Code review comment for lp:~brandontschaefer/mir/client-api-apply-remove-display-config

13 + * to get notified about hardware changes, so that the can apply a new config.
Typo, the→they

15 + * \warning This request may be denied. Check that the request succeeded with mir_connection_get_error_message.
You've copy-pasted these warnings. They don't apply; clients get notified via their registered error handler.

252 + mir::protobuf::Void * response,
Nit: Coding style

291 + virtual void remove(std::shared_ptr<Session> const&) = 0;
Not wild on the naming - could this be remove_session_configuration?

336 + else if ("remove_display" == invocation.method_name())
And likewise, "remove_display_configuration" (or some equivalent)?

532 +void ms::MediatingDisplayChanger::remove(
533 + std::shared_ptr<mf::Session> const& session)
534 +{
535 + std::lock_guard<std::mutex> lg{configuration_mutex};
536 + config_map.erase(session);
537 + observer->session_configuration_removed(session);
538 +}
Shouldn't this result in a hardware configuration change if (and only if) the session is currently focused? This will result in the configuration being set to the base config the *next* time the session is focused.

Which leads me into: please add some acceptance tests testing the expected behaviour. Particularly:
1) That calling remove_session_display_config from a client that hasn't set a session configuration is a no-op
2) That calling remove_session_display_config from a *focused* client that *has* set a session configuration results in a hardware configuration change
3) That calling remove_session_display_config from an *unfocused* client that *has* set a session configuration results in *no* hardware configuration change
4) That calling remove_session_display_config from an *unfocused* client that *has* set a session configuration results in the base configuration getting applied when the client becomes focused.