Merge lp:~3v1n0/unity/fix-load-icon-crash-926658 into lp:unity

[Marco Trevisan (Treviño)]

ResultRendererTile: don't crash in LoadIcon if row.renderer<TextureContainer*>() is null

In ResultRendererTile::LoadIcon we assumed that row.renderer<TextureContainer*>() is always not null; even if this should not happen because it has already just set into ResultRendererTile::Preload it looks like that there are cases that when doing row.set_renderer(new TextureContainer()); in the subsequent row.renderer<TextureContainer*>() returns null.
The implementation of that depends on dee_model_{set,get}_tag (it's basically like doing dee_model_set_tag and dee_model_get_tag), so it seems like that is failing. I've not been able to get a failure in stress tests, btw.

So, at this point, even if it would not the best solution, it's better to add a safety check to prevent crashes.

[Michal Hruby (mhr3)]

Sorry, but I don't like this one bit, dee_model_get_tag can crash if the passed model is not valid (already unreferenced). IMO guarding against null is not going to help, you'll just push the crash to a different place (in the better case, in worse you'll mask the underlying problem completely and there'll be even more random crashes).

review: Needs Fixing

[Marco Trevisan (Treviño) (3v1n0)]

Yes, that's true... But according to the stacktraces I got (attached to the bug), the crash is not happening because dee_model_get_tag returns an unreferenced pointer, but because it's returning a null one...

I'm wondering if that could happen that could be happen this:

ResultRendererTile::Preload()
ResultRendererTile::Unload()
ResultRendererTile::LoadIcon()

But it would be weird as all seems done in the same thread...

[jenkins (martin-mrazik+qa)]

PASSED: Continuous integration, rev:2540
http://jenkins.qa.ubuntu.com/job/unity-ci/108/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-ci/./distribution=quantal,flavor=amd64/108/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-ci/./distribution=quantal,flavor=i386/108/console

review: Approve (continuous-integration)

[Michal Hruby (mhr3)]

As you alone said in the comment, the renderer is set in Preload(), so the only explanation is that the model is already invalid at that point, but dee notices that and ignores the call (while logging a critical), doing dee_model_get_tag([invalid], tag) will then return NULL of course.

Other times dee doesn't notice that the model is invalid, and crashes inside itself.

[jenkins (martin-mrazik+qa)]

PASSED: Continuous integration, rev:2545
http://jenkins.qa.ubuntu.com/job/unity-ci/110/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-ci/./distribution=quantal,flavor=amd64/110/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-ci/./distribution=quantal,flavor=i386/110/console

review: Approve (continuous-integration)

[Marco Trevisan (Treviño) (3v1n0)]

Well, it's the same I thought, but I'm not getting that on the stacktrace...
However, protecting us from possible invalid pointers is something we should do in any case.

[jenkins (martin-mrazik+qa)]

PASSED: Continuous integration, rev:2547
http://jenkins.qa.ubuntu.com/job/unity-ci/113/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-ci/./distribution=quantal,flavor=amd64/113/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-ci/./distribution=quantal,flavor=i386/113/console

review: Approve (continuous-integration)

[Marco Trevisan (Treviño) (3v1n0)]

I've reffed the model in ModelRowAdaptor... As you said it's not the full solution, but it's something we can safely do anyway. Let me know if this is better for you.

[jenkins (martin-mrazik+qa)]

PASSED: Continuous integration, rev:2551
http://jenkins.qa.ubuntu.com/job/unity-ci/115/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-ci/./distribution=quantal,flavor=amd64/115/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-ci/./distribution=quantal,flavor=i386/115/console

review: Approve (continuous-integration)

[jenkins (martin-mrazik+qa)]

PASSED: Continuous integration, rev:2552
http://jenkins.qa.ubuntu.com/job/unity-ci/117/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-ci/./distribution=quantal,flavor=amd64/117/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-ci/./distribution=quantal,flavor=i386/117/console

review: Approve (continuous-integration)

[Michal Hruby (mhr3)]

> I've reffed the model in ModelRowAdaptor... As you said it's not the full
> solution, but it's something we can safely do anyway. Let me know if this is
> better for you.

As mentioned on IRC, I'd rather see a *single* weak ref on the model in LensView? than this. (since adding a reference for each model row isn't a good idea)

