Merge lp:~larsu/libindicator/add-indicator-ng into lp:libindicator/13.04

[Lars Karlitski]

Add IndicatorNg.

IndicatorNg is an indicator object that reads an indicator service file and watches the bus for a corresponding service to appear. It turns the menus and actions exported by the service into an indicator entry. I think this is a good solution for the transition period in which we support both styles of indicators. (It means we don't need to copy templates around.)

An indicator service file must have an ".indicator" extension and contents simlilar to this:

  [Indicator Service]
  Name=indicator-test
  BusName=com.canonical.indicator.test
  ObjectPath=/com/canonical/indicator/test

For unity-panel-service, these files will be installed somewhere. The indicator-loader in this branch accepts a path to such a file as the first command line argument (instead of the .so file).

This can be tested with the example indicator in lp:~larsu/libunity/add-indicator (examples/indicator.vala).

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:475
http://jenkins.qa.ubuntu.com/job/libindicator-ci/13/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/libindicator-ci/./build=pbuilder,distribution=coverity-raring,flavor=amd64/13/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/libindicator-ci/./build=pbuilder,distribution=raring,flavor=armhf/13/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/libindicator-ci/./build=pbuilder,distribution=raring,flavor=i386/13/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/libindicator-ci/13//rebuild/?

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:477
http://jenkins.qa.ubuntu.com/job/libindicator-ci/14/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/libindicator-ci/./build=pbuilder,distribution=coverity-raring,flavor=amd64/14/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/libindicator-ci/./build=pbuilder,distribution=raring,flavor=armhf/14/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/libindicator-ci/./build=pbuilder,distribution=raring,flavor=i386/14/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/libindicator-ci/14//rebuild/?

review: Needs Fixing (continuous-integration)

[lp:~larsu/libindicator/add-indicator-ng]

478. By Lars Karlitski on 2013-01-21

indicator-ng.h: use local include

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:478
http://jenkins.qa.ubuntu.com/job/libindicator-ci/15/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/libindicator-ci/./build=pbuilder,distribution=coverity-raring,flavor=amd64/15/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/libindicator-ci/./build=pbuilder,distribution=raring,flavor=armhf/15/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/libindicator-ci/./build=pbuilder,distribution=raring,flavor=i386/15/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/libindicator-ci/15//rebuild/?

review: Approve (continuous-integration)

[lp:~larsu/libindicator/add-indicator-ng]

479. By Lars Karlitski on 2013-01-21

indicator-ng: always set an accessible description to avoid imminent warning

480. By Lars Karlitski on 2013-01-21

indicator-ng: fix crash (tried to free a string with g_object_unref)

481. By Lars Karlitski on 2013-01-21

indicator-ng: add indicator_ng_new_for_profile

482. By Lars Karlitski on 2013-01-21

indicator-ng: add getters

483. By Lars Karlitski on 2013-01-21

Make sure indicator-ng.h is installed

484. By Lars Karlitski on 2013-01-21

Add basic tests for indicator-ng

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:484
http://jenkins.qa.ubuntu.com/job/libindicator-ci/16/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/libindicator-ci/./build=pbuilder,distribution=coverity-raring,flavor=amd64/16/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/libindicator-ci/./build=pbuilder,distribution=raring,flavor=armhf/16/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/libindicator-ci/./build=pbuilder,distribution=raring,flavor=i386/16/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/libindicator-ci/16//rebuild/?

review: Approve (continuous-integration)

[lp:~larsu/libindicator/add-indicator-ng]

485. By Lars Karlitski on 2013-01-22

indicator-ng: properly unset action group when the service disappears

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:485
http://jenkins.qa.ubuntu.com/job/libindicator-ci/17/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/libindicator-ci/./build=pbuilder,distribution=coverity-raring,flavor=amd64/17/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/libindicator-ci/./build=pbuilder,distribution=raring,flavor=armhf/17/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/libindicator-ci/./build=pbuilder,distribution=raring,flavor=i386/17/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/libindicator-ci/17//rebuild/?

review: Approve (continuous-integration)

[lp:~larsu/libindicator/add-indicator-ng]

486. By Lars Karlitski on 2013-01-23

indicator-ng: auto start service if it's not running

487. By Lars Karlitski on 2013-01-23

indicator-ng: use base->get_entries to get the invisible ones, too

488. By Lars Karlitski on 2013-01-23

indicator-ng: set name hint to the value of the service file's "Name" field

489. By Lars Karlitski on 2013-01-23

indicator-ng: more elaborate testing

Use GTestDBus to spawn a small test service (tests/indicator-test-service.c)
and check whether the indicator menu gets turned into a gtkmenu correctly.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:489
http://jenkins.qa.ubuntu.com/job/libindicator-ci/18/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/libindicator-ci/./build=pbuilder,distribution=coverity-raring,flavor=amd64/18/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/libindicator-ci/./build=pbuilder,distribution=raring,flavor=armhf/18/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/libindicator-ci/./build=pbuilder,distribution=raring,flavor=i386/18/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/libindicator-ci/18//rebuild/?

review: Approve (continuous-integration)

[Charles Kerr (charlesk)]

1. You're overriding get_label, get_image, get_entry, get_name, get_accessible... and also have that indicator_ng_get_entry() func to work around visible/invisible entries in the parent. It would be more straightforward to remove all of that, replace the label/image/gtkmenu/accessible_desc fields in _IndicatorNg with "IndicatorObjectEntry entry;" and override get_entries() as a one-liner "return g_list_append (NULL, &self->entry);"

2. Bonus points if you'd add "void indicator_object_entry_set_accessible_desc (IndicatorObject * io, IndicatorObjectEntry * entry, const char * accessible_desc);" in indicator-object.h, similar to the existing i_o_e_activate() func, and use that instead of indicator_ng_set_accessible_desc()

3. I'm wondering if we should use a 'broken' image as a fallback when setting the image. That would prevent the "disappearing indicator" problem from regressing, such as we had with indicator-session when the cog icon wasn't in the theme. A broken icon isn't ideal, but it's better than no icon at all. Same for the gtk_widget_hide() call in indicator_ng_update_entry().

4. is there a reason to not use (&s&s&sb) in indicator_ng_update_entry's g_variant_get() call?

5. "added < 2 && removed < 2 && added ^ removed" deserves a comment in the source code. I /think/ you're XORing to confirm that added and removed aren't the same. Whether or not that's the case, it deserves a code comment.

6. It might be slightly cleaner to use g_object_connect() in indicator_ng_service_appeared(), and g_object_disconnect(). This isn't an important point.

7. In indicator_ng_menu_changed(), we may use "action" as an uninitialized pointer if g_menu_model_get_item_attribute() fails. This seems unlikely to me, but it would be better to check for failure and log a g_warning() instead of using an uninitialized pointer.

8. g_build_filename() instead of g_strconcat()?

[Charles Kerr (charlesk)]

NVM about g_build_filename(); that doesn't make sense for object paths.

[lp:~larsu/libindicator/add-indicator-ng]

490. By Lars Karlitski on 2013-01-23

indicator-ng: test indicator_ng_get_property

491. By Lars Karlitski on 2013-01-23

Add trim-lcov.py

It strips some unreachable branches from the coverage reports, such as
g_return_if_fail.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:491
http://jenkins.qa.ubuntu.com/job/libindicator-ci/19/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/libindicator-ci/./build=pbuilder,distribution=coverity-raring,flavor=amd64/19/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/libindicator-ci/./build=pbuilder,distribution=raring,flavor=armhf/19/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/libindicator-ci/./build=pbuilder,distribution=raring,flavor=i386/19/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/libindicator-ci/19//rebuild/?

review: Approve (continuous-integration)

[lp:~larsu/libindicator/add-indicator-ng]

492. By Lars Karlitski on 2013-01-24

indicator-ng: use an IndicatorObjectEntry internally

We needed the entry anyway (that's what indicator_ng_get_entry was for).

493. By Lars Karlitski on 2013-01-24

indicator-ng: show broken image when g_icon_for_string returns NULL

494. By Lars Karlitski on 2013-01-24

indicator-ng: save unnecessary allocations by using "&" in g_variant_get

495. By Lars Karlitski on 2013-01-24

indicator-ng: document error conditions in menu_changed

496. By Lars Karlitski on 2013-01-24

indicator-ng: check return value of g_menu_model_get_item_attribute

If it returns false, we'd use uninitialized memory.

[Lars Karlitski (larsu)]

> 1. You're overriding get_label, get_image, get_entry, get_name,
> get_accessible... and also have that indicator_ng_get_entry() func to work
> around visible/invisible entries in the parent. It would be more
> straightforward to remove all of that, replace the
> label/image/gtkmenu/accessible_desc fields in _IndicatorNg with
> "IndicatorObjectEntry entry;" and override get_entries() as a one-liner
> "return g_list_append (NULL, &self->entry);"

Very good point, thank you.

 1 file changed, 29 insertions(+), 86 deletions(-)

r492.

> 2. Bonus points if you'd add "void indicator_object_entry_set_accessible_desc
> (IndicatorObject * io, IndicatorObjectEntry * entry, const char *
> accessible_desc);" in indicator-object.h, similar to the existing
> i_o_e_activate() func, and use that instead of
> indicator_ng_set_accessible_desc()

Makes sense, but I'm not sure this works without potentially breaking some code, because accessible_desc is declared as const. That's the reason I still kept an accessible_desc in the IndicatorNg struct.

Or am I missing something?

> 3. I'm wondering if we should use a 'broken' image as a fallback when setting
> the image. That would prevent the "disappearing indicator" problem from
> regressing, such as we had with indicator-session when the cog icon wasn't in
> the theme. A broken icon isn't ideal, but it's better than no icon at all.
> Same for the gtk_widget_hide() call in indicator_ng_update_entry().

I totally agree and this is the way I intended it to work. The icon string we get falls into one of these cases:

 (1) an empty string
 (2) a valid icon name, for which an icon is available on the system
 (3) a valid icon name without an installed icon
 (4) a malformed icon string (i.e. g_icon_new_for_string() returns NULL)

For (1), no icon should be displayed. (2) obviously shows the installed icon. Both (3) and (4) should display the broken image. I already did this for (3) (set_from_gicon_string() returns TRUE, but GtkImage sets a broken icon), but I missed case (4).

Fixed in r493.

> 4. is there a reason to not use (&s&s&sb) in indicator_ng_update_entry's
> g_variant_get() call?

No, fixed in r494.

> 5. "added < 2 && removed < 2 && added ^ removed" deserves a comment in the
> source code. I /think/ you're XORing to confirm that added and removed aren't
> the same. Whether or not that's the case, it deserves a code comment.

Very true. r495.

> 6. It might be slightly cleaner to use g_object_connect() in
> indicator_ng_service_appeared(), and g_object_disconnect(). This isn't an
> important point.

Yeah, I dislike that call when dealing with swapped arguments.

> 7. In indicator_ng_menu_changed(), we may use "action" as an uninitialized
> pointer if g_menu_model_get_item_attribute() fails. This seems unlikely to me,
> but it would be better to check for failure and log a g_warning() instead of
> using an uninitialized pointer.

Good catch! I don't think it makes sense to throw a warning here. Even if the menu item has an "action" attribute, the value might not point to a valid action.

Fixed in r496.

> 8. g_build_filename() instead of g_strconcat()?

That's an object path, not a filename.

Thank...

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:496
http://jenkins.qa.ubuntu.com/job/libindicator-ci/20/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/libindicator-ci/./build=pbuilder,distribution=coverity-raring,flavor=amd64/20/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/libindicator-ci/./build=pbuilder,distribution=raring,flavor=armhf/20/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/libindicator-ci/./build=pbuilder,distribution=raring,flavor=i386/20/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/libindicator-ci/20//rebuild/?

review: Approve (continuous-integration)

[Ted Gould (ted)]

* When the service vanishes the program is immediately hiding the indicator and destroying everything. It doesn't make much sense to show this error to the user unless it can't be restarted. It should try to restart, and if either a timeout happens or an error on restart happens it hides the indicator. Much better user experience if we can handle the error in a reasonable way.

* I don't understand that, if we only have one action group, the action names need the "indicator" prefix. It seems they could just be unprefixed and make it easier on everyone.

* In indicator_ng_initable_init() you should really check for the group before looking at individual values in the group. It makes the errors much better for people trying to figure out WTH is going on when they mess up the file.

* In indicator_ng_initable_init() it seems like name and the bus name check should be separate, they're pretty independent and again it'll help error reporting.

* It looks like the name that is gotten from the Key file is not going through translations, which it needs to as it is a user visible string. This also means that the indicator file will need to give the translation domain.

* I don't understand why the profile is a string and not an enum. It seems like there'll always be a limited set, and they can't overlap, so enum is the right type to use. Seems like this should be passed up to the "environments" of indicator-object.

* We probably need a way for a .indicator file to support multiple profiles in the same file. It doesn't really make sense to have a whole set of files since all of them will have to be loaded for the convergence story.

* Having a single action that has a state (sssb) doesn't make a lot of sense as it makes it nearly impossible for us to remove or add attributes later. Better to split it into 4 today so that it can be 5 (or 3) tomorrow.

* The indicator needs a way to provide a string that can be used for measuring the space needed. We provide that in the libappindicator API and we should honor it further up the stack.

* If I remember right gtk_image_set_from_gicon() only supports icons that are square. Not all of our icons are square. Icons like the power icon are rectangular.

* You shouldn't use math on string pointers. This is bad "g_strdup (action + 10)". The problem is that if someone changes "indicator." to "bob." they'll grep through the code and never see that. Then they'll introduce a bug. If you do strlen() on a constant string the compiler can optimize that, and you still can explain the meaning of what you're intention is.

* Overall there are *very* few comments in the code.

* In the new interfacing there is no way to check the version of the service. The scenario would be that an upgrade happens, and the service no longer matches the version that the panel-service thinks it's looking for. It needs to handle that error instead of displaying erroneous results or an empty menu. I'd recommend adding this to the indicator file format and as an attribute on the base menu.

* It seems like the about-to-show needs to be passed down to the indicator somehow. This isn't currently handled and it's required for ...

review: Needs Fixing

[lp:~larsu/libindicator/add-indicator-ng]

497. By Lars Karlitski on 2013-01-25

indicator-ng: require header item to have x-canonical-type set

498. By Lars Karlitski on 2013-01-25

indicator-ng: lazily allocate entry.label and entry.image

Most indicators only need one of those.

499. By Lars Karlitski on 2013-01-25

indicator-ng: use indicator_image_helper

gtk_icon_set_from_gicon doesn't scale rectangular icons correctly.

This adds indicator_image_helper_update_from_gicon() and makes refresh_image()
set a broken image instead of erroring out when an icon couldn't be found.

500. By Lars Karlitski on 2013-01-25

indicator-ng: use strlen instead of hard coding the length

501. By Lars Karlitski on 2013-01-25

trim-lcov.py: add license header

[Lars Karlitski (larsu)]

Thanks a lot for the detailed review!

> * When the service vanishes the program is immediately hiding the indicator
> and destroying everything. It doesn't make much sense to show this error to
> the user unless it can't be restarted. It should try to restart, and if
> either a timeout happens or an error on restart happens it hides the
> indicator. Much better user experience if we can handle the error in a
> reasonable way.

Right, this isn't true for all indicators though. How about a "auto-reload" property that is set for all built-in indicators (or even settable from the .indicator file)?

> * I don't understand that, if we only have one action group, the action names
> need the "indicator" prefix. It seems they could just be unprefixed and make
> it easier on everyone.

I want to enable applications to reuse their app. actions in an indicator. Without this distinction, apps would have to add the same actions twice.

> * In indicator_ng_initable_init() you should really check for the group before
> looking at individual values in the group. It makes the errors much better
> for people trying to figure out WTH is going on when they mess up the file.

Good point. I started to implement it when I realised that g_key_file_get_string() already returns an error when the group doesn't exist:

  could not load indicator from <filename>: Key file does not have group 'Indicator Service'

I don't know what else a special check could add.

> * In indicator_ng_initable_init() it seems like name and the bus name check
> should be separate, they're pretty independent and again it'll help error
> reporting.

On second look, this is indeed a bit convoluted. But I'm not sure what you mean: move the g_bus_watch_name call out of the condition, or out of the function? It doesn't have any error conditions (I admit the "return self->name_watch_id > 0" is a bit misleading).

> * It looks like the name that is gotten from the Key file is not going through
> translations, which it needs to as it is a user visible string. This also
> means that the indicator file will need to give the translation domain.

The name from the key file the name-hint really, which isn't user-visible, is it? Would it be clearer if we named the key "NameHint" instead?

> * I don't understand why the profile is a string and not an enum. It seems
> like there'll always be a limited set, and they can't overlap, so enum is the
> right type to use. Seems like this should be passed up to the "environments"
> of indicator-object.

Oh! I meant to look into "environments", but forgot. Maybe we don't even need a separate "profile" property at all.

How exactly do these work? Does unity-panrl-service already create an indicator-object for each "environment" (greeter, desktop)?

> * We probably need a way for a .indicator file to support multiple profiles in
> the same file. It doesn't really make sense to have a whole set of files
> since all of them will have to be loaded for the convergence story.

That's already the way it works: each indicator-service can expose as many profiles as it wants to on <objectpath>/<profile>, where <objectpath> is from the .indicator file.

> ...

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:501
http://jenkins.qa.ubuntu.com/job/libindicator-ci/21/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/libindicator-ci/./build=pbuilder,distribution=coverity-raring,flavor=amd64/21/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/libindicator-ci/./build=pbuilder,distribution=raring,flavor=armhf/21/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/libindicator-ci/./build=pbuilder,distribution=raring,flavor=i386/21/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/libindicator-ci/21//rebuild/?

review: Approve (continuous-integration)

[Ted Gould (ted)]

On Fri, 2013-01-25 at 10:58 +0000, Lars Uebernickel wrote:
> > * When the service vanishes the program is immediately hiding the indicator
> > and destroying everything. It doesn't make much sense to show this error to
> > the user unless it can't be restarted. It should try to restart, and if
> > either a timeout happens or an error on restart happens it hides the
> > indicator. Much better user experience if we can handle the error in a
> > reasonable way.
>
> Right, this isn't true for all indicators though. How about a
> "auto-reload" property that is set for all built-in indicators (or
> even settable from the .indicator file)?

Uhm, which ones don't? That would be bad if there were some that don't.
Remember that we only want to use this for system indicators, not
applications. Because we need to wait to display the panel until all of
the services that have .indicator files are started and giving us
default state. We can't assume that for appindicators.

> > * I don't understand that, if we only have one action group, the action names
> > need the "indicator" prefix. It seems they could just be unprefixed and make
> > it easier on everyone.
>
> I want to enable applications to reuse their app. actions in an
> indicator. Without this distinction, apps would have to add the same
> actions twice.

Are you thinking appindicators here? I don't think we can drop KSNI
there, so we won't be able to use this for that. Otherwise all
indicators don't have any application actions.

Really application actions will probably only be used long term to
support GNOME applications on the desktop. They've shown no desire to
work with us (and KDE) on indicators in the past, I doubt they will in
the future. So never the two shall meet :-)

> > * In indicator_ng_initable_init() you should really check for the group before
> > looking at individual values in the group. It makes the errors much better
> > for people trying to figure out WTH is going on when they mess up the file.
>
> Good point. I started to implement it when I realised that
> g_key_file_get_string() already returns an error when the group
> doesn't exist:
>
> could not load indicator from <filename>: Key file does not have group 'Indicator Service'
>
> I don't know what else a special check could add.

Ah, okay. That should be fine then. Didn't realize it gave that error.

> > * In indicator_ng_initable_init() it seems like name and the bus name check
> > should be separate, they're pretty independent and again it'll help error
> > reporting.
>
> On second look, this is indeed a bit convoluted. But I'm not sure
> what you mean: move the g_bus_watch_name call out of the condition, or
> out of the function? It doesn't have any error conditions (I admit
> the "return self->name_watch_id > 0" is a bit misleading).

