Merge lp:~mhr3/unity/home-lens-fixes into lp:unity

[Michal Hruby]

Activate proper result if the categories aren't displayed in-order

Fixed one longstanding and one recently introduced bug.

Fixes existing AP test (although depending on your data it might not have been failing in the first place).

[Sam Spilsbury (smspillaz)]

Looks good on style and usage. Don't know about functionality.

Are there any automated unit tests for this area?

[Michal Hruby (mhr3)]

As mentioned in the description there's AP test for one of the issues, the second one is more tricky as it requires killing and restarting a lens.

[Tim Penhey (thumper)]

Looks reasonable, minor niggles about * next to vars, but not types, but that is all.

review: Approve

[Paweł Stołowski (stolowski)]

Looks good and works fine. My only gripe is about implicit conversion of glib::Variant to bool

619 +Variant::operator bool() const
620 +{
621 + return bool(variant_);
622 +}

IMHO it should only be used it if it improves readability, whereas in this case, if you just look at the interface declaration, it's not apparent that it relies on the internal pointer not being null - plus one could expect bool operator to return a boolean value of the variant when it makes sense (i.e. what GetBool() does atm).

Would you mind changing this to something more verbose, e.g. bool Variant::IsNull(); also 'return variant_ == null' would be more clear.

I'm sorry for being picky, I'm now spoiled by other languages ;)

review: Needs Fixing

[Michal Hruby (mhr3)]

> Looks good and works fine. My only gripe is about implicit conversion of
> glib::Variant to bool
>
> 619 +Variant::operator bool() const
> 620 +{
> 621 + return bool(variant_);
> 622 +}
>
> IMHO it should only be used it if it improves readability, whereas in this
> case, if you just look at the interface declaration, it's not apparent that it
> relies on the internal pointer not being null - plus one could expect bool
> operator to return a boolean value of the variant when it makes sense (i.e.
> what GetBool() does atm).
>
> Would you mind changing this to something more verbose, e.g. bool
> Variant::IsNull(); also 'return variant_ == null' would be more clear.
>
> I'm sorry for being picky, I'm now spoiled by other languages ;)

I understand your point, but this is consistent with our other wrappers (glib::Object), plus you really shouldn't be using implicit conversion to get the value out of a variant, that sounds like a very bad idea. Moreover this also consistent with the way shared_ptr and unique_ptr work, so as long as you keep in mind this is a wrapper it's not that ambiguous.

[Paweł Stołowski (stolowski)]

I wasn't suggesting implicit conversion to get the value out of a variant, just said one could be tricked into thinking it does that by looking at the interface ;); that would indeed be a bad idea.

If it's consistent with other wrappers, then so be it, though that doesn't automatically make me happy :/

review: Approve

[Unity Merger (unity-merger)]

The Jenkins job https://jenkins.qa.ubuntu.com/job/automerge-unity/1151/console reported an error when processing this lp:~mhr3/unity/home-lens-fixes branch.
Not merging it.

[Unity Merger (unity-merger)]

The Jenkins job https://jenkins.qa.ubuntu.com/job/automerge-unity/1153/console reported an error when processing this lp:~mhr3/unity/home-lens-fixes branch.
Not merging it.

=== modified file 'UnityCore/HomeLens.cpp'
--- UnityCore/HomeLens.cpp	2012-08-17 15:59:41 +0000
+++ UnityCore/HomeLens.cpp	2012-08-29 09:48:20 +0000
@@ -136,6 +136,21 @@
     return target_cat_index;
   }
 