review: Needs Fixing

[jenkins (martin-mrazik+qa)]

FAILED: Continuous integration, rev:2556
http://jenkins.qa.ubuntu.com/job/unity-ci/130/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-ci/./distribution=quantal,flavor=amd64/130/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-ci/./distribution=quantal,flavor=i386/130/console

review: Needs Fixing (continuous-integration)

[Francis Ginther (fginther)]

Review was claimed by accident, please ignore.

review: Abstain

[Michal Hruby (mhr3)]

Discussed on IRC, but so make sure it doesn't get lost:

1) the model can change, you need to listen to changed signal
2) in the weak notify callback you're basically iterating over the model, which you can't since the model is already dead

review: Needs Fixing

=== modified file 'UnityCore/ModelRowAdaptor.h'
--- UnityCore/ModelRowAdaptor.h	2012-06-19 16:47:56 +0000
+++ UnityCore/ModelRowAdaptor.h	2012-08-10 12:37:21 +0000
@@ -21,8 +21,8 @@
 #define UNITY_MODEL_ROW_H
 
 #include <string>
-
 #include <dee.h>
+#include "GLibWrapper.h"
 
 namespace unity
 {
 
=== modified file 'dash/LensView.cpp'
--- dash/LensView.cpp	2012-06-18 02:57:23 +0000
+++ dash/LensView.cpp	2012-08-10 12:37:21 +0000
@@ -123,6 +123,7 @@
   , filters_expanded(false)
   , can_refine_search(false)
   , no_results_active_(false)
+  , results_model_(nullptr)
 {}
 
 LensView::LensView(Lens::Ptr lens, nux::Area* show_filters)
@@ -132,6 +133,7 @@
   , lens_(lens)
   , initial_activation_(true)
   , no_results_active_(false)
+  , results_model_(nullptr)
 {
   SetupViews(show_filters);
   SetupCategories();
@@ -175,6 +177,14 @@
 
 }
 
+LensView::~LensView()
+{
+  if (DEE_IS_MODEL(results_model_))
+  {
+    g_object_weak_unref(G_OBJECT(results_model_), (GWeakNotify)LensView::OnModelDestroyed, this);
+  }
+}
+
 void LensView::SetupViews(nux::Area* show_filters)
 {
   dash::Style& style = dash::Style::Instance();
@@ -231,12 +241,39 @@
     OnCategoryAdded(categories->RowAtIndex(i));
 }
 
+void LensView::OnModelDestroyed(LensView* self, DeeModel* was_here)
+{
+  for (auto group : self->categories_)
+  {
+    auto grid = static_cast<ResultViewGrid*>(group->GetChildView());
+    self->counts_[group] -= grid->GetResultList().size();
+    grid->ClearResults();
+    self->UpdateCounts(group);
+  }
+}
+
 void LensView::SetupResults()
 {
   Results::Ptr results = lens_->results;
   results->result_added.connect(sigc::mem_fun(this, &LensView::OnResultAdded));
   results->result_removed.connect(sigc::mem_fun(this, &LensView::OnResultRemoved));
 
+  results->model.changed.connect([this] (glib::Object<DeeModel> model)
+  {
+    if (DEE_IS_MODEL(results_model_))
+    {
+      g_object_weak_unref(G_OBJECT(results_model_), (GWeakNotify)LensView::OnModelDestroyed, this);
+    }
+
+    results_model_ = nullptr;
+
+    if (model.IsType(DEE_TYPE_MODEL))
+    {
+      results_model_ = model;
+      g_object_weak_ref(glib::object_cast<GObject>(model), (GWeakNotify)LensView::OnModelDestroyed, this);
+    }
+  });
+
   for (unsigned int i = 0; i < results->count(); ++i)
     OnResultAdded(results->RowAtIndex(i));
 }
@@ -285,7 +322,7 @@
     grid->SetModelRenderer(new ResultRendererHorizontalTile(NUX_TRACKER_LOCATION));
     grid->horizontal_spacing = CARD_VIEW_GAP_HORIZ;
     grid->vertical_spacing = CARD_VIEW_GAP_VERT;
-  }    
+  }
   else
     grid->SetModelRenderer(new ResultRendererTile(NUX_TRACKER_LOCATION));
 
