[niepce: 17/22] Hold the selectable as a weakPtr. So that we can ensure memory safety.



commit 291b191d0eea78eea1e95b626e415b7af9b10d2a
Author: Hubert Figuière <hub figuiere net>
Date:   Sun Jul 27 11:25:14 2014 +0200

    Hold the selectable as a weakPtr. So that we can ensure memory safety.
    
    Fix a crash when quitting the app and something is selected.

 src/niepce/ui/moduleshell.cpp         |    2 +-
 src/niepce/ui/niepcewindow.cpp        |    4 +-
 src/niepce/ui/selectioncontroller.cpp |   95 ++++++++++++++++++++-------------
 src/niepce/ui/selectioncontroller.hpp |   13 +++--
 4 files changed, 69 insertions(+), 45 deletions(-)
---
diff --git a/src/niepce/ui/moduleshell.cpp b/src/niepce/ui/moduleshell.cpp
index 6bc3e5f..5631fa9 100644
--- a/src/niepce/ui/moduleshell.cpp
+++ b/src/niepce/ui/moduleshell.cpp
@@ -195,7 +195,7 @@ Gtk::Widget * ModuleShell::buildWidget()
         new GridViewModule(*this, m_selection_controller->get_list_store()));
     add_library_module(m_gridview, _("Library"));
 
-    m_selection_controller->add_selectable(m_gridview.get());
+    m_selection_controller->add_selectable(m_gridview);
     m_selection_controller->signal_selected
         .connect(sigc::mem_fun(*this, &ModuleShell::on_selected));
     m_selection_controller->signal_activated