+  void UnregisterAllForModel(DeeModel* model)
+  {
+    auto it = reg_category_map_.begin();
+
+    // References and iterators to the erased elements are invalidated. 
+    // Other references and iterators are not affected.
+    while (it != reg_category_map_.end())
+    {
+      if (it->first.first == model)
+        reg_category_map_.erase(it++);
+      else
+        ++it;
+    }
+  }
+
   void NotifyOrderChanged ()
   {
     owner_->categories_reordered();
@@ -157,17 +172,18 @@
   ModelMerger(glib::Object<DeeModel> target);
   virtual ~ModelMerger();
 
-  void AddSource(Lens::Ptr& owner_lens, glib::Object<DeeModel> source);
+  virtual void AddSource(Lens::Ptr& owner_lens, glib::Object<DeeModel> source);
+  virtual void RemoveSource(glib::Object<DeeModel> const& old_source);
 
 protected:
-  virtual void OnSourceRowAdded(DeeModel *model, DeeModelIter *iter);
+  virtual void OnSourceRowAdded(DeeModel* model, DeeModelIter* iter);
   virtual void OnSourceRowRemoved(DeeModel* model, DeeModelIter* iter);
   virtual void OnSourceRowChanged(DeeModel* model, DeeModelIter* iter);
-  void EnsureRowBuf(DeeModel *source);
+  void EnsureRowBuf(DeeModel* source);
 
   /* The merge tag lives on the source models, pointing to the mapped
    * row in the target model */
-  DeeModelTag* FindSourceToTargetTag(DeeModel *model);
+  DeeModelTag* FindSourceToTargetTag(DeeModel* model);
 
 protected:
   std::map<Lens::Ptr, glib::Object<DeeModel> > sources_by_owner_;
@@ -197,9 +213,9 @@
                 HomeLens::CategoryRegistry* cat_registry);
 
 protected:
-  void OnSourceRowAdded(DeeModel *model, DeeModelIter *iter);
-  void OnSourceRowRemoved(DeeModel *model, DeeModelIter *iter);
-  void OnSourceRowChanged(DeeModel *model, DeeModelIter *iter);
+  void OnSourceRowAdded(DeeModel* model, DeeModelIter* iter);
+  void OnSourceRowRemoved(DeeModel* model, DeeModelIter* iter);
+  void OnSourceRowChanged(DeeModel* model, DeeModelIter* iter);
 
 private:
   HomeLens::CategoryRegistry* cat_registry_;
@@ -221,10 +237,13 @@
                  HomeLens::CategoryRegistry* cat_registry,
                  MergeMode merge_mode);
 
-  void OnSourceRowAdded(DeeModel *model, DeeModelIter *iter);
-  void OnSourceRowRemoved(DeeModel *model, DeeModelIter *iter);
-
-  std::vector<unsigned> GetOrder();
+  void OnSourceRowAdded(DeeModel* model, DeeModelIter* iter);
+  void OnSourceRowRemoved(DeeModel* model, DeeModelIter* iter);
+
+  std::vector<unsigned> GetDefaultOrder();
+
+protected:
+  void RemoveSource(glib::Object<DeeModel> const& old_source);
 
 private:
   HomeLens::CategoryRegistry* cat_registry_;
@@ -241,9 +260,9 @@
 public:
   FiltersMerger(glib::Object<DeeModel> target);
 
-  void OnSourceRowAdded(DeeModel *model, DeeModelIter *iter);
-  void OnSourceRowRemoved(DeeModel *model, DeeModelIter *iter);
-  void OnSourceRowChanged(DeeModel *model, DeeModelIter *iter);
+  void OnSourceRowAdded(DeeModel* model, DeeModelIter* iter);
+  void OnSourceRowRemoved(DeeModel* model, DeeModelIter* iter);
+  void OnSourceRowChanged(DeeModel* model, DeeModelIter* iter);
 };
 
 /*
@@ -260,6 +279,7 @@
   void EnsureCategoryAnnotation(Lens::Ptr& lens, DeeModel* results, DeeModel* categories);
   Lens::Ptr FindLensForUri(std::string const& uri);
   std::vector<unsigned> GetCategoriesOrder();
+  void LensSearchFinished(Lens::Ptr& lens);
 
   HomeLens* owner_;
   Lenses::LensList lenses_;
@@ -324,7 +344,7 @@
   {
     if (it->second == source)
       return; // this model was already added
-    sig_manager_.Disconnect(it->second);
+    RemoveSource(it->second);
   }
   sources_by_owner_[owner_lens] = source;
 
@@ -344,12 +364,23 @@
                                        sigc::mem_fun(this, &HomeLens::ModelMerger::OnSourceRowChanged)));
 }
 
-void HomeLens::ModelMerger::OnSourceRowAdded(DeeModel *model, DeeModelIter *iter)
+void HomeLens::ModelMerger::RemoveSource(glib::Object<DeeModel> const& source)
+{
+  if (!source)
+  {
+    LOG_ERROR(logger) << "Trying to remove NULL source from ModelMerger";
+    return;
+  }
+
+  sig_manager_.Disconnect(source);
+}
+
+void HomeLens::ModelMerger::OnSourceRowAdded(DeeModel* model, DeeModelIter* iter)
 {
   // Default impl. does nothing.
 }
 
-void HomeLens::ResultsMerger::OnSourceRowAdded(DeeModel *model, DeeModelIter *iter)
+void HomeLens::ResultsMerger::OnSourceRowAdded(DeeModel* model, DeeModelIter* iter)
 {
   DeeModelIter* target_iter;
   int target_cat_offset, source_cat_offset;
@@ -390,7 +421,7 @@
   for (unsigned int i = 0; i < n_cols_; i++) g_variant_unref(row_buf_[i]);
 }
 
-void HomeLens::CategoryMerger::OnSourceRowAdded(DeeModel *model, DeeModelIter *iter)
+void HomeLens::CategoryMerger::OnSourceRowAdded(DeeModel* model, DeeModelIter* iter)
 {
   DeeModel* results_model;
   DeeModelIter* target_iter;
@@ -470,14 +501,14 @@
   for (unsigned int i = 0; i < n_cols_; i++) g_variant_unref(row_buf_[i]);
 }
 
-void HomeLens::FiltersMerger::OnSourceRowAdded(DeeModel *model, DeeModelIter *iter)
+void HomeLens::FiltersMerger::OnSourceRowAdded(DeeModel* model, DeeModelIter* iter)
 {
   /* Supporting filters on the home screen is possible, but *quite* tricky.
    * So... Discard ALL the rows!
    */
 }
 
