Redesign add-on disconnection

Currently, add-ons are disconnected when ChatProtocol::Shutdown() is
called, which the add-on can do by itself― but there is no standard way
for add-ons to notify the app about their Shutdown. Because of this,
they tend to not call Shutdown()― instead (as in the case of the Jabber
add-on), they display a BAlert (IM_ERROR) notifying the user of the
connection error, but the account is considered active by Cardie (and
its threads are still existant, including its ProtocolLooper).

Zombies are bad, so this is redesigned somewhat with this commit:
Protocols should no longer call ChatProtocol::Shutdown() themselves,
they must send an IM_MESSAGE of IM_PROTOCOL_DISABLE to the app.

This will delete its ProtocolLooper, which in turn will send a
notification to the user and delete the ChatProtocol, and so
calling ChatProtocol::Shutdown().

In the included protocols, an IM_ERROR is sent right before
IM_PROTOCOL_DISABLE is sent if due to a connection error. This is not
required, but it is courteous to inform your user about the "why." :)

application/AppMessages.h

@@ -48,7 +48,7 @@ const uint32 APP_MOVE_UP = 'CYmu';
 const uint32 APP_MOVE_DOWN = 'CYmd';
 
 //! Disable a given account
-const uint32 APP_DISABLE_ACCOUNT = 'CYda';
+const uint32 APP_ACCOUNT_DISABLED = 'CYda';
 
 //! Request a "help" message
 const uint32 APP_REQUEST_HELP = 'CYhm';

application/ChatProtocolMessages.h

@@ -412,12 +412,13 @@ enum im_what_code {
 		Should be sent after connection is established, initialization done */
 	IM_PROTOCOL_READY					= 1002,
 
-	/*! Account has just been disabled	→App
-		Sent as the account disconnects, and will not be automatically
-		reconnected on the add-on's side.
-		Should be sent in two cases: When connection fails due to an error,
-		or when account has been closed by user (ChatProtocol::Shutdown()) */
-	IM_PROTOCOL_DISABLED				= 1003
+	/*! Deletion of protocol requested	→App
+		This requests that the app delete the ChatProtocol and its
+		ProtocolLooper― so invoking ChatProtocol::Shutdown().
+		This should be sent by the protocol after connection errors or a
+		disconnect, when the addon doesn't have anything left to do (other
+		than yearn for the sweet hand of death). */
+	IM_PROTOCOL_DISABLE					= 1003
 };
 
 #endif	// _CHAT_PROTOCOL_MESSAGES_H

application/ProtocolLooper.cpp

@@ -12,9 +12,11 @@
 
 #include "ProtocolLooper.h"
 
+#include <Bitmap.h>
 #include <String.h>
 
 #include "Account.h"
+#include "AppMessages.h"
 #include "Conversation.h"
 #include "ConversationAccountItem.h"
 
@@ -42,6 +44,13 @@ ProtocolLooper::ProtocolLooper(ChatProtocol* protocol, int64 instance)
 
 ProtocolLooper::~ProtocolLooper()
 {
+	BMessage* msg = new BMessage(APP_ACCOUNT_DISABLED);
+	BBitmap* icon = fProtocol->Icon();
+
+	icon->Archive(msg);
+	msg->AddString("name", fProtocol->GetName());
+	fProtocol->MessengerInterface()->SendMessage(msg);
+
 	fProtocol->Shutdown();
 	delete fProtocol;
 }

application/Server.cpp

@@ -106,6 +106,12 @@ Server::Filter(BMessage* message, BHandler **target)
 	filter_result result = B_DISPATCH_MESSAGE;
 
 	switch (message->what) {
+		case IM_MESSAGE:
+			result = ImMessage(message);
+			break;
+		case IM_ERROR:
+			ImError(message);
+			break;
 		case APP_CLOSE_CHAT_WINDOW:
 		{
 			BString id = message->FindString("chat_id");
@@ -116,12 +122,24 @@ Server::Filter(BMessage* message, BHandler **target)
 			result = B_SKIP_MESSAGE;
 			break;
 		}
-		case IM_MESSAGE:
-			result = ImMessage(message);
-			break;
-		case IM_ERROR:
-			ImError(message);
+		case APP_ACCOUNT_DISABLED:
+		{
+			BString name;
+			int64 instance;
+			if (message->FindInt64("instance", &instance) != B_OK)
+				return result;
+
+			// "Whoops" notification
+			if (AppPreferences::Item()->NotifyProtocolStatus == true
+					&& message->FindString("name", &name) == B_OK) {
+				BBitmap* icon = new BBitmap(message);
+				BString content("%user% has been disabled!");
+				content.ReplaceAll("%user%", name);
+
+				_SendNotification("Disabled", content, icon);
+			}
 			break;
+		}
 		case APP_REPLICANT_MESSENGER:
 		{
 			BMessenger* messenger = new BMessenger();
@@ -138,18 +156,6 @@ Server::Filter(BMessage* message, BHandler **target)
 			accountManager->ReplicantStatusNotify(accountManager->Status());
 			break;
 		}
-
-		case APP_DISABLE_ACCOUNT:
-		{
-			int64 instance = 0;
-			if (message->FindInt64("instance", &instance) != B_OK) {
-				result = B_SKIP_MESSAGE;
-				break;
-			}
-			RemoveProtocolLooper(instance);
-			break;
-		}
-
 		case APP_USER_INFO:
 		{
 			User* user = _EnsureUser(message);
@@ -159,7 +165,6 @@ Server::Filter(BMessage* message, BHandler **target)
 			}
 			break;
 		}
