Merge lp:~mhr3/libunity/multiple-fixes into lp:libunity

[Michal Hruby]

Apps scope port-related fixes

Apps scope port-related fixes:
1) fix ScopeDBusConnector incorrectly unexporting unused scopes
2) try to coalesce transactions when talking to multiple scopes
3) a small behaviour change to sorting

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:271
http://jenkins.qa.ubuntu.com/job/libunity-ci/89/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/libunity-saucy-amd64-ci/74
    SUCCESS: http://jenkins.qa.ubuntu.com/job/libunity-saucy-armhf-ci/74
    SUCCESS: http://jenkins.qa.ubuntu.com/job/libunity-saucy-i386-ci/74

Click here to trigger a rebuild:
http://s-jenkins:8080/job/libunity-ci/89/rebuild

review: Approve (continuous-integration)

[James Henstridge (jamesh)]

Looks good. At first I was a little confused by the use of multiplier for null string properties, but it made sense after reading the comment again.

review: Approve

=== modified file 'src/unity-aggregator-scope-private.vala'
--- src/unity-aggregator-scope-private.vala	2013-07-25 10:21:46 +0000
+++ src/unity-aggregator-scope-private.vala	2013-08-04 13:41:25 +0000
@@ -159,8 +159,6 @@
         dummy2 = var2.lookup_value ("content", VariantType.VARDICT).lookup_value (field_name, null);
         // handle non-presence of optional fields
         if (dummy1 == null && dummy2 == null) return 0;
-        if (dummy1 == null || dummy2 == null)
-          return dummy1 == null ? 1 : -1;
         var1 = dummy1;
         var2 = dummy2;
       }
@@ -168,23 +166,40 @@
       switch (schema)
       {
         case 's':
-          // FIXME: implement a way to use simple ascii sort
-          result = var1.get_string ().collate (var2.get_string ());
+          // strings are special, if optional field is not present,
+          // bury the result; numeric types behave as if 0 was set
+          if (var1 == null) result = 1 * multiplier;
+          else if (var2 == null) result = -1 * multiplier;
+          else
+          {
+            // FIXME: implement a way to use simple ascii sort
+            result = var1.get_string ().collate (var2.get_string ());
+          }
           break;
         case 'i':
-          result = compare_int (var1.get_int32 (), var2.get_int32 ());
+          int i1 = var1 != null ? var1.get_int32 () : 0;
+          int i2 = var2 != null ? var2.get_int32 () : 0;
+          result = compare_int (i1, i2);
           break;
         case 'u':
-          result = compare_uint (var1.get_uint32 (), var2.get_uint32 ());
+          uint u1 = var1 != null ? var1.get_uint32 () : 0;
+          uint u2 = var2 != null ? var2.get_uint32 () : 0;
+          result = compare_uint (u1, u2);
           break;
         case 'x':
-          result = compare_int64 (var1.get_int64 (), var2.get_int64 ());
+          int64 x1 = var1 != null ? var1.get_int64 () : 0;
+          int64 x2 = var2 != null ? var2.get_int64 () : 0;
+          result = compare_int64 (x1, x2);
           break;
         case 't':
-          result = compare_uint64 (var1.get_uint64 (), var2.get_uint64 ());
+          uint64 t1 = var1 != null ? var1.get_uint64 () : 0;
+          uint64 t2 = var2 != null ? var2.get_uint64 () : 0;
+          result = compare_uint64 (t1, t2);
           break;
         case 'd':
-          result = compare_double (var1.get_double (), var2.get_double ());
+          double d1 = var1 != null ? var1.get_double () : 0.0;
+          double d2 = var2 != null ? var2.get_double () : 0.0;
+          result = compare_double (d1, d2);
           break;
         default:
           return 0;

=== modified file 'src/unity-master-scope.vala'
--- src/unity-master-scope.vala	2013-05-01 22:30:59 +0000
+++ src/unity-master-scope.vala	2013-08-04 13:41:25 +0000
@@ -24,10 +24,13 @@
 
 public class MasterScope : AggregatorScope
 {
+  const int COALESCE_TIME_MS = 150;
+
   public MasterScope (string dbus_path_, string id_)
   {
     Object (dbus_path: dbus_path_, id: id_, is_master: true,
-            merge_mode: AggregatorScope.MergeMode.CATEGORY_ID, proxy_filter_hints: true);
+            merge_mode: AggregatorScope.MergeMode.CATEGORY_ID,
+            proxy_filter_hints: true, automatic_flushing: false);
   }
 
   public string no_content_hint { get; set; }
@@ -94,7 +97,9 @@
     var registry = registry_once.get_data ();
     if (registry == null) return;
 
-    uint num_scopes = 0;
+    uint total_searches = 0;
+    uint running_searches = 0;
+    uint timer_source_id = 0;
 
     AsyncReadyCallback cb = (obj, res) =>
     {
@@ -108,7 +113,7 @@
         if (!(err is IOError.CANCELLED))
           warning ("Unable to search scope: '%s'", err.message);
       }
-      if (--num_scopes == 0) search.callback ();
+      if (--running_searches == 0) search.callback ();
     };
 
     string[] search_subscopes = {};