-void HomeLens::CategoryMerger::OnSourceRowRemoved(DeeModel *model, DeeModelIter *iter)
+void HomeLens::CategoryMerger::OnSourceRowRemoved(DeeModel* model, DeeModelIter* iter)
 {
   /* We don't support removals of categories.
    * You can check out any time you like, but you can never leave
@@ -488,7 +519,7 @@
   LOG_DEBUG(logger) << "Removal of categories not supported.";
 }
 
-void HomeLens::ModelMerger::OnSourceRowRemoved(DeeModel *model, DeeModelIter *iter)
+void HomeLens::ModelMerger::OnSourceRowRemoved(DeeModel* model, DeeModelIter* iter)
 {
   DeeModelIter* target_iter;
   DeeModelTag*  target_tag;
@@ -506,17 +537,17 @@
     dee_model_remove(target_, target_iter);
 }
 
-void HomeLens::ResultsMerger::OnSourceRowRemoved(DeeModel *model, DeeModelIter *iter)
+void HomeLens::ResultsMerger::OnSourceRowRemoved(DeeModel* model, DeeModelIter* iter)
 {
   ModelMerger::OnSourceRowRemoved(model, iter);
 }
 
-void HomeLens::FiltersMerger::OnSourceRowRemoved(DeeModel *model, DeeModelIter *iter)
+void HomeLens::FiltersMerger::OnSourceRowRemoved(DeeModel* model, DeeModelIter* iter)
 {
   /* We aren't adding any rows to the merged model, so nothing to do here */
 }
 
-void HomeLens::ModelMerger::OnSourceRowChanged(DeeModel *model, DeeModelIter *iter)
+void HomeLens::ModelMerger::OnSourceRowChanged(DeeModel* model, DeeModelIter* iter)
 {
   DeeModelIter* target_iter;
   DeeModelTag*  target_tag;
@@ -534,18 +565,18 @@
   for (unsigned int i = 0; i < n_cols_; i++) g_variant_unref(row_buf_[i]);
 }
 
-void HomeLens::ResultsMerger::OnSourceRowChanged(DeeModel *model, DeeModelIter *iter)
+void HomeLens::ResultsMerger::OnSourceRowChanged(DeeModel* model, DeeModelIter* iter)
 {
   // FIXME: We can support this, but we need to re-calculate the category offset
   LOG_WARN(logger) << "In-line changing of results not supported in the home lens. Sorry.";
 }
 
-void HomeLens::FiltersMerger::OnSourceRowChanged(DeeModel *model, DeeModelIter *iter)
+void HomeLens::FiltersMerger::OnSourceRowChanged(DeeModel* model, DeeModelIter* iter)
 {
   /* We aren't adding any rows to the merged model, so nothing to do here */
 }
 
-void HomeLens::ModelMerger::EnsureRowBuf(DeeModel *model)
+void HomeLens::ModelMerger::EnsureRowBuf(DeeModel* model)
 {
   if (G_UNLIKELY (n_cols_ == 0))
   {
@@ -606,12 +637,20 @@
   }
 }
 
