device: eliminate direct calls to check_connection_available() in favor of nm_device_check_connection_available()

It was confusing to understand the difference between calling nm_device_connection_is_available()
and check_connection_available(), they behaved similar, but not really
the same. Especially nm_device_connection_is_available() would look
first into @available_connetions, and might call check_connection_available()
itself. Whereas @available_connetions was also populated by testing
check_connection_available(). This interrelation makes it hard to
understand when nm_device_connection_is_available() returned true.

Rename nm_device_connection_is_available() to nm_device_check_connection_available()
and remove all direct calls of check_connection_available() in favor of
the wrapper nm_device_check_connection_available().

Now we only call nm_device_check_connection_available() with different
parameters (@flags and @specific_object). We also have the additional
guarantee that specifying more @flags will widen the result and making
a connection "more" available, while specifying a @specific_object will
restrict it.

This also changes behavior in several cases. For example before
nm_device_connection_is_available() for user-requests would always
declare matching connections available on Wi-Fi devices (only)
regardless of the device state. Now the device state gets consistently
considered.

For default-unmanaged devices it also changes behavior in complicated
ways, because before we would put connections into @available_connetions
for every device-state, but nm_device_connection_is_available() had a
special over-ride only for unmanaged-state.

This also fixes a bug, that user can activate an unavailable Wi-Fi
device:
  nmcli radio wifi off
  nmcli connection up wlan0

4 files changed, 37 insertions, 67 deletions

diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c
index c4ec949aee..62a288a4c3 100644
--- a/src/devices/nm-device.c
+++ b/src/devices/nm-device.c
@@ -1898,7 +1898,7 @@ can_auto_connect (NMDevice *self,
 	if (!nm_setting_connection_get_autoconnect (s_con))
 		return FALSE;
 
-	return nm_device_connection_is_available (self, connection, FALSE);
+	return nm_device_check_connection_available (self, connection, NM_DEVICE_CHECK_CON_AVAILABLE_NONE, NULL);
 }
 
 /**
@@ -6896,64 +6896,42 @@ nm_device_set_dhcp_anycast_address (NMDevice *self, const char *addr)
 }
 
 /**
- * nm_device_connection_is_available():
+ * nm_device_check_connection_available():
  * @self: the #NMDevice
  * @connection: the #NMConnection to check for availability
- * @for_user_activation_request: set to %TRUE if we are checking whether
- *   the connection is available on an explicit user request. This
- *   also checks for device specific overrides.
+ * @flags: flags to affect the decision making of whether a connection
+ *   is available. Adding a flag can only make a connection more available,
+ *   not less.
+ * @specific_object: a device type dependent argument to further
+ *   filter the result. Passing a non %NULL specific object can only reduce
+ *   the availability of a connection.
  *
- * Check if @connection is available to be activated on @self.  Normally this
- * only checks if the connection is in @self's AvailableConnections property.
- * If @for_user_activation_request is %TRUE then the device is asked to do specific
- * checks that may bypass the AvailableConnections property.
+ * Check if @connection is available to be activated on @self.
  *
  * Returns: %TRUE if @connection can be activated on @self
  */
 gboolean
-nm_device_connection_is_available (NMDevice *self,
-                                   NMConnection *connection,
-                                   gboolean for_user_activation_request)
+nm_device_check_connection_available (NMDevice *self,
+                                      NMConnection *connection,
+                                      NMDeviceCheckConAvailableFlags flags,
+                                      const char *specific_object)
 {
-	NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self);
-	gboolean is_default_unmanaged;
-
-	if (g_hash_table_contains (priv->available_connections, connection))
-		return TRUE;
-
-	is_default_unmanaged = priv->state == NM_DEVICE_STATE_UNMANAGED && nm_device_get_default_unmanaged (self);
+	NMDeviceState state;
 