I was more thinking of breaking up this if statement into to:

515 + if ((self->name = g_key_file_get_string (keyfile, "Indicator
Service", "Name", error)) &&
516 + (bus_name = g_key_file_get_string (keyfile, "Indicator Service",
"BusName", error)) &&
517 + (self->object_path = g_key_file_get_string (keyfile, "Indicator
Service", "ObjectPath", error)))

It seems ...

[Lars Karlitski (larsu)]

> > Right, this isn't true for all indicators though. How about a
> > "auto-reload" property that is set for all built-in indicators (or
> > even settable from the .indicator file)?
>
> Uhm, which ones don't? That would be bad if there were some that don't.
> Remember that we only want to use this for system indicators, not
> applications. Because we need to wait to display the panel until all of
> the services that have .indicator files are started and giving us
> default state. We can't assume that for appindicators.

Matthew is thinking about having the sync indicator only show up when it is syncing. The (now deprecated) print indicator only shows when printing. Similar for keyboard / bluetooth, alhough they would probably only hide their indicators instead of terminating the service.

I definitely want to use the same mechanism for appindicators.

> > I want to enable applications to reuse their app. actions in an
> > indicator. Without this distinction, apps would have to add the same
> > actions twice.
>
> Are you thinking appindicators here? I don't think we can drop KSNI
> there, so we won't be able to use this for that. Otherwise all
> indicators don't have any application actions.

An application having an indicator is pretty common. I think it would be much easier for them if the could just hook up existing actions to the indicator menu, in the same way that they hook up those actions to the application menu.

I'm not saying we should drop support for KSNI. Though I think that spec needs to be updated (it depends on dbusmenu, doesn't it?)

> Really application actions will probably only be used long term to
> support GNOME applications on the desktop. They've shown no desire to
> work with us (and KDE) on indicators in the past, I doubt they will in
> the future. So never the two shall meet :-)

Hm? Why would a $TOOLKIT application not have application actions? I thought application menus were part of our platform?

What do you mean by GNOME applications? Apps using GNOME tech? Lots of those use indicators and go out of their way to support the Unity desktop (tomboy, transmission, shutter, sparkleshare). Also note that even though the indicator concept wasn't picked up by GNOME's default applications, there are features in GTK+ and GMenuModel that are *only* used in Unity's indicators.

I'm really just trying to make it as easy as possible for application developers to add an indicator to their app and hook it up to existing actions (see also my libunity merge request).

> [...]
> > On second look, this is indeed a bit convoluted. But I'm not sure
> > what you mean: move the g_bus_watch_name call out of the condition, or
> > out of the function? It doesn't have any error conditions (I admit
> > the "return self->name_watch_id > 0" is a bit misleading).
>
> I was more thinking of breaking up this if statement into to:
>
> 515 + if ((self->name = g_key_file_get_string (keyfile, "Indicator
> Service", "Name", error)) &&
> 516 + (bus_name = g_key_file_get_string (keyfile, "Indicator Service",
> "BusName", error)) &&
> 517 + (self->object_path = g_key_file_get_string (keyfile, "Indicator
> Service",...

[Ted Gould (ted)]

On Mon, 2013-01-28 at 10:27 +0000, Lars Uebernickel wrote:
> > > Right, this isn't true for all indicators though. How about a
> > > "auto-reload" property that is set for all built-in indicators (or
> > > even settable from the .indicator file)?
> >
> > Uhm, which ones don't? That would be bad if there were some that don't.
> > Remember that we only want to use this for system indicators, not
> > applications. Because we need to wait to display the panel until all of
> > the services that have .indicator files are started and giving us
> > default state. We can't assume that for appindicators.
>
> Matthew is thinking about having the sync indicator only show up when
> it is syncing. The (now deprecated) print indicator only shows when
> printing. Similar for keyboard � bluetooth, alhough they would
> probably only hide their indicators instead of terminating the
> service.

Sure. Though, it would be nice if they could terminate the service...
it seems like that's largely a special case rather than the standard
though. Seems like default should be auto-reload.

> I definitely want to use the same mechanism for appindicators.

Hmm, so then we'd need another property for which ones should be waited
on for initial bring up. And perhaps a way to mark KSNI ones as
there'll be different properties. I think that we're a ways away from
that.

But, in general, I'm not sure if using the same mechanism makes sense.
This can be a lot simpler and cleaner if it doesn't have to take on all
of the KSNI interface as well. Remember it is a registration interface,
so we wouldn't be able to really *replace* it, it'd have to be
additional and we'd have to start the service, then wait for the
registration.

> > > I want to enable applications to reuse their app. actions in an
> > > indicator. Without this distinction, apps would have to add the same
> > > actions twice.
> >
> > Are you thinking appindicators here? I don't think we can drop KSNI
> > there, so we won't be able to use this for that. Otherwise all
> > indicators don't have any application actions.
>
> An application having an indicator is pretty common. I think it would
> be much easier for them if the could just hook up existing actions to
> the indicator menu, in the same way that they hook up those actions to
> the application menu.

I'd challenge the initial assumption. I can think of very few
applications that have indicators, and I'd go further to say that most
that do are annoying :-)

I have no issue with them hooking up actions to the indicator. But it
would probably make more sense to allow registering action groups and
prefixes in general. As most applications don't have app�win actions,
but might have their own scheme for such a separation. For instance, I
think for Inkscape it'd make much more sense to have app�doc�win.
Unfortunately GtkApplication isn't very flexible, we shouldn't make the
same mistake.

> I'm not saying we should drop support for KSNI. Though I think that
> spec needs to be updated (it depends on dbusmenu, doesn't it?)

It just provides a path for a menu. I think if we ported qt-sni and
libappindicator there'd be no issue with those being GMenu ba...

[Lars Karlitski (larsu)]

> On Mon, 2013-01-28 at 10:27 +0000, Lars Uebernickel wrote:
> > > > Right, this isn't true for all indicators though. How about a
> > > > "auto-reload" property that is set for all built-in indicators (or
> > > > even settable from the .indicator file)?
> > >
> > > Uhm, which ones don't? That would be bad if there were some that don't.
> > > Remember that we only want to use this for system indicators, not
> > > applications. Because we need to wait to display the panel until all of
> > > the services that have .indicator files are started and giving us
> > > default state. We can't assume that for appindicators.
> >
> > Matthew is thinking about having the sync indicator only show up when
> > it is syncing. The (now deprecated) print indicator only shows when
> > printing. Similar for keyboard � bluetooth, alhough they would
> > probably only hide their indicators instead of terminating the
> > service.
>
> Sure. Though, it would be nice if they could terminate the service...
> it seems like that's largely a special case rather than the standard
> though. Seems like default should be auto-reload.

The default for system indicator should be auto-reload, I agree. I still think having a key in the .indicator file is a good idea.

> > I definitely want to use the same mechanism for appindicators.
>
> Hmm, so then we'd need another property for which ones should be waited
> on for initial bring up. And perhaps a way to mark KSNI ones as
> there'll be different properties. I think that we're a ways away from
> that.
>
> But, in general, I'm not sure if using the same mechanism makes sense.
> This can be a lot simpler and cleaner if it doesn't have to take on all
> of the KSNI interface as well. Remember it is a registration interface,
> so we wouldn't be able to really *replace* it, it'd have to be
> additional and we'd have to start the service, then wait for the
> registration.

Let's talk about this in more detail later. Or is it important for this MR?

> > > > I want to enable applications to reuse their app. actions in an
> > > > indicator. Without this distinction, apps would have to add the same
> > > > actions twice.
> > >
> > > Are you thinking appindicators here? I don't think we can drop KSNI
> > > there, so we won't be able to use this for that. Otherwise all
> > > indicators don't have any application actions.
> >
> > An application having an indicator is pretty common. I think it would
> > be much easier for them if the could just hook up existing actions to
> > the indicator menu, in the same way that they hook up those actions to
> > the application menu.
>
> I'd challenge the initial assumption. I can think of very few
> applications that have indicators, and I'd go further to say that most
> that do are annoying :-)

Really? Even if there are only a few, doesn't it make sense to allow them to use app. actions? I don't think sticking indicator. in front is such a big deal really.

> I have no issue with them hooking up actions to the indicator. But it
> would probably make more sense to allow registering action groups and
> prefixes in general. As most applications don't have app�win actions,
> but might...

[lp:~larsu/libindicator/add-indicator-ng]

502. By Lars Karlitski on 2013-01-30

indicator-ng: simplify flow in initable_init

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:502
http://jenkins.qa.ubuntu.com/job/libindicator-ci/22/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/libindicator-ci/./build=pbuilder,distribution=coverity-raring,flavor=amd64/22/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/libindicator-ci/./build=pbuilder,distribution=raring,flavor=armhf/22/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/libindicator-ci/./build=pbuilder,distribution=raring,flavor=i386/22/console
Coverity artifacts:
    http://jenkins.qa.ubuntu.com/job/libindicator-ci/./build=pbuilder,distribution=coverity-raring,flavor=amd64/22//artifact/results/coverity/NO_COVERITY_DEFECTS.txt
    http://jenkins.qa.ubuntu.com/job/libindicator-ci/./build=pbuilder,distribution=raring,flavor=armhf/22//artifact/results/coverity/NO_COVERITY_DEFECTS.txt
    http://jenkins.qa.ubuntu.com/job/libindicator-ci/./build=pbuilder,distribution=raring,flavor=i386/22//artifact/results/coverity/NO_COVERITY_DEFECTS.txt

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/libindicator-ci/22//rebuild/?

review: Approve (continuous-integration)

[lp:~larsu/libindicator/add-indicator-ng]

503. By Lars Karlitski on 2013-02-06

indicator-ng: try to restart the service when it crashes

This uses a (slightly) awkward heuristic: when the well-known name vanishes
from the session bus, it only restarts the service when it didn't explicitly
hide the indicator before.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:503
http://jenkins.qa.ubuntu.com/job/libindicator-ci/23/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/libindicator-ci/./build=pbuilder,distribution=coverity-raring,flavor=amd64/23/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/libindicator-ci/./build=pbuilder,distribution=raring,flavor=armhf/23/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/libindicator-ci/./build=pbuilder,distribution=raring,flavor=i386/23/console
Coverity artifacts:
    http://jenkins.qa.ubuntu.com/job/libindicator-ci/./build=pbuilder,distribution=coverity-raring,flavor=amd64/23//artifact/results/coverity/NO_COVERITY_DEFECTS.txt
    http://jenkins.qa.ubuntu.com/job/libindicator-ci/./build=pbuilder,distribution=raring,flavor=armhf/23//artifact/results/coverity/NO_COVERITY_DEFECTS.txt
    http://jenkins.qa.ubuntu.com/job/libindicator-ci/./build=pbuilder,distribution=raring,flavor=i386/23//artifact/results/coverity/NO_COVERITY_DEFECTS.txt

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/libindicator-ci/23//rebuild/?

review: Approve (continuous-integration)

[lp:~larsu/libindicator/add-indicator-ng]

504. By Lars Karlitski on 2013-02-06

indicator-ng: don't hide the indicator if the service is already running

[Ted Gould (ted)]

review: Approve

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:504
http://jenkins.qa.ubuntu.com/job/libindicator-ci/24/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/libindicator-ci/./build=pbuilder,distribution=coverity-raring,flavor=amd64/24/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/libindicator-ci/./build=pbuilder,distribution=raring,flavor=armhf/24/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/libindicator-ci/./build=pbuilder,distribution=raring,flavor=i386/24/console
Coverity artifacts:
    http://jenkins.qa.ubuntu.com/job/libindicator-ci/./build=pbuilder,distribution=coverity-raring,flavor=amd64/24//artifact/results/coverity/NO_COVERITY_DEFECTS.txt
    http://jenkins.qa.ubuntu.com/job/libindicator-ci/./build=pbuilder,distribution=raring,flavor=armhf/24//artifact/results/coverity/NO_COVERITY_DEFECTS.txt
    http://jenkins.qa.ubuntu.com/job/libindicator-ci/./build=pbuilder,distribution=raring,flavor=i386/24//artifact/results/coverity/NO_COVERITY_DEFECTS.txt

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/libindicator-ci/24//rebuild/?

review: Approve (continuous-integration)

[lp:~larsu/libindicator/add-indicator-ng]

505. By Lars Karlitski on 2013-02-14

indicator-ng: add license header

[Charles Kerr (charlesk)]

Thanks for that update :)

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:505
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~larsu/libindicator/add-indicator-ng/+merge/143934/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/libindicator-ci/25/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/libindicator-ci/./build=pbuilder,distribution=coverity-raring,flavor=amd64/25/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/libindicator-ci/./build=pbuilder,distribution=raring,flavor=armhf/25/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/libindicator-ci/./build=pbuilder,distribution=raring,flavor=i386/25/console
Coverity artifacts:
    http://jenkins.qa.ubuntu.com/job/libindicator-ci/./build=pbuilder,distribution=coverity-raring,flavor=amd64/25//artifact/results/coverity/NO_COVERITY_DEFECTS.txt
    http://jenkins.qa.ubuntu.com/job/libindicator-ci/./build=pbuilder,distribution=raring,flavor=armhf/25//artifact/results/coverity/NO_COVERITY_DEFECTS.txt
    http://jenkins.qa.ubuntu.com/job/libindicator-ci/./build=pbuilder,distribution=raring,flavor=i386/25//artifact/results/coverity/NO_COVERITY_DEFECTS.txt

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/libindicator-ci/25//rebuild/?

review: Needs Fixing (continuous-integration)

=== modified file 'Makefile.am.coverage'
--- Makefile.am.coverage	2012-03-27 21:33:31 +0000
+++ Makefile.am.coverage	2013-02-14 21:19:21 +0000
@@ -1,6 +1,8 @@
 
 # Coverage targets
 
+EXTRA_DIST = trim-lcov.py
+
 .PHONY: clean-gcno clean-gcda \
 	coverage-html generate-coverage-html clean-coverage-html \
 	coverage-gcovr generate-coverage-gcovr clean-coverage-gcovr
@@ -23,7 +25,7 @@
   
 generate-coverage-html:
 	@echo Collecting coverage data
-	$(LCOV) --directory $(top_builddir) --capture --output-file coverage.info --no-checksum --compat-libtool
+	$(LCOV) --directory $(top_builddir) --capture --no-checksum --compat-libtool | $(top_srcdir)/trim-lcov.py > coverage.info
 	LANG=C $(GENHTML) --prefix $(top_builddir) --output-directory coveragereport --title "Code Coverage" --legend --show-details coverage.info
   
 clean-coverage-html: clean-gcda

=== modified file 'configure.ac'
--- configure.ac	2012-11-21 19:21:10 +0000
+++ configure.ac	2013-02-14 21:19:21 +0000
@@ -43,7 +43,7 @@
 ##############################
 
 GTK_REQUIRED_VERSION=2.18
-GTK3_REQUIRED_VERSION=2.91
+GTK3_REQUIRED_VERSION=3.6
 GIO_UNIX_REQUIRED_VERSION=2.22
 
 AC_ARG_WITH([gtk],

=== modified file 'libindicator/Makefile.am'
--- libindicator/Makefile.am	2012-01-24 16:10:15 +0000
+++ libindicator/Makefile.am	2013-02-14 21:19:21 +0000
@@ -36,6 +36,10 @@
 	indicator-service.h \
 	indicator-service-manager.h
 
+if USE_GTK3
+indicator_headers += indicator-ng.h
+endif
+
 libindicatorinclude_HEADERS = \
 	$(indicator_headers)
 
@@ -53,6 +57,10 @@
 	indicator-service.c \
 	indicator-service-manager.c
 
+if USE_GTK3
+libindicator_la_SOURCES += indicator-ng.c
+endif
+
 libindicator_la_CFLAGS = \
 	$(LIBINDICATOR_CFLAGS) \
 	$(COVERAGE_CFLAGS) \

=== modified file 'libindicator/indicator-image-helper.c'
--- libindicator/indicator-image-helper.c	2012-04-11 06:01:51 +0000
+++ libindicator/indicator-image-helper.c	2013-02-14 21:19:21 +0000
@@ -62,7 +62,12 @@
 		/* Grab the filename */
 		icon_filename = gtk_icon_info_get_filename(icon_info);
 	}
-	g_return_if_fail(icon_filename != NULL); /* An error because we don't have a filename */
+
+	/* show a broken image if we don't have a filename */
+	if (icon_filename == NULL) {
+		gtk_image_set_from_stock (image, GTK_STOCK_MISSING_IMAGE, GTK_ICON_SIZE_LARGE_TOOLBAR);
+		return;
+	}
 
 	/* Build a pixbuf */
 	GError * error = NULL;
@@ -132,7 +137,8 @@
 	/* Build us an image */
 	GtkImage * image = GTK_IMAGE(gtk_image_new());
 
-	indicator_image_helper_update(image, name);
+	if (name)
+		indicator_image_helper_update(image, name);
 
 	return image;
 }
@@ -144,17 +150,27 @@
 	g_return_if_fail(name != NULL);
 	g_return_if_fail(name[0] != '\0');
 	g_return_if_fail(GTK_IS_IMAGE(image));
-	gboolean seen_previously = FALSE;
 
 	/* Build us a GIcon */
 	GIcon * icon_names = g_themed_icon_new_with_default_fallbacks(name);
 	g_warn_if_fail(icon_names != NULL);
 	g_return_if_fail(icon_names != NULL);
 
+	indicator_image_helper_update_from_gicon (image, icon_names);
+
+	g_object_unref (icon_names);
+	return;
+}
+
+void
+indicator_image_helper_update_from_gicon (GtkImage *image, GIcon *icon)
+{
+	gboolean seen_previously = FALSE;
+
 	seen_previously = (g_object_get_data(G_OBJECT(image), INDICATOR_NAMES_DATA) != NULL);
 
 	/* Attach our names to the image */
-	g_object_set_data_full(G_OBJECT(image), INDICATOR_NAMES_DATA, icon_names, g_object_unref);
+	g_object_set_data_full(G_OBJECT(image), INDICATOR_NAMES_DATA, g_object_ref (icon), g_object_unref);
 
 	/* Put the pixbuf in */
 	refresh_image(image);

=== modified file 'libindicator/indicator-image-helper.h'
--- libindicator/indicator-image-helper.h	2010-03-11 16:22:17 +0000
+++ libindicator/indicator-image-helper.h	2013-02-14 21:19:21 +0000
@@ -26,8 +26,10 @@
 
 #include <gtk/gtk.h>
 
-GtkImage *   indicator_image_helper           (const gchar * name);
-void         indicator_image_helper_update    (GtkImage * image,
-                                               const gchar * name);
+GtkImage *   indicator_image_helper                     (const gchar * name);
+void         indicator_image_helper_update              (GtkImage * image,
+                                                         const gchar * name);
+void         indicator_image_helper_update_from_gicon   (GtkImage * image,
+                                                         GIcon * icon);
 
 #endif /* __INDICATOR_IMAGE_HELPER_H__ */

=== added file 'libindicator/indicator-ng.c'
--- libindicator/indicator-ng.c	1970-01-01 00:00:00 +0000
+++ libindicator/indicator-ng.c	2013-02-14 21:19:21 +0000
@@ -0,0 +1,566 @@
+/*
+ * Copyright 2013 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 3, as published
+ * by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranties of
+ * MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
+ * PURPOSE.  See the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authors:
+ *     Lars Uebernickel <[e-mail]>
+ */
+
+#include "indicator-ng.h"
+#include "indicator-image-helper.h"
+
+#include <string.h>
+
+struct _IndicatorNg
+{
+  IndicatorObject parent;
+
+  gchar *service_file;
+  gchar *name;
+  gchar *object_path;
+  gchar *bus_name;
+  gchar *profile;
+  gchar *header_action;
+
+  guint name_watch_id;
+
+  GDBusConnection *session_bus;
+  GActionGroup *actions;
+  GMenuModel *menu;
+
+  IndicatorObjectEntry entry;
+  gchar *accessible_desc;
+
+  gint64 last_service_restart;
+};
+
+static void indicator_ng_initable_iface_init (GInitableIface *initable);
+G_DEFINE_TYPE_WITH_CODE (IndicatorNg, indicator_ng, INDICATOR_OBJECT_TYPE,
+                         G_IMPLEMENT_INTERFACE (G_TYPE_INITABLE, indicator_ng_initable_iface_init))
+
+enum
+{
+  PROP_0,
+  PROP_SERVICE_FILE,
+  PROP_PROFILE,
+  N_PROPERTIES
+};
+
+static GParamSpec *properties[N_PROPERTIES];
+
+static void
+indicator_ng_get_property (GObject    *object,
+                           guint       property_id,
+                           GValue     *value,
+                           GParamSpec *pspec)
+{
+  IndicatorNg *self = INDICATOR_NG (object);
+
+  switch (property_id)
+    {
+    case PROP_SERVICE_FILE:
+      g_value_set_string (value, self->service_file);
+      break;
+
+    case PROP_PROFILE:
+      g_value_set_string (value, self->profile);
+      break;
+
+    default:
+      G_OBJECT_WARN_INVALID_PROPERTY_ID (object, property_id, pspec);
+    }
+}
+
+static void
+indicator_ng_set_property (GObject      *object,
+                           guint         property_id,
+                           const GValue *value,
+                           GParamSpec   *pspec)
+{
+  IndicatorNg *self = INDICATOR_NG (object);
+
+  switch (property_id)
+    {
+    case PROP_SERVICE_FILE: /* construct-only */
+      self->service_file = g_strdup (g_value_get_string (value));
+      break;
+
+    case PROP_PROFILE: /* construct-only */
+      self->profile = g_strdup (g_value_get_string (value));
+      break;
+
+    default:
+      G_OBJECT_WARN_INVALID_PROPERTY_ID (object, property_id, pspec);
+    }
+}
+
+static void
+indicator_ng_free_actions_and_menu (IndicatorNg *self)
+{
+  if (self->actions)
+    {
+      gtk_widget_insert_action_group (GTK_WIDGET (self->entry.menu), "indicator", NULL);
+      g_signal_handlers_disconnect_by_data (self->actions, self);
+      g_clear_object (&self->actions);
+    }
+
+  if (self->menu)
+    {
+      g_signal_handlers_disconnect_by_data (self->menu, self);
+      g_clear_object (&self->menu);
+    }
+}
+
+static void
+indicator_ng_dispose (GObject *object)
+{
+  IndicatorNg *self = INDICATOR_NG (object);
+
+  if (self->name_watch_id)
+    {
+      g_bus_unwatch_name (self->name_watch_id);
+      self->name_watch_id = 0;
+    }
+
+  g_clear_object (&self->session_bus);
+
+  indicator_ng_free_actions_and_menu (self);
+
+  g_clear_object (&self->entry.label);
+  g_clear_object (&self->entry.image);
+  g_clear_object (&self->entry.menu);
+
+  G_OBJECT_CLASS (indicator_ng_parent_class)->dispose (object);
+}
+
+static void
+indicator_ng_finalize (GObject *object)
+{
+  IndicatorNg *self = INDICATOR_NG (object);
+
+  g_free (self->service_file);
+  g_free (self->name);
+  g_free (self->object_path);
+  g_free (self->bus_name);
+  g_free (self->accessible_desc);
+  g_free (self->header_action);
+
+  G_OBJECT_CLASS (indicator_ng_parent_class)->finalize (object);
+}
+
+static GList *
+indicator_ng_get_entries (IndicatorObject *io)
+{
+  IndicatorNg *self = INDICATOR_NG (io);
+
+  return g_list_append (NULL, &self->entry);
+}
+
+static void
+indicator_ng_set_accessible_desc (IndicatorNg *self,
+                                  const gchar *accessible_desc)
+{
+  g_free (self->accessible_desc);
+  self->accessible_desc = g_strdup (accessible_desc);
+
+  self->entry.accessible_desc = self->accessible_desc;
+  g_signal_emit_by_name (self, INDICATOR_OBJECT_SIGNAL_ACCESSIBLE_DESC_UPDATE, &self->entry);
+}
+
+static void
+indicator_ng_set_icon_from_string (IndicatorNg *self,
+                                   const gchar *str)
+{
+  GIcon *icon;
+  GError *error = NULL;
+
+  if (str == NULL || *str == '\0')
+    {
+      if (self->entry.image)
+        {
+          gtk_image_clear (self->entry.image);
+          gtk_widget_hide (GTK_WIDGET (self->entry.image));
+        }
+      return;
+    }
+
+  if (!self->entry.image)
+    self->entry.image = g_object_ref_sink (gtk_image_new ());
+
+  gtk_widget_show (GTK_WIDGET (self->entry.image));
+
+  icon = g_icon_new_for_string (str, &error);
+  if (icon)
+    {
+      indicator_image_helper_update_from_gicon (self->entry.image, icon);
+      g_object_unref (icon);
+    }
+  else
+    {
+      g_warning ("invalid icon string '%s': %s", str, error->message);
+      gtk_image_set_from_stock (self->entry.image, GTK_STOCK_MISSING_IMAGE, GTK_ICON_SIZE_LARGE_TOOLBAR);
+      g_error_free (error);
+    }
+}
+
+static void
+indicator_ng_set_label (IndicatorNg *self,
+                        const gchar *label)
+{
+  if (label == NULL || *label == '\0')
+    {
+      if (self->entry.label)
+        gtk_widget_hide (GTK_WIDGET (self->entry.label));
+      return;
+    }
+
+  if (!self->entry.label)
+    self->entry.label = g_object_ref_sink (gtk_label_new (NULL));
+
+  gtk_label_set_label (GTK_LABEL (self->entry.label), label);
+  gtk_widget_show (GTK_WIDGET (self->entry.label));
+}
+
+static void
+indicator_ng_update_entry (IndicatorNg *self)
+{
+  GVariant *state;
+
+  g_return_if_fail (self->menu != NULL);
+  g_return_if_fail (self->actions != NULL);
+
+  if (!self->header_action ||
+      !g_action_group_has_action (self->actions, self->header_action))
+    {
+      indicator_object_set_visible (INDICATOR_OBJECT (self), FALSE);
+      return;
+    }
+
+  state = g_action_group_get_action_state (self->actions, self->header_action);
+  if (state && g_variant_is_of_type (state, G_VARIANT_TYPE ("(sssb)")))
+    {
+      const gchar *label;
+      const gchar *iconstr;
+      const gchar *accessible_desc;
+      gboolean visible;
+
+      g_variant_get (state, "(&s&s&sb)", &label, &iconstr, &accessible_desc, &visible);
+
+      indicator_ng_set_label (self, label);
+      indicator_ng_set_icon_from_string (self, iconstr);
+      indicator_ng_set_accessible_desc (self, accessible_desc);
+      indicator_object_set_visible (INDICATOR_OBJECT (self), visible);
+    }
+  else
+    g_warning ("the action of the indicator menu item must have state with type (sssb)");
+
+  if (state)
+    g_variant_unref (state);
+}
+
+static gboolean
+indicator_ng_menu_item_is_of_type (GMenuModel  *menu,
+                                   gint         index,
+                                   const gchar *expected_type)
+{
+  gchar *type;
+  gboolean has_type = FALSE;
+
+  if (g_menu_model_get_item_attribute (menu, index, "x-canonical-type", "s", &type))
+    {
+      has_type = g_str_equal (type, expected_type);
+      g_free (type);
+    }
+
+  return has_type;
+}
+
+static void
+indicator_ng_menu_changed (GMenuModel *menu,
+                           gint        position,
+                           gint        removed,
+                           gint        added,
+                           gpointer    user_data)
+{
+  IndicatorNg *self = user_data;
+
+  /* The menu may only contain one item (the indicator title menu).
+   * Thus, the position is always 0, and there is either exactly one
+   * item added or exactly one item removed.
+   */
+  g_return_if_fail (position == 0);
+  g_return_if_fail (added < 2 && removed < 2 && added ^ removed);
+
+  if (removed)
+    indicator_object_set_visible (INDICATOR_OBJECT (self), FALSE);
+
+  if (added)
+    {
+      g_clear_pointer (&self->header_action, g_free);
+
+      if (indicator_ng_menu_item_is_of_type (self->menu, 0, "com.canonical.indicator.root"))
+        {
+          GMenuModel *popup;
+          gchar *action;
+
+          if (g_menu_model_get_item_attribute (self->menu, 0, G_MENU_ATTRIBUTE_ACTION, "s", &action) &&
+              g_str_has_prefix (action, "indicator."))
+            {
+              self->header_action = g_strdup (action + strlen ("indicator."));
+            }
+
+          popup = g_menu_model_get_item_link (self->menu, 0, G_MENU_LINK_SUBMENU);
+          if (popup)
+            {
+              gtk_menu_shell_bind_model (GTK_MENU_SHELL (self->entry.menu), popup, NULL, TRUE);
+              g_object_unref (popup);
+            }
+
+          indicator_ng_update_entry (self);
+
+          g_free (action);
+        }
+      else
+        g_warning ("indicator menu item must be of type 'com.canonical.indicator.root'");
+    }
+}
+
+static void
+indicator_ng_service_appeared (GDBusConnection *connection,
+                               const gchar     *name,
+                               const gchar     *name_owner,
+                               gpointer         user_data)
+{
+  IndicatorNg *self = user_data;
+  gchar *menu_object_path;
+
+  g_assert (!self->actions);
+  g_assert (!self->menu);
+
+  self->session_bus = g_object_ref (connection);
+
+  self->actions = G_ACTION_GROUP (g_dbus_action_group_get (connection, name_owner, self->object_path));
+  gtk_widget_insert_action_group (GTK_WIDGET (self->entry.menu), "indicator", self->actions);
+  g_signal_connect_swapped (self->actions, "action-added", G_CALLBACK (indicator_ng_update_entry), self);
+  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));
+  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
+indicator_ng_service_started (GObject      *source_object,
+                              GAsyncResult *result,
+                              gpointer      user_data)
+{
+  IndicatorNg *self = user_data;
+  GError *error = NULL;
+  GVariant *reply;
+
+  reply = g_dbus_connection_call_finish (G_DBUS_CONNECTION (source_object), result, &error);
+  if (!reply)
+    {
+      g_warning ("Could not activate service '%s': %s", self->name, error->message);
+      indicator_object_set_visible (INDICATOR_OBJECT (self), FALSE);
+      g_error_free (error);
+      return;
+    }
+
+  switch (g_variant_get_uint32 (reply))
+    {
+    case 1: /* DBUS_START_REPLY_SUCCESS */
+      break;
+
+    case 2: /* DBUS_START_REPLY_ALREADY_RUNNING */
+      g_warning ("could not start service '%s': it is already running", self->name);
+      break;
+
+    default:
+      g_assert_not_reached ();
+    }
+
+  g_variant_unref (reply);
+}
+
+static void
+indicator_ng_service_vanished (GDBusConnection *connection,
+                               const gchar     *name,
+                               gpointer         user_data)
+{
+  IndicatorNg *self = user_data;
+
+  indicator_ng_free_actions_and_menu (self);
+
+  /* Names may vanish because the service decided it doesn't need to
+   * show its indicator anymore, or because it crashed.  Let's assume it
+   * crashes and restart it unless it explicitly hid its indicator. */
+
+  if (indicator_object_entry_is_visible (INDICATOR_OBJECT (self), &self->entry))
+    {
+      gint64 now;
+
+      /* take care not to start it if it repeatedly crashes */
+      now = g_get_monotonic_time ();
+      if (now - self->last_service_restart < 1 * G_USEC_PER_SEC)
+        return;
+
+      self->last_service_restart = now;
+
+      g_dbus_connection_call (self->session_bus,
+                              "org.freedesktop.DBus",
+                              "/",
+                              "org.freedesktop.DBus",
+                              "StartServiceByName",
+                              g_variant_new ("(su)", self->bus_name, 0),
+                              G_VARIANT_TYPE ("(u)"),
+                              G_DBUS_CALL_FLAGS_NONE,
+                              -1,
+                              NULL,
+                              indicator_ng_service_started,
+                              self);
+    }
+}
+
+static gboolean
+indicator_ng_initable_init (GInitable     *initable,
+                            GCancellable  *cancellable,
+                            GError       **error)
+{
+  IndicatorNg *self = INDICATOR_NG (initable);
+  GKeyFile *keyfile;
+
+  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;
+
+  if (!(self->bus_name = g_key_file_get_string (keyfile, "Indicator Service", "BusName", error)))
+    goto out;
+
+  if (!(self->object_path = g_key_file_get_string (keyfile, "Indicator Service", "ObjectPath", error)))
+    goto out;
+
+  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);
+
+out:
+  g_key_file_free (keyfile);
+
+  return self->name_watch_id > 0;
+}
+
+static void
+indicator_ng_class_init (IndicatorNgClass *class)
+{
+  GObjectClass *object_class = G_OBJECT_CLASS (class);
+  IndicatorObjectClass *io_class = INDICATOR_OBJECT_CLASS (class);
+
+  object_class->get_property = indicator_ng_get_property;
+  object_class->set_property = indicator_ng_set_property;
+  object_class->dispose = indicator_ng_dispose;
+  object_class->finalize = indicator_ng_finalize;
+
+  io_class->get_entries = indicator_ng_get_entries;
+
+  properties[PROP_SERVICE_FILE] = g_param_spec_string ("service-file",
+                                                       "Service file",
+                                                       "Path of the service file",
+                                                       NULL,
+                                                       G_PARAM_READWRITE |
+                                                       G_PARAM_CONSTRUCT_ONLY |
+                                                       G_PARAM_STATIC_STRINGS);
+
+  properties[PROP_PROFILE] = g_param_spec_string ("profile",
+                                                  "Profile",
+                                                  "Indicator profile",
+                                                  "desktop",
+                                                  G_PARAM_READWRITE |
+                                                  G_PARAM_CONSTRUCT_ONLY |
+                                                  G_PARAM_STATIC_STRINGS);
+
+  g_object_class_install_properties(object_class, N_PROPERTIES, properties);
+}
+
+static void
+indicator_ng_initable_iface_init (GInitableIface *initable)
+{
+  initable->init = indicator_ng_initable_init;
+}
+
+static void
+indicator_ng_init (IndicatorNg *self)
+{
+  self->entry.menu = g_object_ref_sink (gtk_menu_new ());
+
+  /* work around IndicatorObject's warning that the accessible
+   * description is missing. We never set it on construction, but when
+   * the menu model has arrived on the bus.
+   */
+  self->accessible_desc = g_strdup ("");
+  self->entry.accessible_desc = self->accessible_desc;
+
+  indicator_object_set_visible (INDICATOR_OBJECT (self), FALSE);
+}
+
+IndicatorNg *
+indicator_ng_new (const gchar  *service_file,
+                  GError      **error)
+{
+  return g_initable_new (INDICATOR_TYPE_NG, NULL, error,
+                         "service-file", service_file,
+                         NULL);
+}
+
+IndicatorNg *
+indicator_ng_new_for_profile (const gchar  *service_file,
+                              const gchar  *profile,
+                              GError      **error)
+{
+  return g_initable_new (INDICATOR_TYPE_NG, NULL, error,
+                         "service-file", service_file,
+                         "profile", profile,
+                         NULL);
+}
+
+const gchar *
+indicator_ng_get_service_file (IndicatorNg *self)
+{
+  g_return_val_if_fail (INDICATOR_IS_NG (self), NULL);
+
+  return self->service_file;
+}
+
+const gchar *
+indicator_ng_get_profile (IndicatorNg *self)
+{
+  g_return_val_if_fail (INDICATOR_IS_NG (self), NULL);
+
+  return self->profile;
+}

=== added file 'libindicator/indicator-ng.h'
--- libindicator/indicator-ng.h	1970-01-01 00:00:00 +0000
+++ libindicator/indicator-ng.h	2013-02-14 21:19:21 +0000
@@ -0,0 +1,48 @@
+/*
+ * Copyright 2013 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 3, as published
+ * by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranties of
+ * MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
+ * PURPOSE.  See the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authors:
+ *     Lars Uebernickel <[e-mail]>
+ */
+
+#ifndef __INDICATOR_NG_H__
+#define __INDICATOR_NG_H__
+
+#include "indicator-object.h"
+
+#define INDICATOR_TYPE_NG            (indicator_ng_get_type ())
+#define INDICATOR_NG(obj)            (G_TYPE_CHECK_INSTANCE_CAST ((obj), INDICATOR_TYPE_NG, IndicatorNg))
+#define INDICATOR_NG_CLASS(klass)    (G_TYPE_CHECK_CLASS_CAST ((klass), INDICATOR_TYPE_NG, IndicatorNgClass))
+#define INDICATOR_IS_NG(obj)         (G_TYPE_CHECK_INSTANCE_TYPE ((obj), INDICATOR_TYPE_NG))
+#define INDICATOR_IS_NG_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass), INDICATOR_TYPE_NG))
+#define INDICATOR_NG_GET_CLASS(obj)  (G_TYPE_INSTANCE_GET_CLASS ((obj), INDICATOR_TYPE_NG, IndicatorNgClass))
+
+typedef struct _IndicatorNg   IndicatorNg;
+typedef IndicatorObjectClass IndicatorNgClass;
+
+GType              indicator_ng_get_type            (void);
+
+IndicatorNg *      indicator_ng_new                 (const gchar  *service_file,
+                                                     GError      **error);
+
+IndicatorNg *      indicator_ng_new_for_profile     (const gchar  *service_file,
+                                                     const gchar  *profile,
+                                                     GError      **error);
+
+const gchar *      indicator_ng_get_service_file    (IndicatorNg *indicator);
+
+const gchar *      indicator_ng_get_profile         (IndicatorNg *indicator);
+
+#endif

=== modified file 'libindicator/indicator-object.c'
--- libindicator/indicator-object.c	2012-09-19 20:00:57 +0000
+++ libindicator/indicator-object.c	2013-02-14 21:19:21 +0000
@@ -935,3 +935,10 @@
         }
 }
 
