Created attachment 279205 [details] [review]
sharing: use UUIDs instead of IDs for NM connection identification

Despite the name, the connection ID is a human readable string
and is not guaranteed to be unique.

Created attachment 279206 [details] [review]
sharing: include wwan connection types as broadband

For wwan modems directly attached to the computer.

Created attachment 279207 [details] [review]
sharing: fix deactivation of services when changing network

When switching between networks that both allow sharing in general,
we need to check the enabled connections for each service and
possibly stop it.

Created attachment 279208 [details] [review]
sharing: don't shutdown rygel if not autostarted

Rygel is also dbus activated, so we should not call it unless
we know it is running.

Review of attachment 279208 [details] [review]:

Looks good.

Review of attachment 279207 [details] [review]:

How do you switch between networks without going through the "" (empty) network, and thus bringing down all services?

Review of attachment 279206 [details] [review]:

Yes.

Review of attachment 279205 [details] [review]:

That breaks the UI. The UUIDs would be shown directly in the UI.

(In reply to comment #6)
> Review of attachment 279207 [details] [review]:
> 
> How do you switch between networks without going through the "" (empty)
> network, and thus bringing down all services?

You can switch primary without disconnecting the old one - for example by plugging a cable while connected to wifi. NM will switch the default route when the cable is set up (thus changing PrimaryConnection), but will not disconnect wifi automatically.

(In reply to comment #8)
> Review of attachment 279205 [details] [review]:
> 
> That breaks the UI. The UUIDs would be shown directly in the UI.

Then the UI needs to resolve them...

(In reply to comment #9)
> (In reply to comment #6)
> > Review of attachment 279207 [details] [review] [details]:
> > 
> > How do you switch between networks without going through the "" (empty)
> > network, and thus bringing down all services?
> 
> You can switch primary without disconnecting the old one - for example by
> plugging a cable while connected to wifi. NM will switch the default route when
> the cable is set up (thus changing PrimaryConnection), but will not disconnect
> wifi automatically.

OK, will review.

> (In reply to comment #8)
> > Review of attachment 279205 [details] [review] [details]:
> > 
> > That breaks the UI. The UUIDs would be shown directly in the UI.
> 
> Then the UI needs to resolve them...

The UI doesn't use NetworkManager, and I'd like to keep it that way.

Review of attachment 279207 [details] [review]:

Looks good otherwise.

::: plugins/sharing/gsd-sharing-manager.c
@@ +147,3 @@
 {
+        char **connections;
+        int j;

"guint i" instead.

@@ +169,3 @@
         if (!service->started ||
             service->process == NULL) {
+                return;

Whitespace changes.

@@ +189,3 @@
                 ServiceInfo *service = l->data;
+                gboolean should_be_started = manager->priv->sharing_status == GSD_SHARING_STATUS_AVAILABLE &&
+                        service_is_enabled_on_current_connection (manager, service);

Make a proper conditional out of this.

(In reply to comment #10)
> (In reply to comment #9)
> > (In reply to comment #8)
> > > Review of attachment 279205 [details] [review] [details] [details]:
> > > 
> > > That breaks the UI. The UUIDs would be shown directly in the UI.
> > 
> > Then the UI needs to resolve them...
> 
> The UI doesn't use NetworkManager, and I'd like to keep it that way.

I don't see how that's possible, given that, once again, connections "ids" are human readable (and editable) names that are not guaranteed to be unique, and are not an appropriate way to identify connections.
Besides, gnome-control-center already uses NM for the network panel, and the whole sharing plugin is useless without NM, so I don't see the big deal in using NM in the sharing panel.

Attachment 279206 [details] pushed as 5ac1250 - sharing: include wwan connection types as broadband
Attachment 279208 [details] pushed as 7b2feef - sharing: don't shutdown rygel if not autostarted

Created attachment 279222 [details] [review]
sharing: Fix deactivation of services when changing network

When switching between networks that both allow sharing in general,
we need to check the enabled connections for each service and
possibly stop it.

Attachment 279205 [details] pushed as bc3be2b - sharing: use UUIDs instead of IDs for NM connection identification
Attachment 279222 [details] pushed as 5ebf6fa - sharing: Fix deactivation of services when changing network

https://bugzilla.gnome.org/show_bug.cgi?id=732218 broke the build without networkmanager support because of service_is_enabled_on_current_connection being defined in an #ifdef block.

I meant https://git.gnome.org/browse/gnome-settings-daemon/commit/?id=5ebf6faf4584eef855b6171a40086839da436760 obviously.