Revamp connection_manager.vala

The original implementation was unable to reconnect accounts after
several suspend cycles.

1 files changed, 56 insertions, 127 deletions

diff --git a/libdino/src/service/connection_manager.vala b/libdino/src/service/connection_manager.vala
index 43580c06..35ee9160 100644
--- a/libdino/src/service/connection_manager.vala
+++ b/libdino/src/service/connection_manager.vala
@@ -20,12 +20,7 @@ public class ConnectionManager : Object {
 
     private HashMap<Account, Connection> connections = new HashMap<Account, Connection>(Account.hash_func, Account.equals_func);
     private HashMap<Account, ConnectionError> connection_errors = new HashMap<Account, ConnectionError>(Account.hash_func, Account.equals_func);
-
-    private HashMap<Account, bool> connection_ongoing = new HashMap<Account, bool>(Account.hash_func, Account.equals_func);
-    private HashMap<Account, bool> connection_directly_retry = new HashMap<Account, bool>(Account.hash_func, Account.equals_func);
-
     private NetworkMonitor? network_monitor;
-    private Login1Manager? login1;
     private ModuleManager module_manager;
     public string? log_options;
 
@@ -58,21 +53,20 @@ public class ConnectionManager : Object {
         public string uuid { get; set; }
         public XmppStream? stream { get; set; }
         public ConnectionState connection_state { get; set; default = ConnectionState.DISCONNECTED; }
-        public DateTime? established { get; set; }
-        public DateTime? last_activity { get; set; }
+        public bool acked;
 
         public Connection() {
             reset();
         }
 
         public void reset() {
+            acked = false;
             if (stream != null) {
                 stream.detach_modules();
 
                 stream.disconnect.begin();
             }
             stream = null;
-            established = last_activity = null;
             uuid = Xmpp.random_uuid();
         }
 
@@ -97,30 +91,23 @@ public class ConnectionManager : Object {
         }
     }
 
+    private async void on_network_changed(bool state) {
+        debug(@"on network changed=$(state)");
+
+        if (state) {
+            check_reconnects();
+        }
+        else {
+            make_offline_all();
+        }
+    }
+
     public ConnectionManager(ModuleManager module_manager) {
         this.module_manager = module_manager;
         network_monitor = GLib.NetworkMonitor.get_default();
         if (network_monitor != null) {
             network_monitor.network_changed.connect(on_network_changed);
-            network_monitor.notify["connectivity"].connect(on_network_changed);
         }
-
-        get_login1.begin((_, res) => {
-            login1 = get_login1.end(res);
-            if (login1 != null) {
-                login1.PrepareForSleep.connect(on_prepare_for_sleep);
-            }
-        });
-
-        Timeout.add_seconds(60, () => {
-            foreach (Account account in connections.keys) {
-                if (connections[account].last_activity != null &&
-                        connections[account].last_activity.compare(new DateTime.now_utc().add_minutes(-1)) < 0) {
-                    check_reconnect(account);
-                }
-            }
-            return true;
-        });
     }
 
     public XmppStream? get_stream(Account account) {
@@ -151,8 +138,6 @@ public class ConnectionManager : Object {
     public void connect_account(Account account) {
         if (!connections.has_key(account)) {
             connections[account] = new Connection();
-            connection_ongoing[account] = false;
-            connection_directly_retry[account] = false;
 
             connect_stream.begin(account);
         } else {
@@ -189,25 +174,13 @@ public class ConnectionManager : Object {
 
         XmppStreamResult stream_result;
 
-        if (connection_ongoing[account]) {
-            debug("[%s] Connection attempt already in progress. Directly retry if it fails.", account.bare_jid.to_string());
-            connection_directly_retry[account] = true;
-            return;
-        } else if (connections[account].stream != null) {
-            debug("[%s] Cancelling connecting because there is already a stream", account.bare_jid.to_string());
-            return;
-        } else {
-            connection_ongoing[account] = true;
-            connection_directly_retry[account] = false;
-
-            change_connection_state(account, ConnectionState.CONNECTING);
-            stream_result = yield Xmpp.establish_stream(account.bare_jid, module_manager.get_modules(account, resource), log_options,
-                    (peer_cert, errors) => { return on_invalid_certificate(account.domainpart, peer_cert, errors); }
-            );
-            connections[account].stream = stream_result.stream;
-
-            connection_ongoing[account] = false;
-        }
+        change_connection_state(account, ConnectionState.CONNECTING);
+        stream_result = yield Xmpp.establish_stream(account.bare_jid, module_manager.get_modules(account, resource), log_options,
+                (peer_cert, errors) => {
+                    change_connection_state(account, ConnectionState.DISCONNECTED);
+                    return on_invalid_certificate(account.domainpart, peer_cert, errors); }
+        );
+        connections[account].stream = stream_result.stream;
 
         if (stream_result.stream == null) {
             if (stream_result.tls_errors != null) {
@@ -219,8 +192,6 @@ public class ConnectionManager : Object {
 
             change_connection_state(account, ConnectionState.DISCONNECTED);
 
-            check_reconnect(account, connection_directly_retry[account]);
-
             return;
         }
 
@@ -228,7 +199,6 @@ public class ConnectionManager : Object {
 
         debug("[%s] New connection with resource %s: %p", account.bare_jid.to_string(), resource, stream);
 
-        connections[account].established = new DateTime.now_utc();
         stream.attached_modules.connect((stream) => {
             stream_attached_modules(account, stream);
             change_connection_state(account, ConnectionState.CONNECTED);
@@ -242,7 +212,6 @@ public class ConnectionManager : Object {
         string connection_uuid = connections[account].uuid;
         stream.received_node.connect(() => {
             if (connections[account].uuid == connection_uuid) {
-                connections[account].last_activity = new DateTime.now_utc();
             } else {
                 warning("Got node for outdated connection");
             }
@@ -286,87 +255,47 @@ public class ConnectionManager : Object {
     private void check_reconnect(Account account, bool directly_reconnect = false) {
         if (!connections.has_key(account)) return;
 
-        bool acked = false;
-        DateTime? last_activity_was = connections[account].last_activity;
-
-        if (connections[account].stream == null) {
-            Timeout.add_seconds(10, () => {
-                if (!connections.has_key(account)) return false;
-                if (connections[account].stream != null) return false;
-                if (connections[account].last_activity != last_activity_was) return false;
-
-                connect_stream.begin(account);
-                return false;
-            });
-            return;
-        }
-
-        XmppStream stream = connections[account].stream;
+        XmppStream? stream = connections[account].stream;
 
-        stream.get_module(Xep.Ping.Module.IDENTITY).send_ping.begin(stream, account.bare_jid.domain_jid, () => {
-            acked = true;
-            if (connections[account].stream != stream) return;
-            change_connection_state(account, ConnectionState.CONNECTED);
-        });
-
-        Timeout.add_seconds(10, () => {
-            if (!connections.has_key(account)) return false;
-            if (connections[account].stream != stream) return false;
-            if (acked) return false;
-            if (connections[account].last_activity != last_activity_was) return false;
-
-            // Reconnect. Nothing gets through the stream.
-            debug("[%s %p] Ping timeouted. Reconnecting", account.bare_jid.to_string(), stream);
-            change_connection_state(account, ConnectionState.DISCONNECTED);
-
-            connections[account].reset();
-            connect_stream.begin(account);
-            return false;
-        });
-    }
+        var cancellable = new Cancellable();
+        debug(@"account.domainpart=$(account.domainpart)");
 
-    private bool network_is_online() {
-        /* FIXME: We should also check for connectivity eventually. For more
-         * details on why we don't do it for now, see:
-         *
-         * - https://github.com/dino/dino/pull/236#pullrequestreview-86851793
-         * - https://bugzilla.gnome.org/show_bug.cgi?id=792240
-         */
-        return network_monitor != null && network_monitor.network_available;
-    }
-
-    private void on_network_changed() {
-        if (network_is_online()) {
-            debug("NetworkMonitor: Network reported online");
-            check_reconnects();
-        } else {
-            debug("NetworkMonitor: Network reported offline");
-            foreach (Account account in connections.keys) {
-                change_connection_state(account, ConnectionState.DISCONNECTED);
+        try {
+            var address = new GLib.NetworkAddress(account.domainpart, 5222);
+            var reachable = network_monitor.can_reach(address, cancellable);
+
+            debug(@"can-reach: $(reachable)");
+
+            if (reachable) {
+                if (connections[account].connection_state == ConnectionState.CONNECTING) return;
+                connections[account].acked = false;
+                change_connection_state(account, ConnectionState.CONNECTING);
+
+                stream.get_module(Xep.Ping.Module.IDENTITY).send_ping.begin(stream, account.bare_jid.domain_jid, () => {
+                    connections[account].acked = true;
+                    if (connections[account].stream != stream) return;
+                    change_connection_state(account, ConnectionState.CONNECTED);
+                });
+
+                Timeout.add_seconds(10, () => {
+                    if (!connections.has_key(account)) return false;
+                    if (connections[account].stream != stream) return false;
+                    if (connections[account].acked) return false;
+
+                    // Reconnect. Nothing gets through the stream.
+                    debug("[%s %p] Ping timeouted. Reconnecting", account.bare_jid.to_string(), stream);
+                    change_connection_state(account, ConnectionState.DISCONNECTED);
+
+                    connections[account].reset();
+                    connect_stream.begin(account);
+                    return false;
+                });
             }
-        }
-    }
 
-    private async void on_prepare_for_sleep(bool suspend) {
-        foreach (Account account in connections.keys) {
-            change_connection_state(account, ConnectionState.DISCONNECTED);
-        }
-        if (suspend) {
-            debug("Login1: Device suspended");
-            foreach (Account account in connections.keys) {
-                try {
-                    make_offline(account);
-                    if (connections[account].stream != null) {
-                        yield connections[account].stream.disconnect();
-                    }
-                } catch (Error e) {
-                    debug("Error disconnecting stream %p: %s", connections[account].stream, e.message);
-                }
-            }
-        } else {
-            debug("Login1: Device un-suspend");
-            check_reconnects();
+        } catch (Error e) {
+            print ("Error: %s\n", e.message);
         }
+
     }
 
     private void change_connection_state(Account account, ConnectionState state) {