Merge lp:~larsu/new-indicator-file-format

IndicatorNg: update indicator file format

The old file format had some shortcomings:

(1) It was impossible to efficiently reuse a menu for different profiles,
because the profile name was implicit in the object path. The only way to do
this was to export the same menu twice. Now, object paths have to be set
explicitly in the indicator file.

(2) The well-known dbus name of a service and the name of its service file were
similar but slightly different (com.canonical.indicator.test vs
com.canonical.test.indicator), which caused some confusion on when to use
which. Now, the file name *is* the bus name, and the `BusName` key has been
dropped.

The new file format is documented in README.

6 files changed, 106 insertions, 40 deletions

diff --git a/README b/README
index e69de29..1ea62ef 100644
--- a/README
+++ b/README
@@ -0,0 +1,35 @@
+The indicator service file format
+=================================
+
+Unity's panel finds out about indicator by looking at indicator service
+files in `/usr/share/unity/indicators`. These files have to have the same
+name as the well-known D-Bus name that the corresponding service owns.
+
+An indicator file is a normal key file (like desktop files).  It must have
+an `[Indicator Service]` section, that contains the service's name
+(`Name`) and optionally the object path at which its action group is found
+(`ObjectPath`). For example:
+
+  [Indicator Service]
+  Name=indicator-example
+  ObjectPath=/com/canonical/indicator/example
+
+An indicator can only export one action group, but a menu for each profile
+("desktop", "greeter", "phone") supports. There must be a section for each
+of those profiles, containing the object path on which the menu is
+exported:
+
+  [desktop]
+  ObjectPath=/com/canonical/indicator/example/desktop
+
+  [greeter]
+  ObjectPath=/com/canonical/indicator/example/desktop
+
+  [phone]
+  ObjectPath=/com/canonical/indicator/example/phone
+
+Object paths can be reused for different profiles (the greeter uses the
+same menu as the desktop in the above example).
+
+There are no fallbacks. If a profile is not mentioned in the service file,
+the indicator will not show up for that profile.
diff --git a/libindicator/indicator-ng.c b/libindicator/indicator-ng.c
index fd78662..88db31c 100644
--- a/libindicator/indicator-ng.c
+++ b/libindicator/indicator-ng.c
@@ -29,6 +29,7 @@ struct _IndicatorNg
   gchar *service_file;
   gchar *name;
   gchar *object_path;
+  gchar *menu_object_path;
   gchar *bus_name;
   gchar *profile;
   gchar *header_action;
@@ -152,6 +153,7 @@ indicator_ng_finalize (GObject *object)
   g_free (self->service_file);
   g_free (self->name);
   g_free (self->object_path);
+  g_free (self->menu_object_path);
   g_free (self->bus_name);
   g_free (self->accessible_desc);
   g_free (self->header_action);
