Merge lp:~unity-api-team/indicator-network/krillin-wifi into lp:indicator-network/14.10

[Antti Kaijanmäki]

Fix wifi toggle on krillin.

fix wifi toggle on krillin.

[Charles Kerr (charlesk)]

LGTM

review: Approve

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:429
http://jenkins.qa.ubuntu.com/job/indicator-network-ci/181/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-network-utopic-amd64-ci/55
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-network-utopic-armhf-ci/55
        deb: http://jenkins.qa.ubuntu.com/job/indicator-network-utopic-armhf-ci/55/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/indicator-network-ci/181/rebuild

review: Approve (continuous-integration)

[Charles Kerr (charlesk)]

Nice thread fixes. :)

There are a couple of warts here that I've talked with Antti about offline, but they don't block this Critical and can wait until the next MP.

review: Approve

[Antti Kaijanmäki (kaijanmaki)]

just a simple timeout increase.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:433
http://jenkins.qa.ubuntu.com/job/indicator-network-ci/182/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-network-utopic-amd64-ci/56
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-network-utopic-armhf-ci/56
        deb: http://jenkins.qa.ubuntu.com/job/indicator-network-utopic-armhf-ci/56/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/indicator-network-ci/182/rebuild

review: Approve (continuous-integration)

=== modified file 'src/connectivity-cpp/dbus-cpp/services/urfkill.h'
--- src/connectivity-cpp/dbus-cpp/services/urfkill.h	2014-08-19 19:55:15 +0000
+++ src/connectivity-cpp/dbus-cpp/services/urfkill.h	2014-09-26 13:07:38 +0000
@@ -162,7 +162,7 @@
 
                 static std::chrono::milliseconds default_timeout()
                 {
-                    return std::chrono::seconds{5};
+                    return std::chrono::seconds{15};
                 }
             };
 
@@ -179,7 +179,7 @@
 
                 static std::chrono::milliseconds default_timeout()
                 {
-                    return std::chrono::seconds{5};
+                    return std::chrono::seconds{15};
                 }
             };
 
@@ -196,7 +196,7 @@
 
                 static std::chrono::milliseconds default_timeout()
                 {
-                    return std::chrono::seconds{1};
+                    return std::chrono::seconds{5};
                 }
             };
         };
 
=== modified file 'src/connectivity-cpp/include/connectivity/networking/manager.h'
--- src/connectivity-cpp/include/connectivity/networking/manager.h	2014-08-19 19:55:15 +0000
+++ src/connectivity-cpp/include/connectivity/networking/manager.h	2014-09-26 13:07:38 +0000
@@ -128,6 +128,11 @@
      */
     virtual const core::Property<std::uint32_t>& characteristics() const = 0;
 