+gboolean
+indicator_object_entry_is_visible (IndicatorObject * io, IndicatorObjectEntry * entry)
+{
+	g_return_val_if_fail (INDICATOR_IS_OBJECT (io), FALSE);
+
+	return entry_get_private (io, entry)->visibility == ENTRY_VISIBLE;
+}

=== modified file 'libindicator/indicator-object.h'
--- libindicator/indicator-object.h	2012-02-23 11:28:01 +0000
+++ libindicator/indicator-object.h	2013-02-14 21:19:21 +0000
@@ -199,6 +199,7 @@
 guint   indicator_object_get_location (IndicatorObject * io, IndicatorObjectEntry * entry);
 guint   indicator_object_get_show_now (IndicatorObject * io, IndicatorObjectEntry * entry);
 void	indicator_object_set_visible (IndicatorObject * io, gboolean visible);
+gboolean indicator_object_entry_is_visible (IndicatorObject * io, IndicatorObjectEntry * entry);
 void    indicator_object_entry_activate (IndicatorObject * io, IndicatorObjectEntry * entry, guint timestamp);
 void    indicator_object_entry_activate_window (IndicatorObject * io, IndicatorObjectEntry * entry, guint windowid, guint timestamp);
 void    indicator_object_entry_close (IndicatorObject * io, IndicatorObjectEntry * entry, guint timestamp);

