[ekiga] Opal: Reimplement Accounts and Bank using the Opal EndPoints.

[Damien Sandras <dsandras src gnome org>]

commit 8f29583551c93254d4ac2e5d5549ba7449aa7537
Author: Damien Sandras <dsandras seconix com>
Date:   Sun Jan 25 15:30:12 2015 +0100

    Opal: Reimplement Accounts and Bank using the Opal EndPoints.
    
    We pass them as arguments instead of using the Opal::CallManager to
    prevent useless references to the CallManager. We are using
    shared_ptr's, they control the destruction order of objects, which is
    not what Opal was designed for.  We can safely rely on the H.323 & SIP
    EndPoints as long as: 1) The Opal::Bank and its Opal::Accounts are
    destroyed before the Opal EndPoints. 2) They are destroyed before the
    Opal::CallManager.
    
    However, the SIP & H.323 EndPoints have a null_deleter. That means that
    Opal is taking care of deleting them. They are not deleted when the
    last object having a reference to them is deleted.

 lib/engine/components/opal/opal-account.cpp      |   19 ++++----
 lib/engine/components/opal/opal-account.h        |   28 +++++++++--
 lib/engine/components/opal/opal-bank.cpp         |   54 ++++++++++++---------
 lib/engine/components/opal/opal-bank.h           |   39 ++++++++++++++-
 lib/engine/components/opal/opal-call-manager.cpp |    4 +-
 lib/engine/components/opal/opal-process.cpp      |   15 ++++--
 lib/engine/protocol/call-core.cpp                |    3 +-
 7 files changed, 116 insertions(+), 46 deletions(-)
---
diff --git a/lib/engine/components/opal/opal-account.cpp b/lib/engine/components/opal/opal-account.cpp
index a543d9a..a392dd9 100644
--- a/lib/engine/components/opal/opal-account.cpp
+++ b/lib/engine/components/opal/opal-account.cpp
@@ -48,6 +48,8 @@
 #include <opal/opal.h>
 #include <sip/sippres.h>
 
+#include "config.h"
+
 #include "opal-account.h"
 
 #include "robust-xml.h"
@@ -147,7 +149,10 @@ Opal::Account::Account (Opal::Bank & _bank,
                        boost::shared_ptr<Ekiga::NotificationCore> _notification_core,
                        boost::shared_ptr<Ekiga::PersonalDetails> _personal_details,
                        boost::shared_ptr<Ekiga::AudioOutputCore> _audiooutput_core,
-                       boost::shared_ptr<Opal::CallManager> _call_manager,
+#ifdef HAVE_H323
+                        Opal::H323::EndPoint* _h323_endpoint,
+#endif
+                        Opal::Sip::EndPoint* _sip_endpoint,
                        boost::function0<std::list<std::string> > _existing_groups,
                        xmlNodePtr _node):
   existing_groups(_existing_groups),
@@ -157,7 +162,10 @@ Opal::Account::Account (Opal::Bank & _bank,
   notification_core(_notification_core),
   personal_details(_personal_details),
   audiooutput_core(_audiooutput_core),
-  call_manager(_call_manager)
+#ifdef HAVE_H323
+  h323_endpoint(_h323_endpoint),
+#endif
+  sip_endpoint(_sip_endpoint)
 {
   state = Unregistered;
   status = _("Unregistered");
@@ -236,13 +244,6 @@ Opal::Account::Account (Opal::Bank & _bank,
 
     bank.disable_action ("add-account-ekiga");
   }
-
-  /* We will directly use the OPAL SIP and H.323 EndPoints */
-  boost::shared_ptr<Opal::CallManager> cmanager = call_manager.lock ();
-  if (cmanager) {
-    sip_endpoint = (Opal::Sip::EndPoint*) cmanager->FindEndPoint ("sip");
-    h323_endpoint = (Opal::H323::EndPoint*) cmanager->FindEndPoint ("h323");
-  }
 }
 
 
diff --git a/lib/engine/components/opal/opal-account.h b/lib/engine/components/opal/opal-account.h
index ad03fe5..f636667 100644
--- a/lib/engine/components/opal/opal-account.h
+++ b/lib/engine/components/opal/opal-account.h
@@ -87,12 +87,32 @@ public:
                                  bool enabled,
                                  unsigned timeout);
 