@@ -119,6 +124,31 @@
       search_subscopes = (string[]) filter_variant;
     }
 
+    ulong sig_id;
+    sig_id = scope_search.transaction_complete.connect ((agg_scope_search, scope_id) =>
+    {
+      // if we're searching multiple scopes, try to coalesce the changes,
+      // otherwise flush the changes immediately
+      if (total_searches <= 1)
+      {
+        agg_scope_search.search_context.result_set.flush ();
+      }
+      else
+      {
+        // TODO: do this only if this channel is "visible" (opened by the dash)
+        if (timer_source_id != 0) return;
+        timer_source_id = Timeout.add (COALESCE_TIME_MS, () =>
+        {
+          if (!agg_scope_search.search_context.cancellable.is_cancelled ())
+          {
+            agg_scope_search.search_context.result_set.flush ();
+          }
+          timer_source_id = 0;
+          return false;
+        });
+      }
+    });
+
     foreach (var scope_node in registry.scopes)
     {
       if (search_subscopes.length > 0 && !(scope_node.scope_info.id in search_subscopes))
@@ -126,7 +156,8 @@
 
       if (metadata_matches (scope_node.scope_info))
       {
-        num_scopes++;
+        total_searches++;
+        running_searches++;
         scope_search.search_scope.begin (scope_node.scope_info.id,
                                          scope_search.search_string,
                                          scope_search.search_type,
@@ -139,7 +170,14 @@
       }
     }
 
-    if (num_scopes > 0) yield;
+    if (running_searches > 0) yield;
+
+    SignalHandler.disconnect (scope_search, sig_id);
+    if (timer_source_id != 0)
+    {
+      Source.remove (timer_source_id);
+      scope_search.search_context.result_set.flush ();
+    }
 
     if (scope_search.results_model.get_n_rows () == 0
       && scope_search.search_context.search_query.strip () == ""

=== modified file 'src/unity-scope-dbus-connector.vala'
--- src/unity-scope-dbus-connector.vala	2013-06-19 01:31:25 +0000
+++ src/unity-scope-dbus-connector.vala	2013-08-04 13:41:25 +0000
@@ -42,6 +42,7 @@
   private Internal.DefaultScopeDBusImpl pimpl;
   private bool exported;
   private bool name_owned;
+  private ulong name_unowned_id;
 
   private static int proxy_usage_count = 0;
 
@@ -51,7 +52,8 @@
     {
       if (pimpl == null) pimpl = new Internal.DefaultScopeDBusImpl (scope);
       pimpl.timeout = DAEMON_TIMEOUT_SEC;
-      pimpl.on_timeout_reached.connect (timeout_reached);
+      pimpl.on_timeout_reached.connect (on_inactivity_timeout_reached);
+      pimpl.on_unexport_timeout_reached.connect (on_unexport_timeout_reached);
       pimpl.export ();
       exported = true;
       own_name ();
@@ -72,15 +74,22 @@
     }
   }
 
-  private void timeout_reached ()
+  private void on_inactivity_timeout_reached ()
   {
+    // there were no requests to this for a while, prepare to exit
     unown_name ();
-    Timeout.add_seconds_full (Priority.DEFAULT_IDLE, 30, () =>
-    {
-      unexport ();
-      if (proxy_usage_count == 0) quit ();
-      return false;
-    });
+  }
+
+  private void on_unexport_timeout_reached ()
+  {
+    unexport ();
+    if (proxy_usage_count == 0) quit ();
+  }
+
+  private void on_name_unowned ()
+  {
+    // not starting a timer here, cause it would keep this object alive
+    pimpl.start_unexport_timer ();
   }
 
   private void own_name ()
@@ -89,9 +98,15 @@
     var dbus_name = scope.get_group_name ();
     manager.own_name (dbus_name);
     name_owned = true;
+
+    if (name_unowned_id == 0)
+    {
+      name_unowned_id = manager.name_unowned[dbus_name].connect (
+        this.on_name_unowned);
+    }
   }
 