@@ -254,15 +256,12 @@ indicator_ng_update_entry (IndicatorNg *self)
   state = g_action_group_get_action_state (self->actions, self->header_action);
   if (state && g_variant_is_of_type (state, G_VARIANT_TYPE ("(sssb)")))
     {
-      gchar *iconstr = NULL;
+      const gchar *iconstr = NULL;
 
       g_variant_get (state, "(&s&s&sb)", &label, &iconstr, &accessible_desc, &visible);
 
       if (iconstr)
-        {
-          icon = g_variant_ref_sink (g_variant_new_string (iconstr));
-          g_free (iconstr);
-        }
+        icon = g_variant_ref_sink (g_variant_new_string (iconstr));
     }
   else if (state && g_variant_is_of_type (state, G_VARIANT_TYPE ("a{sv}")))
     {
@@ -362,11 +361,13 @@ indicator_ng_service_appeared (GDBusConnection *connection,
                                gpointer         user_data)
 {
   IndicatorNg *self = user_data;
-  gchar *menu_object_path;
 
   g_assert (!self->actions);
   g_assert (!self->menu);
 
+  /* watch is not established when menu_object_path == NULL */
+  g_assert (self->menu_object_path);
+
   self->session_bus = g_object_ref (connection);
 
   self->actions = G_ACTION_GROUP (g_dbus_action_group_get (connection, name_owner, self->object_path));
@@ -375,15 +376,12 @@ indicator_ng_service_appeared (GDBusConnection *connection,
   g_signal_connect_swapped (self->actions, "action-removed", G_CALLBACK (indicator_ng_update_entry), self);
   g_signal_connect_swapped (self->actions, "action-state-changed", G_CALLBACK (indicator_ng_update_entry), self);
 
-  menu_object_path = g_strconcat (self->object_path, "/", self->profile, NULL);
-  self->menu = G_MENU_MODEL (g_dbus_menu_model_get (connection, name_owner, menu_object_path));
+  self->menu = G_MENU_MODEL (g_dbus_menu_model_get (connection, name_owner, self->menu_object_path));
   g_signal_connect (self->menu, "items-changed", G_CALLBACK (indicator_ng_menu_changed), self);
   if (g_menu_model_get_n_items (self->menu))
     indicator_ng_menu_changed (self->menu, 0, 0, 1, self);
 
   indicator_ng_update_entry (self);
-
-  g_free (menu_object_path);
 }
 
 static void
@@ -460,39 +458,70 @@ indicator_ng_service_vanished (GDBusConnection *connection,
 }
 
 static gboolean
+indicator_ng_load_from_keyfile (IndicatorNg  *self,
+                                GKeyFile     *keyfile,
+                                GError      **error)
+{
+  g_assert (self->name == NULL);
+  g_assert (self->object_path == NULL);
+  g_assert (self->menu_object_path == NULL);
+
+  self->name = g_key_file_get_string (keyfile, "Indicator Service", "Name", error);
+  if (self->name == NULL)
+    return FALSE;
+
+  self->object_path = g_key_file_get_string (keyfile, "Indicator Service", "ObjectPath", error);
+  if (self->object_path == NULL)
+    return FALSE;
+
+  /*
+   * Don't throw an error when the profile doesn't exist. Non-existant
+   * profiles are silently ignored by not showing an indicator at all.
+   */
+  if (g_key_file_has_group (keyfile, self->profile))
+    {
+      /* however, if the profile exists, it must have "ObjectPath" */
+      self->menu_object_path = g_key_file_get_string (keyfile, self->profile, "ObjectPath", error);
+      if (self->menu_object_path == NULL)
+        return FALSE;
+    }
+
+  return TRUE;
+}
+
+static gboolean
 indicator_ng_initable_init (GInitable     *initable,
                             GCancellable  *cancellable,
                             GError       **error)
 {
   IndicatorNg *self = INDICATOR_NG (initable);
   GKeyFile *keyfile;
+  gboolean success;
 
-  keyfile = g_key_file_new ();
-  if (!g_key_file_load_from_file (keyfile, self->service_file, G_KEY_FILE_NONE, error))
-    goto out;
-
-  if (!(self->name = g_key_file_get_string (keyfile, "Indicator Service", "Name", error)))
-    goto out;
-
-  self->entry.name_hint = self->name;
+  self->bus_name = g_path_get_basename (self->service_file);
 
-  if (!(self->bus_name = g_key_file_get_string (keyfile, "Indicator Service", "BusName", error)))
-    goto out;
+  keyfile = g_key_file_new ();
+  if (g_key_file_load_from_file (keyfile, self->service_file, G_KEY_FILE_NONE, error) &&
+      indicator_ng_load_from_keyfile (self, keyfile, error))
+    {
+      self->entry.name_hint = self->name;
 
-  if (!(self->object_path = g_key_file_get_string (keyfile, "Indicator Service", "ObjectPath", error)))
-    goto out;
+      /* only watch the service when it supports the proile we're interested in */
+      if (self->menu_object_path)
+        {
+          self->name_watch_id = g_bus_watch_name (G_BUS_TYPE_SESSION,
+                                                  self->bus_name,
+                                                  G_BUS_NAME_WATCHER_FLAGS_AUTO_START,
+                                                  indicator_ng_service_appeared,
+                                                  indicator_ng_service_vanished,
+                                                  self, NULL);
+        }
 
-  self->name_watch_id = g_bus_watch_name (G_BUS_TYPE_SESSION,
-                                          self->bus_name,
-                                          G_BUS_NAME_WATCHER_FLAGS_AUTO_START,
-                                          indicator_ng_service_appeared,
-                                          indicator_ng_service_vanished,
-                                          self, NULL);
+      success = TRUE;
+    }
 
-out:
   g_key_file_free (keyfile);
-
-  return self->name_watch_id > 0;
+  return success;
 }
 
 static void
diff --git a/tests/com.canonical.test.indicator b/tests/com.canonical.indicator.no-such-service
index dad4c94..54cc4fd 100644
--- a/tests/com.canonical.test.indicator
+++ b/tests/com.canonical.indicator.no-such-service
@@ -1,4 +1,3 @@
 [Indicator Service]
 Name=indicator-test
-BusName=com.canonical.indicator.test
 ObjectPath=/com/canonical/indicator/test
diff --git a/tests/com.canonical.test.nosuchservice.indicator b/tests/com.canonical.indicator.test
index 8464749..0c7122f 100644
--- a/tests/com.canonical.test.nosuchservice.indicator
+++ b/tests/com.canonical.indicator.test
@@ -1,4 +1,6 @@
 [Indicator Service]
 Name=indicator-test
-BusName=com.canonical.indicator.test.nosuchservice
 ObjectPath=/com/canonical/indicator/test
+
+[desktop]
+ObjectPath=/com/canonical/indicator/test/desktop
diff --git a/tests/test-indicator-ng.c b/tests/test-indicator-ng.c
index 150cc37..7b677d3 100644
--- a/tests/test-indicator-ng.c
+++ b/tests/test-indicator-ng.c
@@ -50,11 +50,11 @@ test_instantiation (void)
   GError *error = NULL;
   GMainLoop *loop;
 
-  indicator = indicator_ng_new (SRCDIR "/com.canonical.test.nosuchservice.indicator", &error);
+  indicator = indicator_ng_new (SRCDIR "/com.canonical.indicator.no-such-service", &error);
   g_assert (indicator);
   g_assert (error == NULL);
 
-  g_assert_cmpstr (indicator_ng_get_service_file (indicator), ==, SRCDIR "/com.canonical.test.nosuchservice.indicator");
+  g_assert_cmpstr (indicator_ng_get_service_file (indicator), ==, SRCDIR "/com.canonical.indicator.no-such-service");
   g_assert_cmpstr (indicator_ng_get_profile (indicator), ==, "desktop");
 
   {
@@ -65,7 +65,7 @@ test_instantiation (void)
                              "profile", &profile,
                              NULL);
 
-    g_assert_cmpstr (service_file, ==, SRCDIR "/com.canonical.test.nosuchservice.indicator");
+    g_assert_cmpstr (service_file, ==, SRCDIR "/com.canonical.indicator.no-such-service");
     g_assert_cmpstr (profile, ==, "desktop");
 
     g_free (service_file);
@@ -89,7 +89,7 @@ test_instantiation_with_profile (void)
   IndicatorNg *indicator;
   GError *error = NULL;
 
-  indicator = indicator_ng_new_for_profile (SRCDIR "/com.canonical.test.indicator", "greeter", &error);
+  indicator = indicator_ng_new_for_profile (SRCDIR "/com.canonical.indicator.test", "greeter", &error);
   g_assert (indicator);
   g_assert (error == NULL);
 
@@ -107,7 +107,7 @@ test_menu (void)
   GList *entries;
   IndicatorObjectEntry *entry;
 
-  indicator = indicator_ng_new (SRCDIR "/com.canonical.test.indicator", &error);
+  indicator = indicator_ng_new (SRCDIR "/com.canonical.indicator.test", &error);
   g_assert (indicator);
   g_assert (error == NULL);
 
diff --git a/tools/indicator-loader.c b/tools/indicator-loader.c
index 5659dea..2ec9913 100644
--- a/tools/indicator-loader.c
+++ b/tools/indicator-loader.c
@@ -150,7 +150,7 @@ load_module (const gchar * name, GtkWidget * menu)
 		io = indicator_object_new_from_file(name);
 	}
 #if GTK_MAJOR_VERSION == 3
-	else if (g_str_has_suffix(name, ".indicator")) {
+	else {
 		GError *error = NULL;
 
 		io = INDICATOR_OBJECT (indicator_ng_new_for_profile (name, profile, &error));
@@ -162,9 +162,10 @@ load_module (const gchar * name, GtkWidget * menu)
 
 		title = g_strdup_printf ("%s %s", profile, name);
 	}
-#endif
+#else
 	else
 		return FALSE;
+#endif
 
 	/* Connect to it's signals */
 	g_signal_connect(G_OBJECT(io), INDICATOR_OBJECT_SIGNAL_ENTRY_ADDED,   G_CALLBACK(entry_added),    menu);