-	if (!for_user_activation_request && !is_default_unmanaged) {
-		/* Shortcut: there are only additional checks for either @for_user_activation_request
-		 * or @is_default_unmanaged. Return FALSE right away. */
+	state = nm_device_get_state (self);
+	if (state < NM_DEVICE_STATE_UNMANAGED)
 		return FALSE;
-	}
-
-	if (!nm_device_check_connection_compatible (self, connection)) {
-		/* An incompatilbe connection is never available. */
+	if (   state < NM_DEVICE_STATE_UNAVAILABLE
+	    && nm_device_get_unmanaged_flag (self, NM_UNMANAGED_ALL & ~NM_UNMANAGED_DEFAULT))
+		return FALSE;
+	if (   state < NM_DEVICE_STATE_DISCONNECTED
+	    && !nm_device_is_available (self, NM_DEVICE_CHECK_DEV_AVAILABLE_NONE))
 		return FALSE;
-	}
-
-	if (   is_default_unmanaged
-	    && NM_DEVICE_GET_CLASS (self)->check_connection_available (self, connection, NM_DEVICE_CHECK_CON_AVAILABLE_NONE, NULL)) {
-		/* default-unmanaged  devices in UNMANAGED state have no available connections
-		 * so we must manually check whether the connection is available here. */
-		return TRUE;
-	}
 
-	if (   for_user_activation_request
-	    && NM_DEVICE_GET_CLASS (self)->check_connection_available_has_user_override
-	    && NM_DEVICE_GET_CLASS (self)->check_connection_available (self, connection, NM_DEVICE_CHECK_CON_AVAILABLE_FOR_USER_REQUEST, NULL)) {
-		/* Connections for an explicit user activation request might only be available after
-		 * additional checking.
-		 *
-		 * For example in case of hidden Wi-Fi, the connection might not have the 'hidden' property
-		 * set. Support this by allowing device specific overrides.
-		 */
-		return TRUE;
-	}
+	if (!nm_device_check_connection_compatible (self, connection))
+		return FALSE;
 
-	return FALSE;
+	return NM_DEVICE_GET_CLASS (self)->check_connection_available (self, connection, flags, specific_object);
 }
 
 static void
@@ -6973,16 +6951,10 @@ _clear_available_connections (NMDevice *self, gboolean do_signal)
 static gboolean
 _try_add_available_connection (NMDevice *self, NMConnection *connection)
 {
-	if (   nm_device_get_state (self) < NM_DEVICE_STATE_DISCONNECTED
-	    && !nm_device_get_default_unmanaged (self))
-		return FALSE;
-
-	if (nm_device_check_connection_compatible (self, connection)) {
-		if (NM_DEVICE_GET_CLASS (self)->check_connection_available (self, connection, NM_DEVICE_CHECK_CON_AVAILABLE_NONE, NULL)) {
-			g_hash_table_add (NM_DEVICE_GET_PRIVATE (self)->available_connections,
-			                  g_object_ref (connection));
-			return TRUE;
-		}
+	if (nm_device_check_connection_available (self, connection, NM_DEVICE_CHECK_CON_AVAILABLE_NONE, NULL)) {
+		g_hash_table_add (NM_DEVICE_GET_PRIVATE (self)->available_connections,
+		                  g_object_ref (connection));
+		return TRUE;
 	}
 	return FALSE;
 }
@@ -7057,8 +7029,8 @@ nm_device_get_available_connections (NMDevice *self, const char *specific_object
 			/* If a specific object is given, only include connections that are
 			 * compatible with it.
 			 */
-			if (   !specific_object
-			    || NM_DEVICE_GET_CLASS (self)->check_connection_available (self, connection, NM_DEVICE_CHECK_CON_AVAILABLE_NONE, specific_object))
+			if (   !specific_object /* << Optimization: we know that the connection is available without @specific_object.  */
+			    || nm_device_check_connection_available (self, connection, NM_DEVICE_CHECK_CON_AVAILABLE_NONE, specific_object))
 				g_ptr_array_add (array, connection);
 		}
 	}