-  private void unown_name()
+  private void unown_name ()
   {
     if (name_owned)
     {

=== modified file 'src/unity-scope-dbus-impl.vala'
--- src/unity-scope-dbus-impl.vala	2013-07-24 13:15:22 +0000
+++ src/unity-scope-dbus-impl.vala	2013-08-04 13:41:25 +0000
@@ -51,7 +51,8 @@
   private string _dbus_name;
   private DBusConnection? _dbus_connection;
   private Rand _rand;
-  private uint _timeout_source_id;
+  private uint _inactivity_timeout_source_id;
+  private uint _unexport_timeout_source_id;
   private ulong _scope_results_invalidated_id;
   private bool _query_happened;
 
@@ -66,6 +67,7 @@
   public int timeout { get; set; default = -1; }
 
   public signal void on_timeout_reached ();
+  public signal void on_unexport_timeout_reached ();
 
   public DefaultScopeDBusImpl (AbstractScope owner)
   {
@@ -74,10 +76,16 @@
 
   protected override void dispose ()
   {
-    if (_timeout_source_id != 0)
-    {
-      Source.remove (_timeout_source_id);
-      _timeout_source_id = 0;
+    if (_inactivity_timeout_source_id != 0)
+    {
+      Source.remove (_inactivity_timeout_source_id);
+      _inactivity_timeout_source_id = 0;
+    }
+
+    if (_unexport_timeout_source_id != 0)
+    {
+      Source.remove (_unexport_timeout_source_id);
+      _unexport_timeout_source_id = 0;
     }
   }
 
@@ -164,6 +172,20 @@
     }
   }
 
+  public void start_unexport_timer ()
+  {
+    if (_unexport_timeout_source_id != 0)
+      Source.remove (_unexport_timeout_source_id);
+
+    _unexport_timeout_source_id =
+      Timeout.add_seconds_full (Priority.DEFAULT_IDLE, 30, () =>
+      {
+        this.on_unexport_timeout_reached ();
+        _unexport_timeout_source_id = 0;
+        return false;
+      });
+  }
+
   private VariantBuilder? changed_props;
 
   public void queue_property_notification (string prop_name, Variant prop_value)
@@ -327,7 +349,7 @@
     }
     else
     {
-      _timeout_source_id = 0;
+      _inactivity_timeout_source_id = 0;
       on_timeout_reached ();
       return false;
     }
@@ -599,9 +621,9 @@
       // after first search request, a timeout is started and if no activity
       // happens while the timeout runs, the on_timeout_reached signal
       // will be emitted, which might end up quitting this process
-      if (_timeout_source_id == 0)
+      if (_inactivity_timeout_source_id == 0)
       {
-        _timeout_source_id = Timeout.add_seconds_full (Priority.DEFAULT_IDLE,
+        _inactivity_timeout_source_id = Timeout.add_seconds_full (Priority.DEFAULT_IDLE,
                                                        (uint) timeout,
                                                        timeout_reached);
       }
@@ -841,6 +863,9 @@
     }
   }
 
+  [Signal (detailed=true)]
+  public signal void name_unowned ();
+
   public void unown_name (string dbus_name)
   {
     unowned Internal.OwnedName? info = owned_names.get (dbus_name);
@@ -853,8 +878,9 @@
         if (info.bus_name_own_handle != 0)
         {
           Bus.unown_name (info.bus_name_own_handle);
+          Signal.emit_by_name (this, "name_unowned::" + dbus_name, typeof (void));
         }
-        owned_names.remove(dbus_name);
+        owned_names.remove (dbus_name);
       }
     }
   }