-
 		case APP_REQUEST_HELP:
 		{
 			BString body;
@@ -200,12 +205,10 @@ Server::Filter(BMessage* message, BHandler **target)
 			chat->ImMessage(help);
 			break;
 		}
-
 		default:
 			// Dispatch not handled messages to main window
 			break;
 	}
-
 	return result;
 }
 
@@ -610,12 +613,14 @@ Server::ImMessage(BMessage* msg)
 				}
 			break;
 		}
-		case IM_PROTOCOL_DISABLED:
+		case IM_PROTOCOL_DISABLE:
 		{
-			// "Whoops" notification
-			if (AppPreferences::Item()->NotifyProtocolStatus == true)
-				_ProtocolNotification(_LooperFromMessage(msg),
-					BString("Disabled"), BString("%user% has been disabled!"));
+			int64 instance = 0;
+			if (msg->FindInt64("instance", &instance) != B_OK) {
+				result = B_SKIP_MESSAGE;
+				break;
+			}
+			RemoveProtocolLooper(instance);
 			break;
 		}
 		default:
@@ -983,12 +988,20 @@ Server::_ProtocolNotification(ProtocolLooper* looper, BString title,
 	if (looper == NULL || title.IsEmpty() == true)	return;
 	title.ReplaceAll("%user%", looper->Protocol()->GetName());
 	desc.ReplaceAll("%user%", looper->Protocol()->GetName());
+	_SendNotification(title, desc, looper->Protocol()->Icon(), type);
+}
 
+
+void
+Server::_SendNotification(BString title, BString content, BBitmap* icon,
+	notification_type type)
+{
 	BNotification notification(type);
 	notification.SetGroup(BString(APP_NAME));
 	notification.SetTitle(title);
-	if (desc.IsEmpty() == false)
-		notification.SetContent(desc);
-	notification.SetIcon(looper->Protocol()->Icon());
+	if (content.IsEmpty() == false)
+		notification.SetContent(content);
+	if (icon != NULL)
+		notification.SetIcon(icon);
 	notification.Send();
 }

application/Server.h

@@ -80,6 +80,9 @@ private:
 			void			_ProtocolNotification(ProtocolLooper* looper,
 								BString title, BString desc,
 								notification_type type=B_INFORMATION_NOTIFICATION);
+			void			_SendNotification(BString title, BString content,
+								BBitmap* icon=NULL,
+								notification_type type=B_INFORMATION_NOTIFICATION);
 
 			void			_ReplicantStatusNotify(UserStatus status);
 

application/preferences/PreferencesAccounts.cpp

@@ -18,8 +18,8 @@
 
 #include "AccountDialog.h"
 #include "AccountListItem.h"
-#include "AppMessages.h"
 #include "ChatProtocol.h"
+#include "ChatProtocolMessages.h"
 #include "PreferencesAccounts.h"
 #include "ProtocolManager.h"
 #include "ProtocolSettings.h"
@@ -197,7 +197,8 @@ PreferencesAccounts::MessageReceived(BMessage* msg)
 			if (found == false)
 				return;
 
-			BMessage* remove = new BMessage(APP_DISABLE_ACCOUNT);
+			BMessage* remove = new BMessage(IM_MESSAGE);
+			remove->AddInt32("im_what", IM_PROTOCOL_DISABLE);
 			remove->AddInt64("instance", instance);
 			((TheApp*)be_app)->GetMainWindow()->PostMessage(remove);
 

application/windows/MainWindow.cpp

@@ -205,7 +205,7 @@ MainWindow::MessageReceived(BMessage* message)
 			}
 			break;
 		}
-		case APP_DISABLE_ACCOUNT:
+		case APP_ACCOUNT_DISABLED:
 			_ToggleMenuItems();
 			break;
 		case IM_MESSAGE:

protocols/irc/IrcProtocol.cpp

@@ -58,14 +58,21 @@ connect_thread(void* data)
 	if (irc_connect(session, joinServer.String(), port, password, nick, ident,
 		real_name))
 	{
-		printf("Could not connect: %s\n", irc_strerror (irc_errno(session)));
+		BMessage error(IM_ERROR);
+		error.AddString("error", "Could not connect");
+		error.AddString("details", irc_strerror(irc_errno(session)));
+		_SendMessage(&error);
+		_SendMessage(new BMessage(IM_PROTOCOL_DISABLE));
 		return B_ERROR;
 	}
 
 	// Start network loop
 	if (irc_run(session)) {
-		printf("Could not connect or I/O error: %s\n", irc_strerror
-			(irc_errno(session)));
+		BMessage error(IM_ERROR);
+		error.AddString("error", "Connection or I/O error");
+		error.AddString("details", irc_strerror(irc_errno(session)));
+		_SendMessage(&error);
+		_SendMessage(new BMessage(IM_PROTOCOL_DISABLE));
 		return B_ERROR;
 	}
 	return B_OK;

protocols/purple/PurpleApp.cpp

@@ -115,7 +115,7 @@ PurpleApp::MessageReceived(BMessage* msg)
 			SendMessage(thread, _GetCommands(_AccountFromMessage(msg)));
 			break;
 		}
-		case PURPLE_REQUEST_DISCONNECT:
+		case PURPLE_DISCONNECT_ACCOUNT:
 		{
 			PurpleAccount* account = _AccountFromMessage(msg);
 			if (account == NULL)
@@ -1034,8 +1034,7 @@ signal_account_signed_off(PurpleAccount* account)
 static void
 signal_account_disabled(PurpleAccount* account)
 {
-	BMessage disabled(IM_MESSAGE);
-	disabled.AddInt32("im_what", IM_PROTOCOL_DISABLED);
+	BMessage disabled(PURPLE_SHUTDOWN_ADDON);
 	((PurpleApp*)be_app)->SendMessage(account, disabled);
 }
 

protocols/purple/PurpleMessages.h

@@ -56,7 +56,13 @@ enum purple_message {
 
 	/*! Disconnect add-on's account			→Server
 		Requires:	String account_name */
-	PURPLE_REQUEST_DISCONNECT				= 'Axwx',
+	PURPLE_DISCONNECT_ACCOUNT				= 'Axwx',
+
+	/*! Shutdown an add-on instance			→Protocol
+		Send from server after an account is disabled
+		or errors out; sent so an add-on instance
+		won't run for an unconnected account. */
+	PURPLE_SHUTDOWN_ADDON					= 'Pxwx',
 
 	/*! Register chat commands with proto	→Protocol */
 	PURPLE_REGISTER_COMMANDS				= 'Scmd',

protocols/purple/PurpleProtocol.cpp

@@ -19,8 +19,6 @@
 
 #include "PurpleProtocol.h"
 
-#include <iostream>
-
 #include <Application.h>
 #include <Resources.h>
 #include <Roster.h>
@@ -124,6 +122,13 @@ connect_thread(void* data)
 	while (true) {
 		BMessage* msg = new BMessage(receive_message());
 		switch (msg->what) {
+			case PURPLE_SHUTDOWN_ADDON: {
+				protocol->Shutdown();
+				BMessage* disabled = new BMessage(IM_MESSAGE);
+				disabled->AddInt32("im_what", IM_PROTOCOL_DISABLE);
+				protocol->SendMessage(disabled);
+				break;
+			}
 			case PURPLE_REGISTER_COMMANDS:
 				protocol->Process(msg);
 				break;
@@ -186,7 +191,7 @@ PurpleProtocol::Init(ChatProtocolMessengerInterface* interface)
 status_t
 PurpleProtocol::Shutdown()
 {
-	BMessage* disconnect = new BMessage(PURPLE_REQUEST_DISCONNECT);
+	BMessage* disconnect = new BMessage(PURPLE_DISCONNECT_ACCOUNT);
 	_SendPrplMessage(disconnect);
 
 	kill_thread(fBirdThread);

protocols/xmpp/JabberHandler.cpp

@@ -693,9 +693,12 @@ JabberHandler::HandleConnectionError(gloox::ConnectionError& e)
 			}
 			break;
 		}
-		case gloox::ConnNotConnected:
-			errMsg.AddString("error", "There is no active connection.");
+		case gloox::ConnNotConnected: {
+			BMessage disconnect(IM_MESSAGE);
+			disconnect.AddInt32("im_what", IM_PROTOCOL_DISABLE);
+			_SendMessage(&disconnect);
 			break;
+		}
 		default:
 			break;
 	}