+    virtual const core::Property<bool>& hasWifi() const = 0;
+    virtual const core::Property<bool>& wifiEnabled() const = 0;
+    virtual bool enableWifi() = 0;
+    virtual bool disableWifi() = 0;
+
 protected:
     /**
      * @brief The default constructor is protected.
 
=== modified file 'src/connectivity-cpp/src/platform/nmofono/kill-switch.cpp'
--- src/connectivity-cpp/src/platform/nmofono/kill-switch.cpp	2014-08-19 19:55:15 +0000
+++ src/connectivity-cpp/src/platform/nmofono/kill-switch.cpp	2014-09-26 13:07:38 +0000
@@ -38,6 +38,8 @@
     {
         switch(value) {
         case URfkill::Interface::Killswitch::State::not_available:
+            state.set(KillSwitch::State::not_available);
+            break;
         case URfkill::Interface::Killswitch::State::unblocked:
             state.set(KillSwitch::State::unblocked);
             break;
 
=== modified file 'src/connectivity-cpp/src/platform/nmofono/kill-switch.h'
--- src/connectivity-cpp/src/platform/nmofono/kill-switch.h	2014-08-19 19:55:15 +0000
+++ src/connectivity-cpp/src/platform/nmofono/kill-switch.h	2014-09-26 13:07:38 +0000
@@ -61,6 +61,7 @@
 
     enum class State
     {
+        not_available,
         unblocked,
         soft_blocked,
         hard_blocked
 
=== modified file 'src/connectivity-cpp/src/platform/nmofono/manager.cpp'
--- src/connectivity-cpp/src/platform/nmofono/manager.cpp	2014-08-28 12:13:29 +0000
+++ src/connectivity-cpp/src/platform/nmofono/manager.cpp	2014-09-26 13:07:38 +0000
@@ -55,6 +55,31 @@
     core::Property<connectivity::networking::Manager::NetworkingStatus> m_status;
     core::Property<std::uint32_t> m_characteristics;
     std::unique_ptr<State> m_state;
+
+    core::Property<bool> m_hasWifi;
+    core::Property<bool> m_wifiEnabled  ;
+    std::shared_ptr<KillSwitch> m_wifiKillSwitch;
+
+    void updateHasWifi()
+    {
+        if (m_wifiKillSwitch->state().get() != KillSwitch::State::not_available) {
+            m_hasWifi.set(true);
+            if (m_wifiKillSwitch->state().get() == KillSwitch::State::unblocked)
+                m_wifiEnabled.set(true);
+            else
+                m_wifiEnabled.set(false);
+            return;
+        }
+
+        // ok, killswitch not supported, but we still might have wifi devices
+        bool haswifi = false;
+        for (auto link : m_links.get()) {
+            if (link->type() == networking::Link::Type::wifi)
+                haswifi = true;
+        }
+        m_hasWifi.set(haswifi);
+        m_wifiEnabled.set(haswifi);
+    }
 };
 }
 }
@@ -143,6 +168,10 @@
     /// @todo offload the initialization to a thread or something
     /// @todo those Id() thingies
 
+    p->m_wifiKillSwitch = std::make_shared<KillSwitch>(p->urfkill,
+                                                       p->urfkill->switches[fdo::URfkill::Interface::Killswitch::Type::wlan]);
+    p->m_wifiKillSwitch->state().changed().connect(std::bind(&Private::updateHasWifi, p.get()));
+
     p->nm->device_added->connect([this](const dbus::types::ObjectPath &path){
         std::cout << "Device Added:" << path.as_string() << std::endl;
         auto links = p->m_links.get();
@@ -158,10 +187,11 @@
         if (dev.type() == NM::Interface::Device::Type::wifi) {
             links.insert(std::make_shared<wifi::Link>(dev,
                                                       *p->nm.get(),
-                                                      std::make_shared<KillSwitch>(p->urfkill,
-                                                                                   p->urfkill->switches[fdo::URfkill::Interface::Killswitch::Type::wlan])));
+                                                       p->m_wifiKillSwitch));
             p->m_links.set(links);
         }
+
+        p->updateHasWifi();
     });
     p->nm->device_removed->connect([this](const dbus::types::ObjectPath &path){
         std::cout << "Device Removed:" << path.as_string() << std::endl;
@@ -173,6 +203,8 @@
             }
         }
         p->m_links.set(links);
+
+        p->updateHasWifi();
     });
     std::set<std::shared_ptr<networking::Link> > links;
     for(auto dev : p->nm->get_devices()) {
@@ -182,8 +214,7 @@
             std::shared_ptr<networking::Link> link;
             link.reset(new wifi::Link(dev,
                                       *p->nm.get(),
-                                      std::make_shared<KillSwitch>(p->urfkill,
-                                                                   p->urfkill->switches[fdo::URfkill::Interface::Killswitch::Type::wlan])));
+                                      p->m_wifiKillSwitch));
             links.insert(link);
             break;
         }
@@ -219,6 +250,8 @@
 
     /// @todo set by the default connections.
     p->m_characteristics.set(networking::Link::Characteristics::empty);
+
+    p->updateHasWifi();
 }
 
 void
@@ -269,3 +302,55 @@
 {
     return p->m_characteristics;
 }
+
+const core::Property<bool>&
+Manager::hasWifi() const
+{
+    return p->m_hasWifi;
+}
+
+const core::Property<bool>&
+Manager::wifiEnabled() const
+{
+    return p->m_wifiEnabled;
+}
+
+
+bool
+Manager::enableWifi()
+{
+    if (!p->m_hasWifi.get())
+        return false;
+
+    try {
+        if (p->m_wifiKillSwitch->state() == KillSwitch::State::soft_blocked) {
+            // try to unblock. throws if fails.
+            p->m_wifiKillSwitch->unblock();
+        }
+        p->nm->wireless_enabled->set(true);
+    } catch(std::runtime_error &e) {
+        std::cerr << __PRETTY_FUNCTION__ << ": " << e.what() << std::endl;
+        return false;
+    }
+    return true;
+}
+
+bool
+Manager::disableWifi()
+{
+    if (!p->m_hasWifi.get())
+        return false;
+
+    try {
+        if (p->m_wifiKillSwitch->state() == KillSwitch::State::unblocked) {
+            // block the device. that will disable it also
+            p->m_wifiKillSwitch->block();
+        }
+        p->nm->wireless_enabled->set(false);
+    } catch(std::runtime_error &e) {
+        std::cerr << __PRETTY_FUNCTION__ << ": " << e.what() << std::endl;
+        return false;
+    }
+    return true;
+}
+
 
=== modified file 'src/connectivity-cpp/src/platform/nmofono/manager.h'
--- src/connectivity-cpp/src/platform/nmofono/manager.h	2014-08-19 19:55:15 +0000
+++ src/connectivity-cpp/src/platform/nmofono/manager.h	2014-09-26 13:07:38 +0000
@@ -51,6 +51,12 @@
     void disableFlightMode() override;
     const core::Property<connectivity::networking::Manager::FlightModeStatus>& flightMode() const override;
 
+    const core::Property<bool>& hasWifi() const override;
+    const core::Property<bool>& wifiEnabled() const override;
+
+    bool enableWifi() override;
+    bool disableWifi() override;
+
     const core::Property<std::set<std::shared_ptr<connectivity::networking::Link>>>& links() const;
     const core::Property<std::set<std::shared_ptr<connectivity::networking::Service>>>&services() const override;
     const core::Property<connectivity::networking::Manager::NetworkingStatus> & status() const override;
 
=== modified file 'src/connectivity-cpp/src/platform/nmofono/wifi/link.cpp'
--- src/connectivity-cpp/src/platform/nmofono/wifi/link.cpp	2014-09-04 07:48:13 +0000
+++ src/connectivity-cpp/src/platform/nmofono/wifi/link.cpp	2014-09-26 13:07:38 +0000
@@ -100,6 +100,8 @@
     org::freedesktop::NetworkManager::Interface::NetworkManager nm;
 
     KillSwitch::Ptr killSwitch;
+    // hack hack
+    std::vector<core::ScopedConnection> switchConnection;
 
     std::map<AccessPointKey, std::shared_ptr<GroupedAccessPoint>> grouper;
     bool disabled;
@@ -146,9 +148,9 @@
         // std::uint32_t reason    = std::get<2>(args);
         updateDeviceState(new_state);
     });
-    p->killSwitch->state().changed().connect([this](platform::nmofono::KillSwitch::State){
+    p->switchConnection.emplace_back(p->killSwitch->state().changed().connect([this](platform::nmofono::KillSwitch::State){
         updateDeviceState(p->lastState);
-    });
+    }));
 }
 
 Link::~Link()
@@ -428,6 +430,7 @@
         case KillSwitch::State::soft_blocked:
             p->status_.set(Status::disabled);
             break;
+        case KillSwitch::State::not_available:
         case KillSwitch::State::unblocked:
             p->status_.set(Status::offline);
         }
 
=== modified file 'src/indicator/quick-access-section.cpp'
--- src/indicator/quick-access-section.cpp	2014-05-27 16:06:14 +0000
+++ src/indicator/quick-access-section.cpp	2014-09-26 13:07:38 +0000
@@ -65,10 +65,19 @@
         }
     });
     m_flightModeSwitch->activated().connect([this](){
-        if (m_flightModeSwitch->state().get())
-            m_manager->enableFlightMode();
-        else
-            m_manager->disableFlightMode();
+        if (m_flightModeSwitch->state().get()) {
+            try {
+                m_manager->enableFlightMode();
+            } catch (const std::exception &e) {
+                std::cout << e.what() << std::endl;
+            }
+        } else {
+            try {
+                m_manager->disableFlightMode();
+            } catch (const std::exception &e) {
+                std::cout << e.what() << std::endl;
+            }
+        }
     });
 
     m_actionGroupMerger->add(*m_flightModeSwitch);
 
=== modified file 'src/indicator/wifi-link-item.h'
--- src/indicator/wifi-link-item.h	2014-08-15 11:49:44 +0000
+++ src/indicator/wifi-link-item.h	2014-09-26 13:07:38 +0000
@@ -42,8 +42,6 @@
     /// @todo do something with me...
     Action::Ptr m_actionBusy;
 
-    SwitchItem::Ptr m_switch;
-
     networking::wifi::AccessPoint::Ptr m_activeAccessPoint;
     std::map<networking::wifi::AccessPoint::Ptr, AccessPointItem::Ptr> m_accessPoints;
 
@@ -85,40 +83,6 @@
 
         m_rootMerger = std::make_shared<MenuMerger>();
 
-        m_switch = std::make_shared<SwitchItem>(_("Wi-Fi"), "wifi", "enable");
-        m_actionGroupMerger->add(*m_switch);
-        switch (m_link->status().get()) {
-        case networking::Link::Status::disabled:
-            m_switch->state().set(false);
-            break;
-        case networking::Link::Status::offline:
-        case networking::Link::Status::connecting:
-        case networking::Link::Status::connected:
-        case networking::Link::Status::online:
-            m_switch->state().set(true);
-            break;
-        }
-        m_link->status().changed().connect([this](networking::Link::Status value){
-            switch (value) {
-            case networking::Link::Status::disabled:
-                m_switch->state().set(false);
-                break;
-            case networking::Link::Status::offline:
-            case networking::Link::Status::connecting:
-            case networking::Link::Status::connected:
-            case networking::Link::Status::online:
-                m_switch->state().set(true);
-                break;
-            }
-        });
-        m_switch->activated().connect([this](){
-            if (m_switch->state().get()) {
-                m_link->enable();
-            } else {
-                m_link->disable();
-            }
-        });
-
         m_accessPointCompare = [](MenuItem::Ptr a, MenuItem::Ptr b){
             // order alphabetically by SSID
 
@@ -152,7 +116,6 @@
         m_otherNetwork = std::make_shared<TextItem>(_("Other network…"), "wifi", "othernetwork");
         //m_actionGroupMerger->add(*m_otherNetwork);
 
-        m_topMenu->append(*m_switch);
         m_rootMerger->append(m_topMenu);
 
         m_apsMerger->append(m_connectedBeforeApsMenu);
 
=== modified file 'src/indicator/wifi-section.cpp'
--- src/indicator/wifi-section.cpp	2014-05-12 11:36:59 +0000
+++ src/indicator/wifi-section.cpp	2014-09-26 13:07:38 +0000
@@ -32,6 +32,8 @@
     Menu::Ptr m_menu;
     Menu::Ptr m_settingsMenu;
 
+    SwitchItem::Ptr m_switch;
+
     WifiLinkItem::Ptr m_wifiLink;
     TextItem::Ptr m_openWifiSettings;
 
@@ -44,19 +46,36 @@
         m_settingsMenu = std::make_shared<Menu>();
 
 
-        for (auto link : m_manager->links().get()) {
-            auto wifi_link = std::dynamic_pointer_cast<networking::wifi::Link>(link);
-            m_wifiLink = std::make_shared<WifiLinkItem>(wifi_link);
-
-            m_actionGroupMerger->add(*m_wifiLink);
-            m_menu->append(*m_wifiLink);
-            m_settingsMenu->append(*m_wifiLink);
-
-            // just take the first one now.
-            /// @todo multiple links and links()->changed()
-            break;
+        m_switch = std::make_shared<SwitchItem>(_("Wi-Fi"), "wifi", "enable");
+
+        /// @todo don't now really care about actully being able to detach the whole
+        ///       wifi chipset. on touch devices we always have wifi.
+        if (m_manager->hasWifi().get()) {
+            m_actionGroupMerger->add(*m_switch);
+            m_menu->append(*m_switch);
+            m_settingsMenu->append(*m_switch);
         }
 
+        m_switch->state().set(m_manager->wifiEnabled().get());
+        m_manager->wifiEnabled().changed().connect([this](bool value) {
+            m_switch->state().set(value);
+        });
+        m_switch->activated().connect([this](){
+            if (m_switch->state().get()) {
+                if (!m_manager->enableWifi()) {
+                    /// try to work around the switch getting out of state on unity8 side
+                    m_switch->state().set(false);
+                }
+            } else {
+                if (!m_manager->disableWifi())
+                    m_switch->state().set(true);
+            }
+        });
+
+
+        m_manager->links().changed().connect(std::bind(&Private::updateLinks, this));
+        updateLinks();
+
         m_openWifiSettings = std::make_shared<TextItem>(_("Wi-Fi settings…"), "wifi", "settings");
         m_openWifiSettings->activated().connect([](){
             UrlDispatcher::send("settings:///system/wifi", [](std::string url, bool success){
@@ -69,6 +88,29 @@
         m_menu->append(*m_openWifiSettings);
     }
 
+    void updateLinks()
+    {
+        // remove all and recreate. we have top 1 now anyway
+        if (m_wifiLink) {
+            m_actionGroupMerger->remove(*m_wifiLink);
+            m_menu->removeAll(*m_wifiLink);
+            m_settingsMenu->removeAll(*m_wifiLink);
+            m_wifiLink.reset();
+        }
+
+        for (auto link : m_manager->links().get()) {
+            auto wifi_link = std::dynamic_pointer_cast<networking::wifi::Link>(link);
+            m_wifiLink = std::make_shared<WifiLinkItem>(wifi_link);
+
+            m_actionGroupMerger->add(*m_wifiLink);
+            m_menu->append(*m_wifiLink);
+            m_settingsMenu->append(*m_wifiLink);
+
+            // just take the first one
+            break;
+        }
+    }
+
 public:
 };
 
 
=== modified file 'src/menumodel-cpp/action-group-exporter.h'
--- src/menumodel-cpp/action-group-exporter.h	2014-08-14 23:23:06 +0000
+++ src/menumodel-cpp/action-group-exporter.h	2014-09-26 13:07:38 +0000
@@ -63,19 +63,21 @@
             return;
         }
 
+
+        auto group = m_gSimpleActionGroup;
         GMainLoopDispatch([=](){
             for (auto action : actionGroup->actions()) {
-                g_action_map_add_action(G_ACTION_MAP(m_gSimpleActionGroup.get()), action->gaction().get());
+                g_action_map_add_action(G_ACTION_MAP(group.get()), action->gaction().get());
             }
         });
-        actionGroup->actionAdded().connect([this](Action::Ptr action){
+        actionGroup->actionAdded().connect([=](Action::Ptr action){
             GMainLoopDispatch([=](){
-                g_action_map_add_action(G_ACTION_MAP(m_gSimpleActionGroup.get()), action->gaction().get());
+                g_action_map_add_action(G_ACTION_MAP(group.get()), action->gaction().get());
             });
         });
-        actionGroup->actionRemoved().connect([this](Action::Ptr action){
+        actionGroup->actionRemoved().connect([=](Action::Ptr action){
             GMainLoopDispatch([=](){
-                g_action_map_remove_action(G_ACTION_MAP(m_gSimpleActionGroup.get()),
+                g_action_map_remove_action(G_ACTION_MAP(group.get()),
                                            action->name().c_str());
             });
         });
 
=== modified file 'src/menumodel-cpp/action.cpp'
--- src/menumodel-cpp/action.cpp	2014-08-14 23:23:06 +0000
+++ src/menumodel-cpp/action.cpp	2014-09-26 13:07:38 +0000
@@ -85,6 +85,7 @@
     g_signal_handler_disconnect(m_gaction.get(), m_activateHandlerId);
     if (m_changeStateHandlerId)
         g_signal_handler_disconnect(m_gaction.get(), m_changeStateHandlerId);
+    GMainLoopSync([]{});
 }
 
 std::string
@@ -99,8 +100,9 @@
 {
     std::lock_guard<std::recursive_mutex> lg(m_mutex);
     m_state = value;
+    auto action = m_gaction;
     GMainLoopDispatch([=](){
-      g_simple_action_set_state(G_SIMPLE_ACTION(m_gaction.get()), value);
+      g_simple_action_set_state(G_SIMPLE_ACTION(action.get()), value);
     });
     /// @todo state changes don't work properly. We probably need to make the
     ///       state a Property to be able to get signals on change.
 
=== modified file 'src/menumodel-cpp/gio-helpers/util.cpp'
--- src/menumodel-cpp/gio-helpers/util.cpp	2014-08-14 23:23:06 +0000
+++ src/menumodel-cpp/gio-helpers/util.cpp	2014-09-26 13:07:38 +0000
@@ -20,7 +20,7 @@
 #include "util.h"
 
 std::mutex GMainLoopDispatch::_lock;
-std::vector<GMainLoopDispatch::Func *> GMainLoopDispatch::_funcs;
+std::list<GMainLoopDispatch::Func *> GMainLoopDispatch::_funcs;
 
 gboolean
 GMainLoopDispatch::dispatch_cb(gpointer)
@@ -37,7 +37,12 @@
 GMainLoopDispatch::GMainLoopDispatch(std::function<void()> func)
 {
     if (g_main_context_acquire(g_main_context_default())) {
-        func();
+        if (_funcs.empty())
+            func();
+        else {
+            std::function<void()> *funcPtr = new std::function<void()>(func);
+            _funcs.push_back(funcPtr);
+        }
         g_main_context_release(g_main_context_default());
     } else {
         std::lock_guard<std::mutex> lock(_lock);
 
=== modified file 'src/menumodel-cpp/gio-helpers/util.h'
--- src/menumodel-cpp/gio-helpers/util.h	2014-08-15 11:59:20 +0000
+++ src/menumodel-cpp/gio-helpers/util.h	2014-09-26 13:07:38 +0000
@@ -28,6 +28,7 @@
 #include <condition_variable>
 #include <iostream>
 #include <vector>
+#include <list>
 
 struct GMainLoopSync
 {
@@ -72,8 +73,8 @@
 public:
     typedef std::function<void()> Func;
     static std::mutex _lock;
-    // vector keeps the order of the functions
-    static std::vector<Func *> _funcs;
+    // list keeps the order of the functions
+    static std::list<Func *> _funcs;
 
     //GMainLoopDispatch() = delete;
     GMainLoopDispatch(std::function<void()> func);
 
=== modified file 'src/menumodel-cpp/menu-merger.h'
--- src/menumodel-cpp/menu-merger.h	2014-08-14 23:23:06 +0000
+++ src/menumodel-cpp/menu-merger.h	2014-09-26 13:07:38 +0000
@@ -56,15 +56,18 @@
                       gint        added)
     {
         int offset = m_startPositions[model] + position;
+        auto menu = m_gmenu;
 
-        for (int i = 0; i < removed; ++i) {
-            g_menu_remove(m_gmenu.get(), offset);
-        }
-        for (int i = added-1; i >= 0; --i) {
-            auto item = g_menu_item_new_from_model(model, position + i);
-            g_menu_insert_item(m_gmenu.get(), offset, item);
-            g_object_unref(item);
-        }
+        GMainLoopDispatch([=](){
+            for (int i = 0; i < removed; ++i) {
+                g_menu_remove(menu.get(), offset);
+            }
+            for (int i = added-1; i >= 0; --i) {
+                auto item = g_menu_item_new_from_model(model, position + i);
+                g_menu_insert_item(menu.get(), offset, item);
+                g_object_unref(item);
+            }
+        });
 
         int delta = added - removed;
         bool update = false;
@@ -89,6 +92,12 @@
         m_gmenu = make_gmenu_ptr();
     }
 
+    ~MenuMerger()
+    {
+        clear();
+        GMainLoopSync([]{});
+    }
+
     void append(MenuModel::Ptr menu)
     {
         // calculate the start position for the items for the new menu
@@ -106,7 +115,6 @@
 
         // add all items
         itemsChanged(*menu, 0, 0, g_menu_model_get_n_items(*menu));
-
         m_handlerId[*menu] = g_signal_connect(menu->operator GMenuModel *(),
                                               "items-changed",
                                               G_CALLBACK(MenuMerger::items_changed_cb),
 
=== modified file 'src/menumodel-cpp/menu.h'
--- src/menumodel-cpp/menu.h	2014-08-14 23:23:06 +0000
+++ src/menumodel-cpp/menu.h	2014-09-26 13:07:38 +0000
@@ -51,14 +51,18 @@
     virtual ~Menu()
     {
         std::lock_guard<std::recursive_mutex> lg(m_mutex);
+        clear();
+        GMainLoopSync([]{});
     }
 
     void append(MenuItem::Ptr item)
     {
         std::lock_guard<std::recursive_mutex> lg(m_mutex);
         m_items.push_back(item);
+        // prevent this-> from being captured
+        auto menu = m_gmenu;
         GMainLoopDispatch([=](){
-            g_menu_append_item(m_gmenu.get(), item->gmenuitem());
+            g_menu_append_item(menu.get(), item->gmenuitem());
         });
         if (std::count(m_items.begin(), m_items.end(), item) == 1) {
             /// @todo disconenct
@@ -66,9 +70,11 @@
                 int index = 0;
                 for (auto iter = m_items.begin(); iter != m_items.end(); ++iter) {
                     if (*iter == item) {
+                        // prevent this-> from being captured
+                        auto menu = m_gmenu;
                         GMainLoopDispatch([=](){
-                            g_menu_remove(m_gmenu.get(), index);
-                            g_menu_insert_item(m_gmenu.get(), index, item->gmenuitem());
+                            g_menu_remove(menu.get(), index);
+                            g_menu_insert_item(menu.get(), index, item->gmenuitem());
                         });
                     }
                     ++index;
@@ -88,8 +94,11 @@
             ++index;
             ++iter;
         }
+
+        // prevent this-> from being captured
+        auto menu = m_gmenu;
         GMainLoopDispatch([=](){
-            g_menu_insert_item(m_gmenu.get(), index, item->gmenuitem());
+            g_menu_insert_item(menu.get(), index, item->gmenuitem());
         });
         m_items.insert(position, item);
     }
@@ -132,8 +141,10 @@
         if (iter == m_items.end())
          return;
 
+        // prevent this-> from being captured
+        auto menu = m_gmenu;
         GMainLoopDispatch([=](){
-            g_menu_remove(m_gmenu.get(), index);
+            g_menu_remove(menu.get(), index);
         });
         m_items.erase(item);
     }
@@ -147,9 +158,11 @@
         // work reversed so that GMenu positions match tbe m_items positions
         int index = m_items.size()-1;
         for (auto iter = m_items.rbegin(); iter != m_items.rend(); ++iter) {
+            // prevent this-> from being captured
+            auto menu = m_gmenu;
             if (*iter == item) {
                 GMainLoopDispatch([=](){
-                    g_menu_remove(m_gmenu.get(), index);
+                    g_menu_remove(menu.get(), index);
                 });
             }
             --index;
@@ -201,8 +214,10 @@
     void clear()
     {
         std::lock_guard<std::recursive_mutex> lg(m_mutex);
+        // prevent this-> from being captured
+        auto menu = m_gmenu;
         GMainLoopDispatch([=](){
-            g_menu_remove_all(m_gmenu.get());
+            g_menu_remove_all(menu.get());
         });
         m_items.clear();
     }