=== modified file 'tests/Makefile.am'
--- tests/Makefile.am	2012-06-18 18:42:53 +0000
+++ tests/Makefile.am	2013-02-14 21:19:21 +0000
@@ -26,7 +26,8 @@
 	session.conf.in \
 	service-manager-connect.service.in \
 	service-version-bad.service.in \
-	service-version-good.service.in
+	service-version-good.service.in \
+	com.canonical.indicator.test.service.in
 
 #############################
 # Test Loader
@@ -466,3 +467,43 @@
 
 DISTCLEANFILES += $(XML_REPORT) $(HTML_REPORT)
 
+#############################
+# Indicator-ng
+#############################
+
+if USE_GTK3
+
+com.canonical.indicator.test.service: com.canonical.indicator.test.service.in Makefile.am
+	sed -e "s|\@builddir\@|$(abspath $(builddir))|" $< > $@
+
+check_PROGRAMS += test-indicator-ng
+
+test_indicator_ng_SOURCES = test-indicator-ng.c
+
+test_indicator_ng_CFLAGS = \
+	$(LIBINDICATOR_CFLAGS) \
+	-I$(top_srcdir) \
+	-DSRCDIR="\"$(srcdir)\"" \
+	-DBUILD_DIR="\"$(abs_builddir)\"" \
+	-Wall
+
+test_indicator_ng_LDADD = \
+	$(LIBINDICATOR_LIBS) \
+	-L$(top_builddir)/libindicator/.libs \
+	$(INDICATOR_LIB)
+
+TESTS += test-indicator-ng
+DISTCLEANFILES += test-indicator-ng
+
+# Mock indicator service
+check_PROGRAMS += indicator-test-service
+
+indicator_test_service_SOURCES = indicator-test-service.c
+indicator_test_service_CFLAGS = $(LIBINDICATOR_CFLAGS)
+indicator_test_service_LDADD = $(LIBINDICATOR_LIBS)
+
+EXTRA_indicator_test_service_DEPENDENCIES = com.canonical.indicator.test.service
+
+DISTCLEANFILES += indicator-test-service com.canonical.indicator.test.service
+
+endif