diff --git a/src/niepce/ui/niepcewindow.cpp b/src/niepce/ui/niepcewindow.cpp
index b0ff241..ccac139 100644
--- a/src/niepce/ui/niepcewindow.cpp
+++ b/src/niepce/ui/niepcewindow.cpp
@@ -1,7 +1,7 @@
 /*
  * niepce - ui/niepcewindow.cpp
  *
- * Copyright (C) 2007-2013 Hubert Figuiere
+ * Copyright (C) 2007-2014 Hubert Figuiere
  *
  * This program is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -133,7 +133,7 @@ NiepceWindow::_createModuleShell()
     m_vbox.pack_start(m_statusBar, Gtk::PACK_SHRINK);
     m_statusBar.push(Glib::ustring(_("Ready")));
 
-    selection_controller->add_selectable(m_filmstrip.get());
+    selection_controller->add_selectable(m_filmstrip);
 
     m_vbox.show_all_children();
     m_vbox.show();
diff --git a/src/niepce/ui/selectioncontroller.cpp b/src/niepce/ui/selectioncontroller.cpp
index 954e4a2..75449fa 100644
--- a/src/niepce/ui/selectioncontroller.cpp
+++ b/src/niepce/ui/selectioncontroller.cpp
@@ -48,24 +48,27 @@ void SelectionController::_added()
        m_imageliststore->set_parent_controller(m_parent);
 }
 
-void SelectionController::add_selectable(IImageSelectable * selectable)
-{ 
-    DBG_OUT("added %p", (void*)selectable);
-    m_selectables.push_back(selectable);
+void SelectionController::add_selectable(const IImageSelectable::WeakPtr & selectableWeak)
+{
+    auto selectable = selectableWeak.lock();
+    DBG_OUT("added %p", (void*)selectable.get());
+    m_selectables.push_back(selectableWeak);
     selectable->image_list()->signal_selection_changed().connect(
-        sigc::bind(sigc::mem_fun(*this, &SelectionController::selected),
-                   selectable));
+        [this, selectableWeak](){
+            this->selected(selectableWeak);
+        });
     selectable->image_list()->signal_item_activated().connect(
-        sigc::bind(sigc::mem_fun(*this, &SelectionController::activated),
-                   selectable));
+        sigc::bind(
+            sigc::mem_fun(*this, &SelectionController::activated),
+                   selectableWeak));
 }
 
 
 void SelectionController::activated(const Gtk::TreeModel::Path & path,
-                                    IImageSelectable * /*selectable*/)
+                                    const IImageSelectable::WeakPtr & /*selectable*/)
 {
     fwk::AutoFlag f(m_in_handler);
-    Gtk::TreeIter iter = m_imageliststore->get_iter(path);
+    auto iter = m_imageliststore->get_iter(path);
     if(iter) {
         eng::LibFile::Ptr file = (*iter)[m_imageliststore->columns().m_libfile];
         if(file) {
@@ -76,7 +79,7 @@ void SelectionController::activated(const Gtk::TreeModel::Path & path,
     }
 }
 
-void SelectionController::selected(IImageSelectable * selectable)
+void SelectionController::selected(const IImageSelectable::WeakPtr & selectableWeak)
 {
     if(m_in_handler) {
         DBG_OUT("%p already in handler", (void*)this);
@@ -85,9 +88,17 @@ void SelectionController::selected(IImageSelectable * selectable)
 
     fwk::AutoFlag f(m_in_handler);
 
+    auto selectable = selectableWeak.lock();
+    if (!selectable) {
+        // it can be, but at that point we should be terminating the app.
+        ERR_OUT("selectable is null");
+        return;
+    }
+
     auto selection = selectable->get_selected();
-    for(auto iter : m_selectables) {
-        if(iter != selectable) {
+    for(auto iterWeak : m_selectables) {
+        auto iter = iterWeak.lock();
+        if(iter && iter != selectable) {
             iter->select_image(selection);
         }
     }
@@ -105,7 +116,11 @@ libraryclient::LibraryClient::Ptr SelectionController::getLibraryClient()
 eng::library_id_t SelectionController::get_selection() const
 {
     DBG_ASSERT(!m_selectables.empty(), "selectables list can't be empty");
-    return m_selectables[0]->get_selected();
+
+    auto selectable = m_selectables[0].lock();
+    DBG_ASSERT(static_cast<bool>(selectable), "selectable is null");
+
+    return selectable->get_selected();
 }
 
 eng::LibFile::Ptr SelectionController::get_file(eng::library_id_t id) const
@@ -119,32 +134,36 @@ eng::LibFile::Ptr SelectionController::get_file(eng::library_id_t id) const
 
 void SelectionController::_selection_move(bool backwards)
 {
-       eng::library_id_t selection = get_selection();
-    Gtk::TreeIter iter = m_imageliststore->get_iter_from_id(selection);
-    if(iter) {
-        if(backwards) {
-            if(iter != m_imageliststore->children().begin()) {
-                --iter;
-            }
-        }
-        else {
-            iter++;
-        }
-        if(iter) {
-            // make sure the iterator is valid...
-            eng::LibFile::Ptr libfile 
-                = (*iter)[m_imageliststore->columns().m_libfile];
-            selection = libfile->id();
-
-            fwk::AutoFlag f(m_in_handler);
-
-            using std::placeholders::_1;
-            std::for_each(m_selectables.begin(), m_selectables.end(),
-                          std::bind(&IImageSelectable::select_image, _1,
-                                      selection));
-            signal_selected(selection);
+    auto selection = get_selection();
+    auto iter = m_imageliststore->get_iter_from_id(selection);
+    if(!iter) {
+        return;
+    }
+
+    if(backwards) {
+        if(iter != m_imageliststore->children().begin()) {
+            --iter;
         }
     }
+    else {
+        iter++;
+    }
+    if(iter) {
+        // make sure the iterator is valid...
+        eng::LibFile::Ptr libfile
+            = (*iter)[m_imageliststore->columns().m_libfile];
+        selection = libfile->id();
+
+        fwk::AutoFlag f(m_in_handler);
+
+        std::for_each(m_selectables.begin(), m_selectables.end(),
+                      [selection] (const IImageSelectable::WeakPtr & s) {
+                          if (!s.expired()) {
+                              s.lock()->select_image(selection);
+                          }
+                      });
+        signal_selected(selection);
+    }
 }
 
 /** select the previous image. Emit the signal */
diff --git a/src/niepce/ui/selectioncontroller.hpp b/src/niepce/ui/selectioncontroller.hpp
index 0bd6c9e..df5d0c3 100644
--- a/src/niepce/ui/selectioncontroller.hpp
+++ b/src/niepce/ui/selectioncontroller.hpp
@@ -21,6 +21,8 @@
 #ifndef _UI_SELECTIONCONTROLLER_H__
 #define _UI_SELECTIONCONTROLLER_H__
 
+#include <memory>
+
 #include <gtk/gtk.h>
 
 #include <sigc++/signal.h>
@@ -43,6 +45,9 @@ namespace ui {
 class IImageSelectable
 {
 public:
+    typedef std::shared_ptr<IImageSelectable> Ptr;
+    typedef std::weak_ptr<IImageSelectable> WeakPtr;
+
     virtual ~IImageSelectable() {}
     virtual Gtk::IconView * image_list() = 0;
     /** Return the id of the selection. <= 0 is none. */
@@ -61,11 +66,11 @@ public:
     typedef std::shared_ptr<SelectionController> Ptr;
     SelectionController();
 
-    void add_selectable(IImageSelectable *);
+    void add_selectable(const IImageSelectable::WeakPtr &);
 
     void activated(const Gtk::TreeModel::Path & /*path*/,
-                   IImageSelectable * selectable);
-    void selected(IImageSelectable *);
+                   const IImageSelectable::WeakPtr & selectable);
+    void selected(const IImageSelectable::WeakPtr &);
 
 
     const Glib::RefPtr<ImageListStore> & get_list_store() const
@@ -125,7 +130,7 @@ private:
 
     Glib::RefPtr<ImageListStore>  m_imageliststore;
     bool m_in_handler;
-    std::vector<IImageSelectable *> m_selectables;
+    std::vector<IImageSelectable::WeakPtr> m_selectables;
 };
 
 }


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]