+    /* The Opal::Account is implemented using the H.323 and SIP
+     * EndPoints from OPAL.
+     *
+     * We pass them as arguments instead of using the Opal::CallManager
+     * to prevent useless references to the CallManager. We are using
+     * shared_ptr's, they control the destruction order of objects, which
+     * is not what Opal was designed for.
+     *
+     * We can safely rely on the H.323 & SIP EndPoints as long as:
+     *   1) The Opal::Bank and its Opal::Accounts are destroyed
+     *      before the Opal EndPoints.
+     *   2) They are destroyed before the Opal::CallManager.
+     *
+     * However, the SIP & H.323 EndPoints have a null_deleter. That means
+     * that Opal is taking care of deleting them. They are not deleted when
+     * the last object having a reference to them is deleted.
+     */
     Account (Opal::Bank & bank,
             boost::weak_ptr<Ekiga::PresenceCore> _presence_core,
             boost::shared_ptr<Ekiga::NotificationCore> _notification_core,
             boost::shared_ptr<Ekiga::PersonalDetails> _personal_details,
             boost::shared_ptr<Ekiga::AudioOutputCore> _audiooutput_core,
-            boost::shared_ptr<Opal::CallManager> _call_manager,
+#ifdef HAVE_H323
+             Opal::H323::EndPoint* _h323_endpoint,
+#endif
+             Opal::Sip::EndPoint* _sip_endpoint,
             boost::function0<std::list<std::string> > _existing_groups,
             xmlNodePtr node_);
 
@@ -241,15 +261,13 @@ private:
 
     Opal::Bank & bank;
 
-    Opal::Sip::EndPoint* sip_endpoint;
-    Opal::H323::EndPoint* h323_endpoint;
-
     boost::weak_ptr<Ekiga::PresenceCore> presence_core;
     boost::weak_ptr<Ekiga::NotificationCore> notification_core;
     boost::weak_ptr<Ekiga::PersonalDetails> personal_details;
     boost::weak_ptr<Ekiga::AudioOutputCore> audiooutput_core;
 
-    boost::weak_ptr<Opal::CallManager> call_manager;
+    Opal::H323::EndPoint* h323_endpoint;
+    Opal::Sip::EndPoint* sip_endpoint;
   };
 
   typedef boost::shared_ptr<Account> AccountPtr;
diff --git a/lib/engine/components/opal/opal-bank.cpp b/lib/engine/components/opal/opal-bank.cpp
index e04a8ad..a7cbe98 100644
--- a/lib/engine/components/opal/opal-bank.cpp
+++ b/lib/engine/components/opal/opal-bank.cpp
@@ -47,22 +47,23 @@
 
 #include "opal-bank.h"
 #include "opal-presentity.h"
-#include "sip-endpoint.h"
-#include "h323-endpoint.h"
 
 
 Opal::Bank::Bank (Ekiga::ServiceCore& core,
-                  boost::shared_ptr<Opal::CallManager> _call_manager):
-  call_manager(_call_manager),
+#ifdef HAVE_H323
+                  Opal::H323::EndPoint* _h323_endpoint,
+#endif
+                  Opal::Sip::EndPoint* _sip_endpoint):
   presence_core(core.get<Ekiga::PresenceCore> ("presence-core")),
+  call_core(core.get<Ekiga::CallCore> ("call-core")),
   notification_core(core.get<Ekiga::NotificationCore> ("notification-core")),
   personal_details(core.get<Ekiga::PersonalDetails> ("personal-details")),
-  audiooutput_core(core.get<Ekiga::AudioOutputCore> ("audiooutput-core"))
+  audiooutput_core(core.get<Ekiga::AudioOutputCore> ("audiooutput-core")),
+#ifdef HAVE_H323
+  h323_endpoint(_h323_endpoint),
+#endif
+  sip_endpoint(_sip_endpoint)
 {
-  boost::shared_ptr<Opal::CallManager> cmanager = call_manager.lock ();
-  if (!cmanager)
-    return;
-
   is_ready = false;
 
   std::list<std::string> accounts;
@@ -96,7 +97,10 @@ Opal::Bank::Bank (Ekiga::ServiceCore& core,
                                                       notification_core,
                                                       personal_details,
                                                       audiooutput_core,
-                                                      cmanager,
+#ifdef HAVE_H323
+                                                      _h323_endpoint,
+#endif
+                                                      _sip_endpoint,
                                                       boost::bind(&Opal::Bank::existing_groups, this),
                                                       child));
 
@@ -120,21 +124,19 @@ Opal::Bank::Bank (Ekiga::ServiceCore& core,
     }
   }
 