=== added file 'tests/com.canonical.indicator.test.service.in'
--- tests/com.canonical.indicator.test.service.in	1970-01-01 00:00:00 +0000
+++ tests/com.canonical.indicator.test.service.in	2013-02-14 21:19:21 +0000
@@ -0,0 +1,3 @@
+[D-BUS Service]
+Name=com.canonical.indicator.test
+Exec=@builddir@/indicator-test-service

=== added file 'tests/com.canonical.test.indicator'
--- tests/com.canonical.test.indicator	1970-01-01 00:00:00 +0000
+++ tests/com.canonical.test.indicator	2013-02-14 21:19:21 +0000
@@ -0,0 +1,4 @@
+[Indicator Service]
+Name=indicator-test
+BusName=com.canonical.indicator.test
+ObjectPath=/com/canonical/indicator/test

=== added file 'tests/com.canonical.test.nosuchservice.indicator'
--- tests/com.canonical.test.nosuchservice.indicator	1970-01-01 00:00:00 +0000
+++ tests/com.canonical.test.nosuchservice.indicator	2013-02-14 21:19:21 +0000
@@ -0,0 +1,4 @@
+[Indicator Service]
+Name=indicator-test
+BusName=com.canonical.indicator.test.nosuchservice
+ObjectPath=/com/canonical/indicator/test