@@ -305,8 +342,7 @@
     PlacesGroup* group = categories_.at(result.category_index);
     ResultViewGrid* grid = static_cast<ResultViewGrid*>(group->GetChildView());
 
-    std::string uri = result.uri;
-    LOG_TRACE(logger) << "Result added: " << uri;
+    LOG_TRACE(logger) << "Result added: " << result.uri();
 
     grid->AddResult(const_cast<Result&>(result));
     counts_[group]++;
@@ -329,8 +365,7 @@
     PlacesGroup* group = categories_.at(result.category_index);
     ResultViewGrid* grid = static_cast<ResultViewGrid*>(group->GetChildView());
 
-    std::string uri = result.uri;
-    LOG_TRACE(logger) << "Result removed: " << uri;
+    LOG_TRACE(logger) << "Result removed: " << result.uri();
 
     grid->RemoveResult(const_cast<Result&>(result));
     counts_[group]--;
 
=== modified file 'dash/LensView.h'
--- dash/LensView.h	2012-06-18 02:57:23 +0000
+++ dash/LensView.h	2012-08-10 12:37:21 +0000
@@ -52,6 +52,7 @@
 public:
   LensView();
   LensView(Lens::Ptr lens, nux::Area* show_filters);
+  ~LensView();
 
   CategoryGroups& categories() { return categories_; }
   FilterBar* filter_bar() const { return filter_bar_; }
@@ -101,6 +102,8 @@
 
   std::string get_search_string() const;
 
+  static void OnModelDestroyed(LensView* self, DeeModel* was_here);
+
 private:
   Lens::Ptr lens_;
   CategoryGroups categories_;
@@ -116,6 +119,7 @@
   nux::VLayout* fscroll_layout_;
   FilterBar* filter_bar_;
   nux::StaticCairoText* no_results_;
+  DeeModel* results_model_;
 
   UBusManager ubus_manager_;
   glib::Source::UniquePtr fix_rendering_idle_;
 
=== modified file 'dash/ResultRendererTile.cpp'
--- dash/ResultRendererTile.cpp	2012-06-18 02:57:23 +0000
+++ dash/ResultRendererTile.cpp	2012-08-10 12:37:21 +0000
@@ -21,14 +21,10 @@
  */
 
 
-#include <sstream>     // for ostringstream
 #include "ResultRendererTile.h"
 
-#include <boost/algorithm/string.hpp>
-
 #include <pango/pango.h>
 #include <pango/pangocairo.h>
-#include <gdk/gdk.h>
 #include <gtk/gtk.h>
 
 #include <NuxCore/Logger.h>
@@ -40,19 +36,18 @@
 #include "unity-shared/IconTexture.h"
 #include "unity-shared/TextureCache.h"
 
-
-namespace
-{
-  bool neko;
-}
-
 namespace unity
 {
 namespace
 {
 nux::logging::Logger logger("unity.dash.results");
+const std::string DEFAULT_GICON = ". GThemedIcon text-x-preview";
 
 const int FONT_SIZE = 10;
+bool neko = false;
+const std::string neko_env = {0x55, 0x4e, 0x49, 0x54, 0x59, 0x5f, 0x4e, 0x45, 0x4b, 0x4f};
+const std::string neko_src = {0x68, 0x74, 0x74, 0x70, 0x3a, 0x2f, 0x2f, 0x70, 0x6c, 0x61, 0x63,
+                              0x65, 0x6b, 0x69, 0x74, 0x74, 0x65, 0x6e, 0x2e, 0x63, 0x6f, 0x6d};
 }
 
 namespace dash
@@ -69,11 +64,7 @@
   dash::Style& style = dash::Style::Instance();
   width = style.GetTileWidth();
   height = style.GetTileHeight();
-
-  gsize tmp;
-  gchar* tmp1 = (gchar*)g_base64_decode("VU5JVFlfTkVLTw==", &tmp);
-  neko = (g_getenv(tmp1));
-  g_free (tmp1);
+  neko = g_getenv(neko_env.c_str());
 
   // pre-load the highlight texture
   // try and get a texture from the texture cache
@@ -84,10 +75,6 @@
                                       sigc::mem_fun(this, &ResultRendererTile::DrawHighlight));
 }
 