-  std::cout << "FIXME" << std::endl << std::flush;
-  //sip_endpoint.mwi_event.connect (boost::bind(&Opal::Bank::on_mwi_event, this, _1, _2));
+  // FIXME
+  sip_endpoint->mwi_event.connect (boost::bind(&Opal::Bank::on_mwi_event, this, _1, _2));
 
   // Enable accounts when the manager is ready
-  cmanager->ready.connect (boost::bind (&Opal::Bank::set_ready, this));
+  boost::shared_ptr<Ekiga::CallCore> ccore = call_core.lock ();
+  if (ccore)
+    ccore->ready.connect (boost::bind (&Opal::Bank::set_ready, this));
 }
 
 
 Opal::Bank::~Bank ()
 {
-  // do it forcibly so we're sure the accounts are freed before our
-  // reference to the call manager. Indeed they try to unregister from
-  // presence when killed, and that gives a crash if the call manager
-  // is already gone!
-  Ekiga::RefLister<Opal::Account>::remove_all_objects ();
+  std::cout << "BANK DESTROY" << std::endl << std::flush;
 
   delete protocols_settings;
 }
@@ -258,10 +260,6 @@ Opal::Bank::add (Account::Type acc_type,
                 bool enabled,
                 unsigned timeout)
 {
-  boost::shared_ptr<Opal::CallManager> cmanager = call_manager.lock ();
-  if (!cmanager)
-    return;
-
   xmlNodePtr child = Opal::Account::build_node (acc_type, name, host, user, auth_user, password, enabled, 
timeout);
 
   xmlAddChild (node, child);
@@ -275,7 +273,10 @@ Opal::Bank::add (Account::Type acc_type,
                                    notification_core,
                                    personal_details,
                                    audiooutput_core,
-                                    cmanager,
+#ifdef HAVE_H323
+                                    h323_endpoint,
+#endif
+                                    sip_endpoint,
                                    boost::bind(&Opal::Bank::existing_groups, this),
                                    child));
   Ekiga::BankImpl<Account>::add_connection (account, account->presentity_added.connect (boost::bind 
(boost::ref(presentity_added), account, _1)));
@@ -332,6 +333,13 @@ Opal::Bank::find_account (const std::string& _aor)
 
 
 void