=== added file 'tests/indicator-test-service.c'
--- tests/indicator-test-service.c	1970-01-01 00:00:00 +0000
+++ tests/indicator-test-service.c	2013-02-14 21:19:21 +0000
@@ -0,0 +1,108 @@
+
+#include <gio/gio.h>
+
+typedef struct
+{
+  GSimpleActionGroup *actions;
+  GMenu *menu;
+
+  guint actions_export_id;
+  guint menu_export_id;
+} IndicatorTestService;
+
+static void
+bus_acquired (GDBusConnection *connection,
+              const gchar     *name,
+              gpointer         user_data)
+{
+  IndicatorTestService *indicator = user_data;
+  GError *error = NULL;
+
+  indicator->actions_export_id = g_dbus_connection_export_action_group (connection,
+                                                                        "/com/canonical/indicator/test",
+                                                                        G_ACTION_GROUP (indicator->actions),
+                                                                        &error);
+  if (indicator->actions_export_id == 0)
+    {
+      g_warning ("cannot export action group: %s", error->message);
+      g_error_free (error);
+      return;
+    }
+
+  indicator->menu_export_id = g_dbus_connection_export_menu_model (connection,
+                                                                   "/com/canonical/indicator/test/desktop",
+                                                                   G_MENU_MODEL (indicator->menu),
+                                                                   &error);
+  if (indicator->menu_export_id == 0)
+    {
+      g_warning ("cannot export menu: %s", error->message);
+      g_error_free (error);
+      return;
+    }
+}
+
+static void
+name_lost (GDBusConnection *connection,
+           const gchar     *name,
+           gpointer         user_data)
+{
+  IndicatorTestService *indicator = user_data;
+
+  if (indicator->actions_export_id)
+    g_dbus_connection_unexport_action_group (connection, indicator->actions_export_id);
+
+  if (indicator->menu_export_id)
+    g_dbus_connection_unexport_menu_model (connection, indicator->menu_export_id);
+}
+
+static void
+activate_show (GSimpleAction *action,
+               GVariant      *parameter,
+               gpointer       user_data)
+{
+  g_message ("showing");
+}
+
+int
+main (int argc, char **argv)
+{
+  IndicatorTestService indicator = { 0 };
+  GMenuItem *item;
+  GMenu *submenu;
+  GActionEntry entries[] = {
+    { "_header", NULL, NULL, "('Test', 'indicator-test', 'Test indicator', true)", NULL },
+    { "show", activate_show, NULL, NULL, NULL }
+  };
+  GMainLoop *loop;
+
+  indicator.actions = g_simple_action_group_new ();
+  g_simple_action_group_add_entries (indicator.actions, entries, G_N_ELEMENTS (entries), NULL);
+
+  submenu = g_menu_new ();
+  g_menu_append (submenu, "Show", "indicator.show");
+  item = g_menu_item_new (NULL, "indicator._header");
+  g_menu_item_set_attribute (item, "x-canonical-type", "s", "com.canonical.indicator.root");
+  g_menu_item_set_submenu (item, G_MENU_MODEL (submenu));
+  indicator.menu = g_menu_new ();
+  g_menu_append_item (indicator.menu, item);
+
+  g_bus_own_name (G_BUS_TYPE_SESSION,
+                  "com.canonical.indicator.test",
+                  G_BUS_NAME_OWNER_FLAGS_NONE,
+                  bus_acquired,
+                  NULL,
+                  name_lost,
+                  &indicator,
+                  NULL);
+
+  loop = g_main_loop_new (NULL, FALSE);
+  g_main_loop_run (loop);
+
+  g_object_unref (submenu);
+  g_object_unref (item);
+  g_object_unref (indicator.actions);
+  g_object_unref (indicator.menu);
+  g_object_unref (loop);
+
+  return 0;
+}