-DeeModelTag* HomeLens::ModelMerger::FindSourceToTargetTag(DeeModel *model)
+DeeModelTag* HomeLens::ModelMerger::FindSourceToTargetTag(DeeModel* model)
 {
   return source_to_target_tags_[model];
 }
 
-std::vector<unsigned> HomeLens::CategoryMerger::GetOrder()
+void HomeLens::CategoryMerger::RemoveSource(glib::Object<DeeModel> const& source)
+{
+  // call base()
+  HomeLens::ModelMerger::RemoveSource(source);
+
+  cat_registry_->UnregisterAllForModel(source);
+}
+
+std::vector<unsigned> HomeLens::CategoryMerger::GetDefaultOrder()
 {
   std::vector<unsigned> result;
   for (auto it = category_ordering_.begin(); it != category_ordering_.end(); ++it)
@@ -746,6 +785,7 @@
   lens->global_search_finished.connect([&] (Hints const& hints) {
       running_searches_--;
 
+      LensSearchFinished(lens);
       if (running_searches_ <= 0)
       {
         owner_->search_finished.emit(Hints());
@@ -784,41 +824,15 @@
       filters_merger_.AddSource(lens, filters_prop());
   }
 
-  /*
-   * We'll assume that the models' swarm names do not change during life cycle
-   * of a lens.
-   * Otherwise we might run into a race where we would associate category
-   * model to a results model that is about to be replaced by a new one.
-   */
-  lens->connected.changed.connect([&] (bool is_connected)
-  {
-    if (is_connected)
-    {
-      EnsureCategoryAnnotation(lens, lens->categories()->model(),
-                               lens->global_results()->model());
-      categories_merger_.AddSource(lens, lens->categories()->model());
-      results_merger_.AddSource(lens, lens->global_results()->model());
-      filters_merger_.AddSource(lens, lens->filters()->model());
-    }
-  });
-  /*
-  results_prop.changed.connect([&] (glib::Object<DeeModel> model)
-  {
-    EnsureCategoryAnnotation(lens, lens->categories()->model(), model);
-    results_merger_.AddSource(model);
-  });
-
-  categories_prop.changed.connect([&] (glib::Object<DeeModel> model)
-  {
-    EnsureCategoryAnnotation(lens, model, lens->global_results()->model());
-    categories_merger_.AddSource(model);
-  });
-
-  filters_prop.changed.connect([&] (glib::Object<DeeModel> model)
-  {
-    filters_merger_.AddSource(model);
-  });
-  */
+  /* Make sure the models are properly annotated when they change */
+  lens->models_changed.connect([&] ()
+  {
+    EnsureCategoryAnnotation(lens, lens->categories()->model(),
+                             lens->global_results()->model());
+    categories_merger_.AddSource(lens, lens->categories()->model());
+    results_merger_.AddSource(lens, lens->global_results()->model());
+    filters_merger_.AddSource(lens, lens->filters()->model());
+  });
 
   /*
    * Register pre-existing categories up front
@@ -838,9 +852,13 @@
   }
 }
 
+void HomeLens::Impl::LensSearchFinished(Lens::Ptr& lens)
+{
+}
+
 std::vector<unsigned> HomeLens::Impl::GetCategoriesOrder()
 {
-  return categories_merger_.GetOrder();
+  return categories_merger_.GetDefaultOrder();
 }
 
 HomeLens::HomeLens(std::string const& name,

=== modified file 'UnityCore/Lens.cpp'
--- UnityCore/Lens.cpp	2012-08-15 21:47:27 +0000
+++ UnityCore/Lens.cpp	2012-08-29 09:48:20 +0000
@@ -36,6 +36,8 @@
 namespace
 {
 nux::logging::Logger logger("unity.dash.lens");
+
+const unsigned CATEGORY_COLUMN = 2;
 }
 
 using std::string;
@@ -91,6 +93,8 @@
                      glib::Variant const& preview_update,
                      glib::DBusProxy::ReplyCallback reply_cb);
   std::vector<unsigned> GetCategoriesOrder();
+  glib::Object<DeeModel> GetFilterModelForCategory(unsigned category);
+  void GetFilterForCategoryIndex(unsigned category, DeeFilter* filter);
 
   string const& id() const;
   string const& dbus_name() const;
@@ -109,6 +113,9 @@
   Filters::Ptr const& filters() const;
   bool connected() const;
 
+  string const& last_search_string() const { return last_search_string_; }
+  string const& last_global_search_string() const { return last_global_search_string_; }
+
   Lens* owner_;
 
   string id_;
@@ -128,14 +135,16 @@
   bool connected_;
 
   string private_connection_name_;
+  string last_search_string_;
+  string last_global_search_string_;
 
   glib::DBusProxy* proxy_;
   glib::Object<GCancellable> search_cancellable_;
   glib::Object<GCancellable> global_search_cancellable_;
   glib::Object<GCancellable> preview_cancellable_;
 
-  GVariant *results_variant_;
-  GVariant *global_results_variant_;
+  glib::Variant results_variant_;
+  glib::Variant global_results_variant_;
 };
 
 Lens::Impl::Impl(Lens* owner,
@@ -166,8 +175,6 @@
   , filters_(new Filters(model_type))
   , connected_(false)
   , proxy_(NULL)
-  , results_variant_(NULL)
-  , global_results_variant_(NULL)
 {
   if (model_type == ModelType::REMOTE)
   {
@@ -199,6 +206,8 @@
   owner_->categories.SetGetterFunction(sigc::mem_fun(this, &Lens::Impl::categories));
   owner_->filters.SetGetterFunction(sigc::mem_fun(this, &Lens::Impl::filters));
   owner_->connected.SetGetterFunction(sigc::mem_fun(this, &Lens::Impl::connected));
+  owner_->last_search_string.SetGetterFunction(sigc::mem_fun(this, &Lens::Impl::last_search_string));
+  owner_->last_global_search_string.SetGetterFunction(sigc::mem_fun(this, &Lens::Impl::last_global_search_string));
   owner_->view_type.changed.connect(sigc::mem_fun(this, &Lens::Impl::OnViewTypeChanged));
 }
 
@@ -236,13 +245,11 @@
 void Lens::Impl::ResultsModelUpdated(unsigned long long begin_seqnum,
                                      unsigned long long end_seqnum)
 {
-  if (results_variant_ != NULL &&
-      end_seqnum >= ExtractModelSeqnum (results_variant_))
+  if (results_variant_ && end_seqnum >= ExtractModelSeqnum (results_variant_))
   {
-    glib::Variant dict(results_variant_, glib::StealRef());
     Hints hints;
 
-    dict.ASVToHints(hints);
+    results_variant_.ASVToHints(hints);
 
     owner_->search_finished.emit(hints);
 
@@ -253,13 +260,12 @@
 void Lens::Impl::GlobalResultsModelUpdated(unsigned long long begin_seqnum,
                                            unsigned long long end_seqnum)
 {
-  if (global_results_variant_ != NULL &&
+  if (global_results_variant_ &&
       end_seqnum >= ExtractModelSeqnum (global_results_variant_))
   {
-    glib::Variant dict(global_results_variant_, glib::StealRef());
     Hints hints;
 
-    dict.ASVToHints(hints);
+    global_results_variant_.ASVToHints(hints);
 
     owner_->global_search_finished.emit(hints);
 
@@ -296,8 +302,7 @@
   if (results_->seqnum < reply_seqnum)
   {
     // wait for the end-transaction signal
-    if (results_variant_) g_variant_unref (results_variant_);
-    results_variant_ = g_variant_ref (parameters);
+    results_variant_ = parameters;
 
     // ResultsModelUpdated will emit OnSearchFinished
     return;
@@ -318,8 +323,7 @@
   if (global_results_->seqnum < reply_seqnum)
   {
     // wait for the end-transaction signal
-    if (global_results_variant_) g_variant_unref (global_results_variant_);
-    global_results_variant_ = g_variant_ref (parameters);
+    global_results_variant_ = parameters;
 
     // GlobalResultsModelUpdated will emit OnGlobalSearchFinished
     return;
@@ -367,15 +371,27 @@
                     << "  Filters: " << filters_model_name << "\n";
   if (dbus_path.Str() == dbus_path_)
   {
+    std::string categories_model_swarm_name(categories_model_name.Str());
+    std::string filters_model_swarm_name(filters_model_name.Str());
+    std::string results_model_swarm_name(results_model_name.Str());
+    std::string global_results_model_swarm_name(global_results_model_name.Str());
+    bool models_changed = 
+      categories_model_swarm_name != categories_->swarm_name ||
+      filters_model_swarm_name != filters_->swarm_name ||
+      results_model_swarm_name != results_->swarm_name ||
+      global_results_model_swarm_name != global_results_->swarm_name;
+
     /* FIXME: We ignore hints for now */
     UpdateProperties(search_in_global,
                      visible,
                      search_hint.Str(),
                      private_connection_name.Str(),
-                     results_model_name.Str(),
-                     global_results_model_name.Str(),
-                     categories_model_name.Str(),
-                     filters_model_name.Str());
+                     results_model_swarm_name,
+                     global_results_model_swarm_name,
+                     categories_model_swarm_name,
+                     filters_model_swarm_name);
+
+    if (models_changed) owner_->models_changed.emit();
   }
   else
   {
@@ -447,11 +463,8 @@
     g_cancellable_cancel (global_search_cancellable_);
   global_search_cancellable_ = g_cancellable_new ();
 
-  if (global_results_variant_)
-  {
-    g_variant_unref (global_results_variant_);
-    global_results_variant_ = NULL;
-  }
+  global_results_variant_ = NULL;
+  last_global_search_string_ = search_string;
 
   proxy_->Call("GlobalSearch",
               g_variant_new("(sa{sv})",
@@ -478,11 +491,8 @@
   if (search_cancellable_) g_cancellable_cancel (search_cancellable_);
   search_cancellable_ = g_cancellable_new ();
 
-  if (results_variant_)
-  {
-    g_variant_unref (results_variant_);
-    results_variant_ = NULL;
-  }
+  results_variant_ = NULL;
+  last_search_string_ = search_string;
 
   proxy_->Call("Search",
                g_variant_new("(sa{sv})",
@@ -621,6 +631,61 @@
   return result;
 }
 
+glib::Object<DeeModel> Lens::Impl::GetFilterModelForCategory(unsigned category)
+{
+  DeeFilter filter;
+  GetFilterForCategoryIndex(category, &filter);
+  glib::Object<DeeModel> filter_model(dee_filter_model_new(results_->model(), &filter));
+
+  return filter_model;
+}
+
+static void category_filter_map_func (DeeModel* orig_model,
+                                      DeeFilterModel* filter_model,
+                                      gpointer user_data)
+{
+  DeeModelIter* iter;
+  DeeModelIter* end;
+  unsigned index = GPOINTER_TO_UINT(user_data);
+
+  iter = dee_model_get_first_iter(orig_model);
+  end = dee_model_get_last_iter(orig_model);
+  while (iter != end)
+  {
+    unsigned category_index = dee_model_get_uint32(orig_model, iter,
+                                                   CATEGORY_COLUMN);
+    if (index == category_index)
+    {
+      dee_filter_model_append_iter(filter_model, iter);
+    }
+    iter = dee_model_next(orig_model, iter);
+  }
+}
+
+static gboolean category_filter_notify_func (DeeModel* orig_model,
+                                             DeeModelIter* orig_iter,
+                                             DeeFilterModel* filter_model,
+                                             gpointer user_data)
+{
+  unsigned index = GPOINTER_TO_UINT(user_data);
+  unsigned category_index = dee_model_get_uint32(orig_model, orig_iter,
+                                                 CATEGORY_COLUMN);
+
+  if (index != category_index)
+    return FALSE;
+
+  dee_filter_model_insert_iter_with_original_order(filter_model, orig_iter);
+  return TRUE;
+}
+
+void Lens::Impl::GetFilterForCategoryIndex(unsigned category, DeeFilter* filter)
+{
+  filter->map_func = category_filter_map_func;
+  filter->map_notify = category_filter_notify_func;
+  filter->destroy = nullptr;
+  filter->userdata = GUINT_TO_POINTER(category);
+}
+
 string const& Lens::Impl::id() const
 {
   return id_;
@@ -797,6 +862,11 @@
   return pimpl->GetCategoriesOrder();
 }
 
+glib::Object<DeeModel> Lens::GetFilterModelForCategory(unsigned category)
+{
+  return pimpl->GetFilterModelForCategory(category);
+}
+
 
 }
 }

=== modified file 'UnityCore/Lens.h'
--- UnityCore/Lens.h	2012-08-15 21:47:27 +0000
+++ UnityCore/Lens.h	2012-08-29 09:48:20 +0000
@@ -90,6 +90,13 @@
   virtual void SignalPreview(std::string const& uri,
       glib::Variant const& preview_update,
       glib::DBusProxy::ReplyCallback reply_cb = nullptr);
+
+  /**
+   * Note that this model is only valid for as long as the results model
+   * doesn't change.
+   * (you should call this again after models_changed is emitted)
+   */
+  virtual glib::Object<DeeModel> GetFilterModelForCategory(unsigned category);
   virtual std::vector<unsigned> GetCategoriesOrder();
 
   nux::RWProperty<std::string> id;
@@ -107,10 +114,15 @@
   nux::RWProperty<Categories::Ptr> categories;
   nux::RWProperty<Filters::Ptr> filters;
   nux::RWProperty<bool> connected;
+  nux::ROProperty<std::string> last_search_string;
+  nux::ROProperty<std::string> last_global_search_string;
 
   nux::Property<ViewType> view_type;
 
   sigc::signal<void> categories_reordered;
+  /* Emitted when any of the models' swarm name changes, but collates the name
+   * changes into a single signal emission (when all are changed) */
+  sigc::signal<void> models_changed;
   sigc::signal<void, Hints const&> search_finished;
   sigc::signal<void, Hints const&> global_search_finished;
   sigc::signal<void, std::string const&, HandledType, Hints const&> activated;

=== modified file 'UnityCore/Variant.cpp'
--- UnityCore/Variant.cpp	2012-06-01 14:01:29 +0000
+++ UnityCore/Variant.cpp	2012-08-29 09:48:20 +0000
@@ -97,8 +97,8 @@
 
 Variant& Variant::operator=(GVariant* val)
 {
-  if (variant_) g_variant_unref (variant_);
-  variant_ = g_variant_ref_sink (val);
+  if (variant_) g_variant_unref(variant_);
+  variant_ = val ? g_variant_ref_sink(val) : val;
 
   return *this;
 }
@@ -108,6 +108,11 @@
   return variant_;
 }
 
+Variant::operator bool() const
+{
+  return bool(variant_);
+}
+
 } // namespace glib
 
 namespace variant

=== modified file 'UnityCore/Variant.h'
--- UnityCore/Variant.h	2012-06-01 14:01:29 +0000
+++ UnityCore/Variant.h	2012-08-29 09:48:20 +0000
@@ -55,6 +55,8 @@
 
   Variant& operator=(GVariant*);
   operator GVariant*() const;
+  operator bool() const;
+
 private:
   GVariant* variant_;
 };