-ResultRendererTile::~ResultRendererTile()
-{
-}
-
 void ResultRendererTile::Render(nux::GraphicsEngine& GfxContext,
                                 Result& row,
                                 ResultRendererState state,
@@ -95,7 +82,8 @@
                                 int x_offset, int y_offset)
 {
   TextureContainer* container = row.renderer<TextureContainer*>();
-  if (container == nullptr)
+
+  if (!container)
     return;
 
   dash::Style& style = dash::Style::Instance();
@@ -105,7 +93,7 @@
   nux::TexCoordXForm texxform;
 
   int icon_width, icon_height;
-  if (container->icon == nullptr)
+  if (!container->icon)
   {
     icon_width = icon_height = tile_icon_size;
   }
@@ -230,7 +218,7 @@
 
 void ResultRendererTile::Preload(Result& row)
 {
-  if (row.renderer<TextureContainer*>() == nullptr)
+  if (!row.renderer<TextureContainer*>())
   {
     row.set_renderer(new TextureContainer());
     LoadIcon(row);
@@ -240,52 +228,53 @@
 
 void ResultRendererTile::Unload(Result& row)
 {
-  TextureContainer *container = row.renderer<TextureContainer*>();
-  delete container;
+  delete row.renderer<TextureContainer*>();
   row.set_renderer<TextureContainer*>(nullptr);
 }
 
 void ResultRendererTile::LoadIcon(Result& row)
 {
-  Style& style = Style::Instance();
-  std::string const& icon_hint = row.icon_hint;
-#define DEFAULT_GICON ". GThemedIcon text-x-preview"
-  std::string icon_name;
-  if (G_UNLIKELY(neko))
-  {
-    int tmp1 = style.GetTileIconSize() + (rand() % 16) - 8;
-    gsize tmp3;
-    gchar* tmp2 = (gchar*)g_base64_decode("aHR0cDovL3BsYWNla2l0dGVuLmNvbS8laS8laS8=", &tmp3);
-    gchar* tmp4 = g_strdup_printf(tmp2, tmp1, tmp1);
-    icon_name = tmp4;
-    g_free(tmp4);
-    g_free(tmp2);
-  }
-  else
-  {
-    icon_name = !icon_hint.empty() ? icon_hint : DEFAULT_GICON;
-  }
-
-  GIcon*  icon = g_icon_new_for_string(icon_name.c_str(), NULL);
-  TextureContainer* container = row.renderer<TextureContainer*>();
-
-  IconLoader::IconLoaderCallback slot = sigc::bind(sigc::mem_fun(this, &ResultRendererTile::IconLoaded), icon_hint, row);
-
-  if (g_strrstr(icon_name.c_str(), "://"))
-  {
-    container->slot_handle = IconLoader::GetDefault().LoadFromURI(icon_name, style.GetTileIconSize(), slot);
-  }
-  else if (G_IS_ICON(icon))
-  {
-    container->slot_handle = IconLoader::GetDefault().LoadFromGIconString(icon_name, style.GetTileIconSize(), slot);
-  }
-  else
-  {
-    container->slot_handle = IconLoader::GetDefault().LoadFromIconName(icon_name, style.GetTileIconSize(), slot);
-  }
-
-  if (icon != NULL)
-    g_object_unref(icon);
+  auto container = row.renderer<TextureContainer*>();
+
+  if (!container)
+  {
+    LOG_ERROR(logger) << "No valid container for Result " << row.name() << " with URI "
+                      << row.uri();
+    return;
+  }
+
+  int icon_size = Style::Instance().GetTileIconSize();
+  std::string icon_name = row.icon_hint();
+
+  if (neko)
+  {
+    auto tmp = std::to_string(icon_size + (rand() % 16) - 8);
+    icon_name = neko_src + '/' + tmp + '/' + tmp;
+  }
+  else if (icon_name.empty())
+  {
+    icon_name = DEFAULT_GICON;
+  }
+
+  auto slot = sigc::bind(sigc::mem_fun(this, &ResultRendererTile::IconLoaded), row);
+
+  if (icon_name.find("://") != std::string::npos)
+  {
+    container->slot_handle = IconLoader::GetDefault().LoadFromURI(icon_name, icon_size, slot);
+  }
+  else
+  {
+    glib::Object<GIcon> icon(g_icon_new_for_string(icon_name.c_str(), nullptr));
+
+    if (icon.IsType(G_TYPE_ICON))
+    {
+      container->slot_handle = IconLoader::GetDefault().LoadFromGIconString(icon_name, icon_size, slot);
+    }
+    else
+    {
+      container->slot_handle = IconLoader::GetDefault().LoadFromIconName(icon_name, icon_size, slot);
+    }
+  }
 }
 
 nux::BaseTexture* ResultRendererTile::CreateTextureCallback(std::string const& texid,
@@ -405,20 +394,20 @@
 void ResultRendererTile::IconLoaded(std::string const& texid,
                                     unsigned size,
                                     glib::Object<GdkPixbuf> const& pixbuf,
-                                    std::string icon_name,
                                     Result& row)
 {
   TextureContainer *container = row.renderer<TextureContainer*>();
-  Style& style = Style::Instance();
+  int icon_size = Style::Instance().GetTileIconSize();
 
   if (pixbuf && container)
   {
+    std::string const& icon_name = row.icon_hint();
     TextureCache& cache = TextureCache::GetDefault();
-    BaseTexturePtr texture(cache.FindTexture(icon_name, style.GetTileIconSize(), style.GetTileIconSize(),
+    BaseTexturePtr texture(cache.FindTexture(icon_name, icon_size, icon_size,
                            sigc::bind(sigc::mem_fun(this, &ResultRendererTile::CreateTextureCallback), pixbuf)));
 
     std::string blur_texid = icon_name + "_blurred";
-    BaseTexturePtr texture_blurred(cache.FindTexture(blur_texid, style.GetTileIconSize(), style.GetTileIconSize(),
+    BaseTexturePtr texture_blurred(cache.FindTexture(blur_texid, icon_size, icon_size,
                                    sigc::bind(sigc::mem_fun(this, &ResultRendererTile::CreateBlurredTextureCallback), pixbuf)));
 
     BaseTexturePtr texture_prelight(cache.FindTexture("resultview_prelight", texture->GetWidth() + highlight_padding*2, texture->GetHeight() + highlight_padding*2,  sigc::mem_fun(this, &ResultRendererTile::DrawHighlight)));
@@ -426,17 +415,15 @@
     container->icon = texture;
     container->blurred_icon = texture_blurred;
     container->prelight = texture_prelight;
+    container->slot_handle = 0;
 
     NeedsRedraw.emit();
-
-    if (container)
-      container->slot_handle = 0;
   }
   else if (container)
   {
     // we need to load a missing icon
-    IconLoader::IconLoaderCallback slot = sigc::bind(sigc::mem_fun(this, &ResultRendererTile::IconLoaded), icon_name, row);
-    container->slot_handle = IconLoader::GetDefault().LoadFromGIconString(". GThemedIcon text-x-preview", style.GetTileIconSize(), slot);
+    IconLoader::IconLoaderCallback slot = sigc::bind(sigc::mem_fun(this, &ResultRendererTile::IconLoaded), row);
+    container->slot_handle = IconLoader::GetDefault().LoadFromGIconString(DEFAULT_GICON, icon_size, slot);
   }
 
 }
@@ -444,29 +431,31 @@
 
 void ResultRendererTile::LoadText(Result& row)
 {
+  TextureContainer *container = row.renderer<TextureContainer*>();
+
+  if (!container)
+    return;
+
+  GdkScreen* screen = gdk_screen_get_default();
   Style& style = Style::Instance();
   nux::CairoGraphics _cairoGraphics(CAIRO_FORMAT_ARGB32,
                                     style.GetTileWidth() - (padding * 2),
                                     style.GetTileHeight() - style.GetTileIconSize() - spacing);
 
+  glib::String font;
+  g_object_get(gtk_settings_get_default(), "gtk-font-name", &font, nullptr);
+
+  int dpi = -1;
+  g_object_get(gtk_settings_get_default(), "gtk-xft-dpi", &dpi, nullptr);
+
   cairo_t* cr = _cairoGraphics.GetContext();
-
-  PangoLayout*          layout     = NULL;
-  PangoFontDescription* desc       = NULL;
-  PangoContext*         pango_context   = NULL;
-  GdkScreen*            screen     = gdk_screen_get_default();    // not ref'ed
-  glib::String          font;
-  int                   dpi = -1;
-
-  g_object_get(gtk_settings_get_default(), "gtk-font-name", &font, NULL);
-  g_object_get(gtk_settings_get_default(), "gtk-xft-dpi", &dpi, NULL);
-
   cairo_set_font_options(cr, gdk_screen_get_font_options(screen));
-  layout = pango_cairo_create_layout(cr);
-  desc = pango_font_description_from_string(font.Value());
-  pango_font_description_set_size (desc, FONT_SIZE * PANGO_SCALE);
+  glib::Object<PangoLayout> layout(pango_cairo_create_layout(cr));
+  std::shared_ptr<PangoFontDescription> desc(pango_font_description_from_string(font),
+                                             pango_font_description_free);
+  pango_font_description_set_size (desc.get(), FONT_SIZE * PANGO_SCALE);
 
-  pango_layout_set_font_description(layout, desc);
+  pango_layout_set_font_description(layout, desc.get());
   pango_layout_set_alignment(layout, PANGO_ALIGN_CENTER);
 
   pango_layout_set_wrap(layout, PANGO_WRAP_WORD_CHAR);
@@ -474,17 +463,14 @@
   pango_layout_set_width(layout, (style.GetTileWidth() - (padding * 2))* PANGO_SCALE);
   pango_layout_set_height(layout, -2);
 
-  char *escaped_text = g_markup_escape_text(row.name().c_str()  , -1);
-
+  glib::String escaped_text(g_markup_escape_text(row.name().c_str(), -1));
   pango_layout_set_markup(layout, escaped_text, -1);
 
-  g_free (escaped_text);
-
-  pango_context = pango_layout_get_context(layout);  // is not ref'ed
+  PangoContext* pango_context = pango_layout_get_context(layout);  // is not ref'ed
   pango_cairo_context_set_font_options(pango_context,
                                        gdk_screen_get_font_options(screen));
   pango_cairo_context_set_resolution(pango_context,
-                                     dpi == -1 ? 96.0f : dpi/(float) PANGO_SCALE);
+                                     dpi == -1 ? 96.0f : dpi/static_cast<float>(PANGO_SCALE));
   pango_layout_context_changed(layout);
 
   cairo_set_operator(cr, CAIRO_OPERATOR_CLEAR);
@@ -497,13 +483,9 @@
   pango_cairo_show_layout(cr, layout);
 
   // clean up
-  pango_font_description_free(desc);
-  g_object_unref(layout);
   cairo_destroy(cr);
 
-  TextureContainer *container = row.renderer<TextureContainer*>();
-  if (container)
-    container->text = texture_ptr_from_cairo_graphics(_cairoGraphics);
+  container->text = texture_ptr_from_cairo_graphics(_cairoGraphics);
 }
 
 
 
=== modified file 'dash/ResultRendererTile.h'
--- dash/ResultRendererTile.h	2012-07-04 02:37:23 +0000
+++ dash/ResultRendererTile.h	2012-08-10 12:37:21 +0000
@@ -67,7 +67,6 @@
   NUX_DECLARE_OBJECT_TYPE(ResultRendererTile, ResultRenderer);
 
   ResultRendererTile(NUX_FILE_LINE_PROTO);
-  ~ResultRendererTile();
 
   virtual void Render(nux::GraphicsEngine& GfxContext,
                       Result& row,
@@ -93,7 +92,7 @@
   //icon loading callbacks
   void IconLoaded(std::string const& texid, unsigned size,
                   glib::Object<GdkPixbuf> const& pixbuf,
-                  std::string icon_name, Result& row);
+                  Result& row);
   nux::BaseTexture* CreateTextureCallback(std::string const& texid,
                                           int width, int height,
                                           glib::Object<GdkPixbuf> const& pixbuf);
 
=== modified file 'dash/ResultView.cpp'
--- dash/ResultView.cpp	2012-07-30 16:22:53 +0000
+++ dash/ResultView.cpp	2012-08-10 12:37:21 +0000
@@ -24,9 +24,7 @@
 #include "ResultView.h"
 #include "unity-shared/IntrospectableWrappers.h"
 #include <UnityCore/Variant.h>
-#include <Nux/HLayout.h>
-#include <Nux/VLayout.h>
-#include <Nux/Button.h>
+#include <Nux/Layout.h>
 #include <NuxCore/Logger.h>
 
 namespace unity
@@ -43,12 +41,10 @@
 ResultView::ResultView(NUX_FILE_LINE_DECL)
   : View(NUX_FILE_LINE_PARAM)
   , expanded(true)
-  , renderer_(NULL)
 {
   expanded.changed.connect([&](bool value)
   {
     QueueRelayout();
-    NeedRedraw();
   });
 }
 
@@ -60,28 +56,17 @@
   {
     renderer_->Unload(result);
   }
-
-  renderer_->UnReference();
 }
 
 void ResultView::Draw(nux::GraphicsEngine& GfxContext, bool force_draw)
-{
-
-}
+{}
 
 void ResultView::SetModelRenderer(ResultRenderer* renderer)
 {
-  if (renderer_ != NULL)
-    renderer_->UnReference();
-
   renderer_ = renderer;
-  renderer->NeedsRedraw.connect([&]()
-  {
-    NeedRedraw();
-  });
-  renderer_->SinkReference();
+  renderer_->NeedsRedraw.connect(sigc::mem_fun(this, &ResultView::QueueDraw));
 
-  NeedRedraw();
+  QueueDraw();
 }
 
 void ResultView::AddResult(Result& result)
@@ -89,40 +74,44 @@
   results_.push_back(result);
   renderer_->Preload(result);
 
-  NeedRedraw();
+  QueueDraw();
 }
 
 void ResultView::RemoveResult(Result& result)
 {
-  ResultList::iterator it;
-  std::string uri = result.uri;
+  std::string const& uri = result.uri;
 
-  for (it = results_.begin(); it != results_.end(); ++it)
+  for (auto it = results_.begin(); it != results_.end(); ++it)
   {
-    if (result.uri == (*it).uri)
+    if (it->uri == uri)
     {
       results_.erase(it);
       break;
     }
   }
+
   renderer_->Unload(result);
 }
 
-ResultView::ResultList ResultView::GetResultList()
+void ResultView::ClearResults()
+{
+  for (Result& result : results_)
+  {
+    renderer_->Unload(result);
+  }
+
+  results_.clear();
+}
+
+ResultView::ResultList const& ResultView::GetResultList() const
 {
   return results_;
 }
 
 
-long ResultView::ComputeContentSize()
-{
-  return View::ComputeContentSize();
-}
-
-
 void ResultView::DrawContent(nux::GraphicsEngine& GfxContent, bool force_draw)
 {
-  nux::Geometry base = GetGeometry();
+  nux::Geometry const& base = GetGeometry();
   GfxContent.PushClippingRectangle(base);
 
   if (GetCompositionLayout())
 
=== modified file 'dash/ResultView.h'
--- dash/ResultView.h	2012-07-04 02:37:23 +0000
+++ dash/ResultView.h	2012-08-10 12:37:21 +0000
@@ -25,9 +25,6 @@
 
 #include <Nux/Nux.h>
 #include <Nux/View.h>
-#include <dee.h>
-
-#include <UnityCore/GLibSignal.h>
 #include <UnityCore/Results.h>
 
 #include "unity-shared/Introspectable.h"
@@ -51,8 +48,9 @@
 
   void AddResult(Result& result);
   void RemoveResult(Result& result);
+  void ClearResults();
 
-  ResultList GetResultList ();
+  ResultList const& GetResultList() const;
 
   nux::Property<bool> expanded;
   nux::Property<int> results_per_row;
@@ -66,10 +64,9 @@
 protected:
   virtual void Draw(nux::GraphicsEngine& GfxContext, bool force_draw);
   virtual void DrawContent(nux::GraphicsEngine& GfxContext, bool force_draw);
-  virtual long ComputeContentSize();
 
   // properties
-  ResultRenderer* renderer_;
+  nux::ObjectPtr<ResultRenderer> renderer_;
   ResultList results_;
   IntrospectableList introspectable_children_;