diff --git a/src/devices/nm-device.h b/src/devices/nm-device.h
index a863571f29..25586a5fa9 100644
--- a/src/devices/nm-device.h
+++ b/src/devices/nm-device.h
@@ -173,8 +173,6 @@ typedef struct {
 	                                            NMDeviceCheckConAvailableFlags flags,
 	                                            const char *specific_object);
 
-	gboolean    check_connection_available_has_user_override;
-
 	gboolean    (* complete_connection)         (NMDevice *self,
 	                                             NMConnection *connection,
 	                                             const char *specific_object,
@@ -397,9 +395,10 @@ gboolean nm_device_has_pending_action    (NMDevice *device);
 GPtrArray *nm_device_get_available_connections (NMDevice *device,
                                                 const char *specific_object);
 
-gboolean   nm_device_connection_is_available (NMDevice *device,
-                                              NMConnection *connection,
-                                              gboolean for_user_activation_request);
+gboolean   nm_device_check_connection_available (NMDevice *device,
+                                                 NMConnection *connection,
+                                                 NMDeviceCheckConAvailableFlags flags,
+                                                 const char *specific_object);
 
 gboolean nm_device_notify_component_added (NMDevice *device, GObject *component);
 
diff --git a/src/devices/wifi/nm-device-wifi.c b/src/devices/wifi/nm-device-wifi.c
index 0983dcd31a..15d28d56d5 100644
--- a/src/devices/wifi/nm-device-wifi.c
+++ b/src/devices/wifi/nm-device-wifi.c
@@ -3327,7 +3327,6 @@ nm_device_wifi_class_init (NMDeviceWifiClass *klass)
 	parent_class->is_available = is_available;
 	parent_class->check_connection_compatible = check_connection_compatible;
 	parent_class->check_connection_available = check_connection_available;
-	parent_class->check_connection_available_has_user_override = TRUE;
 	parent_class->complete_connection = complete_connection;
 	parent_class->set_enabled = set_enabled;
 
diff --git a/src/nm-manager.c b/src/nm-manager.c
index 47e98a6206..a42f89e5d3 100644
--- a/src/nm-manager.c
+++ b/src/nm-manager.c
@@ -2538,7 +2538,7 @@ ensure_master_active_connection (NMManager *self,
 				if (!is_compatible_with_slave (candidate, connection))
 					continue;
 
-				if (nm_device_connection_is_available (master_device, candidate, TRUE)) {
+				if (nm_device_check_connection_available (master_device, candidate, NM_DEVICE_CHECK_CON_AVAILABLE_FOR_USER_REQUEST, NULL)) {
 					master_ac = nm_manager_activate_connection (self,
 					                                            candidate,
 					                                            NULL,
@@ -2579,7 +2579,7 @@ ensure_master_active_connection (NMManager *self,
 				continue;
 			}
 
-			if (!nm_device_connection_is_available (candidate, master_connection, TRUE))
+			if (!nm_device_check_connection_available (candidate, master_connection, NM_DEVICE_CHECK_CON_AVAILABLE_FOR_USER_REQUEST, NULL))
 				continue;
 
 			found_device = TRUE;
@@ -2729,7 +2729,7 @@ _internal_activate_device (NMManager *self, NMActiveConnection *active, GError *
 	}
 
 	/* Final connection must be available on device */
-	if (!nm_device_connection_is_available (device, connection, TRUE)) {
+	if (!nm_device_check_connection_available (device, connection, NM_DEVICE_CHECK_CON_AVAILABLE_FOR_USER_REQUEST, NULL)) {
 		g_set_error (error, NM_MANAGER_ERROR, NM_MANAGER_ERROR_UNKNOWN_CONNECTION,
 		             "Connection '%s' is not available on the device %s at this time.",
 		             nm_connection_get_id (connection), nm_device_get_iface (device));