=== modified file 'dash/LensView.cpp'
--- dash/LensView.cpp	2012-08-28 12:23:15 +0000
+++ dash/LensView.cpp	2012-08-29 09:48:20 +0000
@@ -45,8 +45,6 @@
 
 const int CARD_VIEW_GAP_VERT  = 24; // pixels
 const int CARD_VIEW_GAP_HORIZ = 25; // pixels
-
-const unsigned CATEGORY_COLUMN = 2;
 }
 
 // This is so we can access some protected members in scrollview.
@@ -248,11 +246,8 @@
   {
     for (unsigned int i = 0; i < categories_.size(); ++i)
     {
-      PlacesGroup* group = categories_[i];
-      ResultViewGrid* grid = static_cast<ResultViewGrid*>(group->GetChildView());
-      DeeFilter filter;
-      GetFilterForCategoryIndex(i, &filter);
-      glib::Object<DeeModel> filter_model(dee_filter_model_new(model, &filter));
+      ResultViewGrid* grid = GetGridForCategory(i);
+      glib::Object<DeeModel> filter_model(lens_->GetFilterModelForCategory(i));
       Results::Ptr results_model = lens_->results;
       grid->SetModel(filter_model, results_model->GetTag());
     }
@@ -360,9 +355,7 @@
   Results::Ptr results_model = lens_->results;
   if (results_model->model())
   {
-    DeeFilter filter;
-    GetFilterForCategoryIndex(index, &filter);
-    glib::Object<DeeModel> filter_model(dee_filter_model_new(results_model->model(), &filter));
+    glib::Object<DeeModel> filter_model(lens_->GetFilterModelForCategory(index));
     grid->SetModel(filter_model, results_model->GetTag());
   }
 
@@ -422,62 +415,13 @@
   }
 }
 