=== added file 'tests/test-indicator-ng.c'
--- tests/test-indicator-ng.c	1970-01-01 00:00:00 +0000
+++ tests/test-indicator-ng.c	2013-02-14 21:19:21 +0000
@@ -0,0 +1,167 @@
+
+#include <libindicator/indicator-ng.h>
+
+static void
+indicator_ng_test_func (gconstpointer user_data)
+{
+  GTestFunc test_func = user_data;
+  GTestDBus *bus;
+
+  bus = g_test_dbus_new (G_TEST_DBUS_NONE);
+  g_test_dbus_add_service_dir (bus, BUILD_DIR);
+  g_test_dbus_up (bus);
+
+  test_func ();
+
+  g_test_dbus_down (bus);
+  g_object_unref (bus);
+}
+
+#define indicator_ng_test_add(name, test_func) \
+  g_test_add_data_func ("/indicator-ng/" name, test_func, indicator_ng_test_func)
+
+static gboolean
+stop_main_loop (gpointer user_data)
+{
+  GMainLoop *loop = user_data;
+
+  g_main_loop_quit (loop);
+
+  return FALSE;
+}
+
+static void
+test_non_existing (void)
+{
+  IndicatorNg *indicator;
+  GError *error = NULL;
+
+  indicator = indicator_ng_new (SRCDIR "/com.canonical.does.not.exist.indicator", &error);
+  g_assert (indicator == NULL);
+  g_assert_error (error, G_FILE_ERROR, G_FILE_ERROR_NOENT);
+
+  g_clear_error (&error);
+}
+
+static void
+test_instantiation (void)
+{
+  IndicatorNg *indicator;
+  GError *error = NULL;
+  GMainLoop *loop;
+
+  indicator = indicator_ng_new (SRCDIR "/com.canonical.test.nosuchservice.indicator", &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_profile (indicator), ==, "desktop");
+
+  {
+    gchar *service_file;
+    gchar *profile;
+
+    g_object_get (indicator, "service-file", &service_file,
+                             "profile", &profile,
+                             NULL);
+
+    g_assert_cmpstr (service_file, ==, SRCDIR "/com.canonical.test.nosuchservice.indicator");
+    g_assert_cmpstr (profile, ==, "desktop");
+
+    g_free (service_file);
+    g_free (profile);
+  }
+
+  loop = g_main_loop_new (NULL, FALSE);
+  g_timeout_add (200, stop_main_loop, loop);
+  g_main_loop_run (loop);
+
+  /* no such service, so there shouldn't be any indicators */
+  g_assert (indicator_object_get_entries (INDICATOR_OBJECT (indicator)) == NULL);
+
+  g_main_loop_unref (loop);
+  g_object_unref (indicator);
+}
+
+static void
+test_instantiation_with_profile (void)
+{
+  IndicatorNg *indicator;
+  GError *error = NULL;
+
+  indicator = indicator_ng_new_for_profile (SRCDIR "/com.canonical.test.indicator", "greeter", &error);
+  g_assert (indicator);
+  g_assert (error == NULL);
+
+  g_assert_cmpstr (indicator_ng_get_profile (indicator), ==, "greeter");
+
+  g_object_unref (indicator);
+}
+
+static void
+test_menu (void)
+{
+  IndicatorNg *indicator;
+  GError *error = NULL;
+  GMainLoop *loop;
+  GList *entries;
+  IndicatorObjectEntry *entry;
+
+  indicator = indicator_ng_new (SRCDIR "/com.canonical.test.indicator", &error);
+  g_assert (indicator);
+  g_assert (error == NULL);
+
+  loop = g_main_loop_new (NULL, FALSE);
+  g_timeout_add (500, stop_main_loop, loop);
+  g_main_loop_run (loop);
+
+  entries = indicator_object_get_entries (INDICATOR_OBJECT (indicator));
+  g_assert_cmpint (g_list_length (entries), ==, 1);
+
+  entry = entries->data;
+  g_assert_cmpstr (entry->name_hint, ==, "indicator-test");
+  g_assert_cmpstr (entry->accessible_desc, ==, "Test indicator");
+  g_assert_cmpstr (gtk_label_get_label (entry->label), ==, "Test");
+  g_assert (gtk_image_get_storage_type (entry->image) == GTK_IMAGE_STOCK);
+  {
+    GList *children;
+    GtkMenuItem *item;
+
+    g_assert (entry->menu != NULL);
+
+    children = gtk_container_get_children (GTK_CONTAINER (entry->menu));
+    g_assert_cmpint (g_list_length (children), ==, 1);
+
+    item = children->data;
+    g_assert (GTK_IS_MENU_ITEM (item));
+    g_assert (gtk_widget_is_sensitive (GTK_WIDGET (item)));
+    g_assert_cmpstr (gtk_menu_item_get_label (item), ==, "Show");
+
+    g_list_free (children);
+  }
+
+  g_list_free (entries);
+  g_main_loop_unref (loop);
+  g_object_unref (indicator);
+}
+
+int
+main (int argc, char **argv)
+{
+  /* gvfs, dconf, and appmenu-gtk leak GDbusConnections, which confuses
+   * g_test_dbus_down.  Make sure we're not using any of those.
+   */
+  g_setenv ("GIO_USE_VFS", "local", TRUE);
+  g_setenv ("GSETTINGS_BACKEND", "memory", TRUE);
+  g_unsetenv ("UBUNTU_MENUPROXY");
+
+  g_test_init (&argc, &argv, NULL);
+  gtk_init (&argc, &argv);
+
+  indicator_ng_test_add ("non-existing", test_non_existing);
+  indicator_ng_test_add ("instantiation", test_instantiation);
+  indicator_ng_test_add ("instantiation-with-profile", test_instantiation_with_profile);
+  indicator_ng_test_add ("menu", test_menu);
+
+  return g_test_run ();
+}