+Opal::Bank::clear ()
+{
+  Ekiga::RefLister<Opal::Account>::remove_all_objects ();
+}
+
+
+void
 Opal::Bank::save () const
 {
   xmlChar *buffer = NULL;
diff --git a/lib/engine/components/opal/opal-bank.h b/lib/engine/components/opal/opal-bank.h
index 62c302f..feb05ff 100644
--- a/lib/engine/components/opal/opal-bank.h
+++ b/lib/engine/components/opal/opal-bank.h
@@ -39,12 +39,17 @@
 #ifndef __OPAL_BANK_H__
 #define __OPAL_BANK_H__
 
+#include "config.h"
+
 #include "contact-core.h"
 #include "presence-core.h"
 
 #include "opal-account.h"
 #include "ekiga-settings.h"
 
+#include "sip-endpoint.h"
+#include "h323-endpoint.h"
+
 namespace Opal
 {
   /**
@@ -61,8 +66,28 @@ namespace Opal
     friend class Account;
 public:
 
+    /* The Opal::Bank is implemented using the H.323 and SIP
+     * EndPoints from OPAL.
+     *
+     * We pass them as arguments instead of using the Opal::CallManager
+     * to prevent useless references to the CallManager. We are using
+     * shared_ptr's, they control the destruction order of objects, which
+     * is not what Opal was designed for.
+     *
+     * We can safely rely on the H.323 & SIP EndPoints as long as:
+     *   1) The Opal::Bank and its Opal::Accounts are destroyed
+     *      before the Opal EndPoints.
+     *   2) They are destroyed before the Opal::CallManager.
+     *
+     * However, the SIP & H.323 EndPoints have a null_deleter. That means
+     * that Opal is taking care of deleting them. They are not deleted when
+     * the last object having a reference to them is deleted.
+     */
     Bank (Ekiga::ServiceCore& _core,
-          boost::shared_ptr<Opal::CallManager> _call_manager);
+#ifdef HAVE_H323
+          Opal::H323::EndPoint* _h323_endpoint,
+#endif
+          Opal::Sip::EndPoint* _sip_endpoint);
 
     ~Bank ();
 
@@ -90,6 +115,10 @@ public:
      */
     AccountPtr find_account (const std::string& aor);
 
+    /** Clear all Accounts
+     */
+    void clear ();
+
     /* this object is an Ekiga::Cluster */
     void visit_heaps (boost::function1<bool, Ekiga::HeapPtr> visitor) const;
 
@@ -106,9 +135,8 @@ private:
     void set_ready ();
     bool is_ready;
 
-    boost::weak_ptr<Opal::CallManager> call_manager;
-
     boost::weak_ptr<Ekiga::PresenceCore> presence_core;
+    boost::weak_ptr<Ekiga::CallCore> call_core;
     boost::shared_ptr<Ekiga::NotificationCore> notification_core;
     boost::shared_ptr<Ekiga::PersonalDetails> personal_details;
     boost::shared_ptr<Ekiga::AudioOutputCore> audiooutput_core;
@@ -142,6 +170,11 @@ private:
     void add_actions ();
 
     Ekiga::Settings *protocols_settings;
+
+#ifdef HAVE_H323
+    Opal::H323::EndPoint* h323_endpoint;
+#endif
+    Opal::Sip::EndPoint* sip_endpoint;
   };
 
   /**
diff --git a/lib/engine/components/opal/opal-call-manager.cpp 
b/lib/engine/components/opal/opal-call-manager.cpp
index 7f20496..cf46d53 100644
--- a/lib/engine/components/opal/opal-call-manager.cpp
+++ b/lib/engine/components/opal/opal-call-manager.cpp
@@ -545,8 +545,10 @@ void CallManager::set_stun_enabled (bool enabled)
     stun_thread = new StunDetector (stun_server, *this, queue);
     patience = 20;
     Ekiga::Runtime::run_in_main (boost::bind (&CallManager::HandleSTUNResult, this), 1);
-  } else
+  }
+  else {
     ready ();
+  }
 
   PTRACE (4, "Opal::CallManager\tSTUN Detection: " << enabled);
 }
diff --git a/lib/engine/components/opal/opal-process.cpp b/lib/engine/components/opal/opal-process.cpp
index 188a6b3..841f814 100644
--- a/lib/engine/components/opal/opal-process.cpp
+++ b/lib/engine/components/opal/opal-process.cpp
@@ -58,6 +58,13 @@ GnomeMeeting::GnomeMeeting (Ekiga::ServiceCore& _core)
 GnomeMeeting::~GnomeMeeting ()
 {
   boost::shared_ptr<Ekiga::AccountCore> acore = account_core.lock ();
+  // First remove all Opal::Accounts from our Bank.
+  //
+  // Do it forcibly so we're sure the accounts are freed before our
+  // reference to the endpoints. Indeed they try to unregister from
+  // presence when killed, and that gives a crash if the call manager
+  // is already gone!
+  bank->clear ();
   acore->remove_bank (bank);
 
   boost::shared_ptr<Ekiga::PresenceCore> pcore = presence_core.lock ();
@@ -93,13 +100,13 @@ void GnomeMeeting::Start ()
   boost::shared_ptr<Ekiga::CallCore> ccore = call_core.lock ();
 
   call_manager = boost::shared_ptr<Opal::CallManager> (new Opal::CallManager (core));
-  bank = boost::shared_ptr<Opal::Bank> (new Opal::Bank (core, call_manager));
-
+  bank = boost::shared_ptr<Opal::Bank> (new Opal::Bank (core,
+                                                        (Opal::H323::EndPoint*) call_manager->FindEndPoint 
("h323"),
+                                                        (Opal::Sip::EndPoint*) call_manager->FindEndPoint 
("sip")));
   acore->add_bank (bank);
   pcore->add_cluster (bank);
   core.add (bank);
+  ccore->add_manager (call_manager);
   call_manager->setup ();
   pcore->add_presence_publisher (bank);
-
-  ccore->add_manager (call_manager);
 }
diff --git a/lib/engine/protocol/call-core.cpp b/lib/engine/protocol/call-core.cpp
index b519efb..f28bd78 100644
--- a/lib/engine/protocol/call-core.cpp
+++ b/lib/engine/protocol/call-core.cpp
@@ -193,8 +193,9 @@ void CallCore::on_manager_ready (boost::shared_ptr<CallManager> manager)
   manager_ready (manager);
   nr_ready++;
 
-  if (nr_ready >= managers.size ())
+  if (nr_ready >= managers.size ()) {
     ready ();
+  }
 }