-static void category_filter_map_func (DeeModel* orig_model,
-                                      DeeFilterModel* filter_model,
-                                      gpointer user_data)
-{
-  DeeModelIter* iter;
-  DeeModelIter* end;
-  unsigned index = GPOINTER_TO_UINT(user_data);
-
-  iter = dee_model_get_first_iter(orig_model);
-  end = dee_model_get_last_iter(orig_model);
-  while (iter != end)
-  {
-    unsigned category_index = dee_model_get_uint32(orig_model, iter,
-                                                   CATEGORY_COLUMN);
-    if (index == category_index)
-    {
-      dee_filter_model_append_iter(filter_model, iter);
-    }
-    iter = dee_model_next(orig_model, iter);
-  }
-}
-
-static gboolean category_filter_notify_func (DeeModel* orig_model,
-                                             DeeModelIter* orig_iter,
-                                             DeeFilterModel* filter_model,
-                                             gpointer user_data)
-{
-  unsigned index = GPOINTER_TO_UINT(user_data);
-  unsigned category_index = dee_model_get_uint32(orig_model, orig_iter,
-                                                 CATEGORY_COLUMN);
-
-  if (index != category_index)
-    return FALSE;
-
-  dee_filter_model_insert_iter_with_original_order(filter_model, orig_iter);
-  return TRUE;
-}
-
-void LensView::GetFilterForCategoryIndex(unsigned index, DeeFilter* filter)
-{
-  filter->map_func = category_filter_map_func;
-  filter->map_notify = category_filter_notify_func;
-  filter->destroy = nullptr;
-  filter->userdata = GUINT_TO_POINTER(index);
-}
-
 bool LensView::ReinitializeFilterModels()
 {
   Results::Ptr results_model = lens_->results;
   for (unsigned i = last_good_filter_model_ + 1; i < categories_.size(); ++i)
   {
-    PlacesGroup* group = categories_[i];
-    ResultViewGrid* grid = static_cast<ResultViewGrid*>(group->GetChildView());
-    DeeFilter filter;
-    GetFilterForCategoryIndex(i, &filter);
-    glib::Object<DeeModel> filter_model(dee_filter_model_new(results_model->model(), &filter));
+    ResultViewGrid* grid = GetGridForCategory(i);
+    glib::Object<DeeModel> filter_model(lens_->GetFilterModelForCategory(i));
     grid->SetModel(filter_model, results_model->GetTag());
   }
 
@@ -486,6 +430,13 @@
   return false;
 }
 