=== modified file 'tools/indicator-loader.c'
--- tools/indicator-loader.c	2012-09-19 20:00:57 +0000
+++ tools/indicator-loader.c	2013-02-14 21:19:21 +0000
@@ -25,6 +25,10 @@
 #include <gtk/gtk.h>
 #include <libindicator/indicator-object.h>
 
+#if GTK_MAJOR_VERSION == 3
+#include <libindicator/indicator-ng.h>
+#endif
+
 static GHashTable * entry_to_menuitem = NULL;
 
 #define ENTRY_DATA_NAME "indicator-custom-entry-data"
@@ -129,14 +133,27 @@
 	g_debug("Looking at Module: %s", name);
 	g_return_val_if_fail(name != NULL, FALSE);
 
-	if (!g_str_has_suffix(name, G_MODULE_SUFFIX)) {
-		return FALSE;
-	}
-
 	g_debug("Loading Module: %s", name);
 
 	/* Build the object for the module */
-	IndicatorObject * io = indicator_object_new_from_file(name);
+	IndicatorObject *io;
+	if (g_str_has_suffix(name, G_MODULE_SUFFIX)) {
+		io = indicator_object_new_from_file(name);
+	}
+#if GTK_MAJOR_VERSION == 3
+	else if (g_str_has_suffix(name, ".indicator")) {
+		GError *error = NULL;
+
+		io = INDICATOR_OBJECT(indicator_ng_new(name, &error));
+		if (!io) {
+			g_warning ("could not load indicator from '%s': %s", name, error->message);
+			g_error_free (error);
+			return FALSE;
+		}
+	}
+#endif
+	else
+		return FALSE;
 
 	/* Connect to it's signals */
 	g_signal_connect(G_OBJECT(io), INDICATOR_OBJECT_SIGNAL_ENTRY_ADDED,   G_CALLBACK(entry_added),    menu);

=== added file 'trim-lcov.py'
--- trim-lcov.py	1970-01-01 00:00:00 +0000
+++ trim-lcov.py	2013-02-14 21:19:21 +0000
@@ -0,0 +1,69 @@
+#!/usr/bin/python
+
+# Copyright 2013 Canonical Ltd.
+#
+# This library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# version 3.0 as published by the Free Software Foundation.
+#
+# This library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License version 3.0 for more details.
+#
+# You should have received a copy of the GNU General Public
+# License along with this library. If not, see
+# <http://www.gnu.org/licenses/>.
+#
+# Author: Ryan Lortie <[e-mail]>
+
+
+# This script removes branch and/or line coverage data for lines that
+# contain a particular substring.
+#
+# In the interest of "fairness" it removes all branch or coverage data
+# when a match is found -- not just negative data.  It is therefore
+# likely that running this script will actually reduce the total number
+# of lines and branches that are marked as covered (in absolute terms).
+#
+# This script intentionally avoids checking for errors.  Any exceptions
+# will trigger make to fail.
+
+import sys
+
+line_suppress = ['g_assert_not_reached']
+branch_suppress = ['g_assert', 'g_return_if_fail', 'g_return_val_if_fail', 'G_DEFINE_TYPE']
+
+def check_suppress(suppressions, source, data):
+    line, _, rest = data.partition(',')
+    line = int(line) - 1
+
+    assert line < len(source)
+
+    for suppression in suppressions:
+        if suppression in source[line]:
+            return True
+
+    return False
+
+source = []
+for line in sys.stdin:
+    line = line[:-1]
+
+    keyword, _, rest = line.partition(':')
+
+    # Source file
+    if keyword == 'SF':
+        source = file(rest).readlines()
+
+    # Branch coverage data
+    elif keyword == 'BRDA':
+        if check_suppress(branch_suppress, source, rest):
+            continue
+
+    # Line coverage data
+    elif keyword == 'DA':
+        if check_suppress(line_suppress, source, rest):
+            continue
+
+    print line