+ResultViewGrid* LensView::GetGridForCategory(unsigned category_index)
+{
+  if (category_index >= categories_.size()) return nullptr;
+  PlacesGroup* group = categories_.at(category_index);
+  return static_cast<ResultViewGrid*>(group->GetChildView());
+}
+
 void LensView::OnResultAdded(Result const& result)
 {
   try {
@@ -755,18 +706,20 @@
   Results::Ptr results = lens_->results;
   if (results->count())
   {
-    // FIXME: Linear search in the result model, use category grid's model!
-    for (unsigned int c = 0; c < scroll_layout_->GetChildren().size(); ++c)
+    // the first displayed category might not be categories_[0]
+    auto category_order = lens_->GetCategoriesOrder();
+    for (unsigned int i = 0; i < category_order.size(); i++)
     {
-      for (unsigned int i = 0; i < results->count(); ++i)
+      unsigned cat_index = category_order.at(i);
+      ResultViewGrid* grid = GetGridForCategory(cat_index);
+      if (grid == nullptr) continue;
+      auto it = grid->GetIteratorAtRow(0);
+      if (!it.IsLast())
       {
-        Result result = results->RowAtIndex(i);
-        if (result.category_index == c && result.uri != "")
-        {
-          uri_activated(result.uri);
-          lens_->Activate(result.uri);
-          return;
-        }
+        Result result(*it);
+        uri_activated(result.uri);
+        lens_->Activate(result.uri);
+        return;
       }
     }
     // Fallback

=== modified file 'dash/LensView.h'
--- dash/LensView.h	2012-08-28 12:23:15 +0000
+++ dash/LensView.h	2012-08-29 09:48:20 +0000
@@ -95,8 +95,7 @@
   void QueueFixRenderering();
   bool FixRenderering();
   bool ReinitializeFilterModels();
-
-  static void GetFilterForCategoryIndex(unsigned index, DeeFilter* filter);
+  ResultViewGrid* GetGridForCategory(unsigned category_index);
 
   void BuildPreview(std::string const& uri, Preview::Ptr model);