Merge lp:~3v1n0/unity/indicators-p into lp:unity

[Marco Trevisan (Treviño)]

Updated unity-panel-service and libunity-core against the libindicator API changes

Updated unity-panel-service and libunity-core against the libindicator API changes

Supporting the new libindicator API into unity-panel-service and exposing them in the higher level wrapper library for future usage.
Doing this, I've performed some cleanup of the code, and in particular I've:
 - Introduced the new AppmenuIndicator class to handle the appmenu indicator
   (typing it differently allows us to remove the type-checking based only on the
   indicator name, this is something we should keep at lower level).
 - Fixed an issue in Indicator class, that didn't disconnect from signals of the removed
   entries
 - Removed the IsUnused / MarkUnused methods from IndicatorEntry, since they were
   already deprecated since last cycle and actually useless. The visible() method can
   now be used to perform a similar check.
 - Removed the sscanf based matching from the panel-service.

On the unity-panel-service side I've also fixed two issues:
 - When the menus are open now they are moved to the right position if their
   entry has been moved (this happens when scrolling on the language selector).
 - It is now synced with the compiz gsetting key used to define which keybinding
   must be used to open the menus (so now, changing that value applies to both
   opening and closing the indicators, not only to the open action).

On the Tests side, I've re-enabled and updated the panel-service unit-test, that now is working again, plus I've added new tests for the Indicator and AppmenuIndicator classes and updated the tests for IndicatorEntry against the new API.
Some more tests are covered on the lp:~3v1n0/unity/indicators-tests dependent branch.

[Thomi Richards (thomir-deactivatedaccount)]

Hi,

202 + // We have to do this because on certain systems X won't have time to
203 + // respond to our request for XUngrabPointer and this will cause the
204 + // menu not to show
205 + auto data = new ShowEntryData();

Can we use a smart pointer for this please? Once you use a smart pointer you won't need this:

279 + delete data;

...and there's a larger design issue around how we create Indicator instances. This code:

671 - Indicator::Ptr indicator(new Indicator(name));
672 + Indicator* indptr;
673 +
674 + if (name == "libappmenu.so")
675 + indptr = new AppmenuIndicator(name);
676 + else
677 + indptr = new Indicator(name);
678 +
679 + Indicator::Ptr indicator(indptr);

Makes me uneasy - especially this bit:

685 +
686 + if (indicator->IsAppmenu())
687 + {
688 + AppmenuIndicator *appmenu = dynamic_cast<AppmenuIndicator*>(indicator.get());
689 +
690 + if (appmenu)
691 + appmenu->on_show_appmenu.connect(sigc::mem_fun(owner_, &Indicators::OnShowAppMenu));
692 + }
693 +

Why are we using a boolean member variable to encode class type? Maybe I'm missing something, but why not just do this:

AppmenuIndicator *appmenu = dynamic_cast<AppmenuIndicator*>(indicator.get());

if (appmenu)
    appmenu->on_show_appmenu.connect(sigc::mem_fun(owner_, &Indicators::OnShowAppMenu));

..or even better, hook up the signal for all indicators, and have child classes that don't need the signal simply not use it.

This is wrong:

818 + return proxy_->id().c_str();

The method returns a std::string, so you don't need the ".c_str" on the end.

These:

1127 +#define COMPIZ_OPTIONS_PATH "/apps/compiz-1/plugins/unityshell/screen0/options"
1128 +#define MENU_TOGGLE_KEYBINDING_PATH COMPIZ_OPTIONS_PATH"/panel_first_menu"

Don't need to be #defines - nor do the three #defines right above them.

In panel-service.c, I'm 99% sure there's a memory leak on line 829, where we do this:

  gchar *entry_id = get_indicator_entry_id_by_entry (entry);
  g_hash_table_insert (self->priv->id2entry_hash, entry_id, entry);

entry_id needs to be freed.

The points above need to be fixed. You should also get someone else to look at this (someone who understands the panel code).

Cheers,

review: Needs Fixing

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

> 202 + // We have to do this because on certain systems X won't have time
> to
> 203 + // respond to our request for XUngrabPointer and this will cause the
> 204 + // menu not to show
> 205 + auto data = new ShowEntryData();
>
> Can we use a smart pointer for this please? Once you use a smart pointer you
> won't need this:
>
> 279 + delete data;

Mh, I didn't used these, not to change too much the pre-existent code.

> ...and there's a larger design issue around how we create Indicator instances.
> This code:
>
> 671 - Indicator::Ptr indicator(new Indicator(name));
> 672 + Indicator* indptr;
> 673 +
> 674 + if (name == "libappmenu.so")
> 675 + indptr = new AppmenuIndicator(name);
> 676 + else
> 677 + indptr = new Indicator(name);
> 678 +
> 679 + Indicator::Ptr indicator(indptr);
>
> Makes me uneasy - especially this bit:
>
> 685 +
> 686 + if (indicator->IsAppmenu())
> 687 + {
> 688 + AppmenuIndicator *appmenu =
> dynamic_cast<AppmenuIndicator*>(indicator.get());
> 689 +
> 690 + if (appmenu)
> 691 + appmenu->on_show_appmenu.connect(sigc::mem_fun(owner_,
> &Indicators::OnShowAppMenu));
> 692 + }
> 693 +
>
> Why are we using a boolean member variable to encode class type? Maybe I'm
> missing something, but why not just do this:
>
> AppmenuIndicator *appmenu = dynamic_cast<AppmenuIndicator*>(indicator.get());

Using that bool check is faster than doing an unneeded dymamic_cast (actually we could even use just a static_cast there). And currently we only have two kinds of indicators that we must care of, standard indicators and appmenu indicators. Maybe we could use a GetType or something...

> if (appmenu)
> appmenu->on_show_appmenu.connect(sigc::mem_fun(owner_,
> &Indicators::OnShowAppMenu));
>
> ..or even better, hook up the signal for all indicators, and have child
> classes that don't need the signal simply not use it.

Mh, I don't like too much the idea of adding a signal that is not related to all the indicators to the Indicator class. Also because that class will be filled with some other stuff soon, and I don't want to repeat them in the base class...

> This is wrong:
>
> 818 + return proxy_->id().c_str();
>
> The method returns a std::string, so you don't need the ".c_str" on the end.

True. The mistake was already there, but I didn't notice it. Thanks.

> These:
>
> 1127 +#define COMPIZ_OPTIONS_PATH
> "/apps/compiz-1/plugins/unityshell/screen0/options"
> 1128 +#define MENU_TOGGLE_KEYBINDING_PATH
> COMPIZ_OPTIONS_PATH"/panel_first_menu"
>
> Don't need to be #defines - nor do the three #defines right above them.

Mh... In a C scope they don't look so bad... Also I should repeat some strings without them.

> In panel-service.c, I'm 99% sure there's a memory leak on line 829, where we
> do this:
>
> gchar *entry_id = get_indicator_entry_id_by_entry (entry);
> g_hash_table_insert (self->priv->id2entry_hash, entry_id, entry);
>
> entry_id needs to be freed.

Not really: http://developer.gnome.org/glib/2.29/glib-Hash-Tables.html#g-hash-table-insert :)

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

> > This is wrong:
> >
> > 818 + return proxy_->id().c_str();
> >
> > The method returns a std::string, so you don't need the ".c_str" on the end.
>
> True. The mistake was already there, but I didn't notice it. Thanks.

Ah, actually I already fixed in another branch... I put the fix here too.

[Andrea Azzarone (azzar1)]

671 - Indicator::Ptr indicator(new Indicator(name));
672 + Indicator* indptr;
673 +
674 + if (name == "libappmenu.so")
675 + indptr = new AppmenuIndicator(name);
676 + else
677 + indptr = new Indicator(name);
678 +
679 + Indicator::Ptr indicator(indptr);

It's a kind of object factory, for me it's ok.

685 +
686 + if (indicator->IsAppmenu())
687 + {
688 + AppmenuIndicator *appmenu = dynamic_cast<AppmenuIndicator*>(indicator.get());
689 +
690 + if (appmenu)
691 + appmenu->on_show_appmenu.connect(sigc::mem_fun(owner_, &Indicators::OnShowAppMenu));
692 + }
693 +

If you can remove this please do... :) Otherwise it's ok for me.

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

> 685 +
> 686 + if (indicator->IsAppmenu())
> 687 + {
> 688 + AppmenuIndicator *appmenu =
> dynamic_cast<AppmenuIndicator*>(indicator.get());
> 689 +
> 690 + if (appmenu)
> 691 + appmenu->on_show_appmenu.connect(sigc::mem_fun(owner_,
> &Indicators::OnShowAppMenu));
> 692 + }
> 693 +
>
> If you can remove this please do... :) Otherwise it's ok for me.

Yes, done. It was possible to get that before as well, but I didn't like to connect to the appmenu specific signal before than the others :)

[Michal Hruby (mhr3)]

183 data->proxy = proxy_;

Makes me sad that it doesn't use the dbus proxy class from unity core.

194 + g_idle_add_full (G_PRIORITY_DEFAULT, (GSourceFunc) send_show_entry, data, NULL);

Moreover this is pretty dangerous, it should at least grab a ref on the proxy. Alternatively such timeouts should be removed in the destructor.

1114 +#include <fcntl.h>

I hope this isn't needed?

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

> 183 data->proxy = proxy_;
>
> Makes me sad that it doesn't use the dbus proxy class from unity core.

Yes, you're right.
Unfortunately that code was written before it was available I guess, so I just didn't rewrite the whole thing.
I see if that's quickly feasible ;)

> 194 + g_idle_add_full (G_PRIORITY_DEFAULT, (GSourceFunc) send_show_entry,
> data, NULL);
>
> Moreover this is pretty dangerous, it should at least grab a ref on the proxy.
> Alternatively such timeouts should be removed in the destructor.

Sure, again... I just followed the old school! :)

> 1114 +#include <fcntl.h>
>
> I hope this isn't needed?

Yes, no more needed. /me cleaning it.

[Michal Hruby (mhr3)]

> DBusIndicators: use glib::Object to handle the dbus proxy, so we have ref-counting for free

So much nicer, thx!

review: Approve

[Andrea Azzarone (azzar1)]

review: Approve

[Tim Penhey (thumper)]

Gah... I was wanting to work out how to remove the IsAppmenu bit, but since the cure is worse than the symptoms, I'm prepared to accept it.

review: Approve

[Thomi Richards (thomir-deactivatedaccount)]

review: Approve

[Unity Merger (unity-merger)]

Attempt to merge into lp:unity failed due to conflicts:

text conflict in CMakeLists.txt
text conflict in plugins/unityshell/src/PanelIndicatorEntryView.cpp

=== modified file 'CMakeLists.txt'
--- CMakeLists.txt	2012-03-13 11:57:33 +0000
+++ CMakeLists.txt	2012-03-20 10:02:20 +0000
@@ -122,7 +122,7 @@
 #
 # Compiz Plugins
 #
-set (UNITY_PLUGIN_DEPS "compiz;nux-2.0>=2.0.0;libbamf3;dee-1.0;gio-2.0;gio-unix-2.0;dbusmenu-glib-0.4;x11;libstartup-notification-1.0;gthread-2.0;indicator3-0.4;atk;unity-misc>=0.4.0;gconf-2.0;libutouch-geis;gtk+-3.0>=3.1;sigc++-2.0;json-glib-1.0;libnotify;gnome-desktop-3.0;gdu;xfixes")
+set (UNITY_PLUGIN_DEPS "compiz;nux-2.0>=2.0.0;libbamf3;dee-1.0;gio-2.0;gio-unix-2.0;dbusmenu-glib-0.4;x11;libstartup-notification-1.0;gthread-2.0;indicator3-0.4>=0.4.90;atk;unity-misc>=0.4.0;gconf-2.0;libutouch-geis;gtk+-3.0>=3.1;sigc++-2.0;json-glib-1.0;libnotify;gnome-desktop-3.0;gdu;xfixes")
 
 add_subdirectory(plugins/unityshell)
 add_subdirectory(plugins/gtkloader)

=== added file 'UnityCore/AppmenuIndicator.cpp'
--- UnityCore/AppmenuIndicator.cpp	1970-01-01 00:00:00 +0000
+++ UnityCore/AppmenuIndicator.cpp	2012-03-20 10:02:20 +0000
@@ -0,0 +1,37 @@
+// -*- Mode: C++; indent-tabs-mode: nil; tab-width: 2 -*-
+/*
+ * Copyright (C) 2012 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 warranty of
+ * MERCHANTABILITY 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/>.
+ *
+ * Authored by: Marco Trevisan (Treviño) <[e-mail]>
+ */
+
+#include "AppmenuIndicator.h"
+
+namespace unity
+{
+namespace indicator
+{
+
+AppmenuIndicator::AppmenuIndicator(std::string const& name)
+  : Indicator(name)
+{}
+
+void AppmenuIndicator::ShowAppmenu(unsigned int xid, int x, int y, unsigned int timestamp) const
+{
+  on_show_appmenu.emit(xid, x, y, timestamp);
+}
+
+} // namespace indicator
+} // namespace unity

=== added file 'UnityCore/AppmenuIndicator.h'
--- UnityCore/AppmenuIndicator.h	1970-01-01 00:00:00 +0000
+++ UnityCore/AppmenuIndicator.h	2012-03-20 10:02:20 +0000
@@ -0,0 +1,45 @@
+// -*- Mode: C++; indent-tabs-mode: nil; tab-width: 2 -*-
+/*
+ * Copyright (C) 2012 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 warranty of
+ * MERCHANTABILITY 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/>.
+ *
+ * Authored by: Marco Trevisan (Treviño) <[e-mail]>
+ */
+
+#ifndef UNITY_APPMENU_INDICATOR_H
+#define UNITY_APPMENU_INDICATOR_H
+
+#include "Indicator.h"
+
+namespace unity
+{
+namespace indicator
+{
+
+class AppmenuIndicator : public Indicator
+{
+public:
+  AppmenuIndicator(std::string const& name);
+
+  virtual bool IsAppmenu() const { return true; }
+
+  void ShowAppmenu(unsigned int xid, int x, int y, unsigned int timestamp) const;
+
+  sigc::signal<void, unsigned int, int, int, unsigned int> on_show_appmenu;
+};
+
+}
+}
+
+#endif // UNITY_APPMENU_INDICATOR_H

=== modified file 'UnityCore/CMakeLists.txt'
--- UnityCore/CMakeLists.txt	2012-02-07 07:42:12 +0000
+++ UnityCore/CMakeLists.txt	2012-03-20 10:02:20 +0000
@@ -8,6 +8,7 @@
 #
 set (CORE_HEADERS
      ApplicationPreview.h
+     AppmenuIndicator.h
      Categories.h
      Category.h
      CheckOptionFilter.h
@@ -44,6 +45,7 @@
 
 set (CORE_SOURCES
      ApplicationPreview.cpp
+     AppmenuIndicator.cpp
      Categories.cpp
      Category.cpp
      CheckOptionFilter.cpp

=== modified file 'UnityCore/DBusIndicators.cpp'
--- UnityCore/DBusIndicators.cpp	2012-02-09 10:33:04 +0000
+++ UnityCore/DBusIndicators.cpp	2012-03-20 10:02:20 +0000
@@ -79,12 +79,13 @@
 
 struct ShowEntryData
 {
-  GDBusProxy* proxy;
+  glib::Object<GDBusProxy> proxy;
   std::string entry_id;
+  guint xid;
   int x;
   int y;
+  guint button;
   guint timestamp;
-  guint32 button;
 };
 
 bool run_local_panel_service();
@@ -95,6 +96,7 @@
 void on_sync_ready_cb(GObject* source, GAsyncResult* res, gpointer data);
 
 bool send_show_entry(ShowEntryData* data);
+bool send_show_appmenu(ShowEntryData* data);
 
 } // anonymous namespace
 
@@ -105,7 +107,6 @@
 {
 public:
   Impl(DBusIndicators* owner);
-  ~Impl();
 
   void OnRemoteProxyReady(GDBusProxy* proxy);
   void Reconnect();
@@ -121,17 +122,20 @@
                              GVariant* parameters);
 
   virtual void OnEntryScroll(std::string const& entry_id, int delta);
-  virtual void OnEntryShowMenu(std::string const& entry_id,
-                               int x, int y, int timestamp, int button);
+  virtual void OnEntryShowMenu(std::string const& entry_id, unsigned int xid,
+                               int x, int y, unsigned int button,
+                               unsigned int timestamp);
   virtual void OnEntrySecondaryActivate(std::string const& entry_id,
                                         unsigned int timestamp);
+  virtual void OnShowAppMenu(unsigned int xid, int x, int y,
+                             unsigned int timestamp);
 
   std::string name() const;
   std::string owner_name() const;
   bool using_local_service() const;
 
   DBusIndicators* owner_;
-  GDBusProxy* proxy_;
+  glib::Object<GDBusProxy> proxy_;
   typedef std::vector<SyncDataPtr> PendingSyncs;
   PendingSyncs pending_syncs_;
 
@@ -143,19 +147,10 @@
 // Public Methods
 DBusIndicators::Impl::Impl(DBusIndicators* owner)
   : owner_(owner)
-  , proxy_(NULL)
 {
   Reconnect();
 }
 
-DBusIndicators::Impl::~Impl()
-{
-  if (G_IS_OBJECT(proxy_))
-  {
-    g_object_unref(proxy_);
-  }
-}
-
 void DBusIndicators::Impl::Reconnect()
 {
   g_spawn_command_line_sync("killall -9 unity-panel-service",
@@ -212,22 +207,43 @@
 
 
 void DBusIndicators::Impl::OnEntryShowMenu(std::string const& entry_id,
-                                           int x, int y, int timestamp, int button)
+                                           unsigned int xid, int x, int y,
+                                           unsigned int button,
+                                           unsigned int timestamp)
 {
-  owner_->on_entry_show_menu.emit(entry_id, x, y, timestamp, button);
+  owner_->on_entry_show_menu.emit(entry_id, xid, x, y, button, timestamp);
 
   // We have to do this because on certain systems X won't have time to
   // respond to our request for XUngrabPointer and this will cause the
   // menu not to show
-  ShowEntryData* data = new ShowEntryData();
+  auto data = new ShowEntryData();
   data->proxy = proxy_;
   data->entry_id = entry_id;
+  data->xid = xid;
   data->x = x;
   data->y = y;
-  data->timestamp = timestamp;
   data->button = button;
-
-  g_idle_add_full (G_PRIORITY_DEFAULT, (GSourceFunc)send_show_entry, data, NULL);
+  data->timestamp = timestamp;
+
+  g_idle_add_full (G_PRIORITY_DEFAULT, (GSourceFunc) send_show_entry, data, NULL);
+}
+
+void DBusIndicators::Impl::OnShowAppMenu(unsigned int xid, int x, int y,
+                                         unsigned int timestamp)
+{
+  owner_->on_show_appmenu.emit(xid, x, y, timestamp);
+
+  // We have to do this because on certain systems X won't have time to
+  // respond to our request for XUngrabPointer and this will cause the
+  // menu not to show
+  auto data = new ShowEntryData();
+  data->proxy = proxy_;
+  data->xid = xid;
+  data->x = x;
+  data->y = y;
+  data->timestamp = timestamp;
+
+  g_idle_add_full (G_PRIORITY_DEFAULT, (GSourceFunc) send_show_appmenu, data, NULL);
 }
 
 void DBusIndicators::Impl::OnEntrySecondaryActivate(std::string const& entry_id,
@@ -457,10 +473,13 @@
   std::string signal_name(signal_name_);
   if (signal_name == "EntryActivated")
   {
-    const char* entry_name = g_variant_get_string(g_variant_get_child_value(parameters, 0), NULL);
+    glib::String entry_name;
+    nux::Rect geo;
+    g_variant_get(parameters, "(s(iiuu))", &entry_name, &geo.x, &geo.y, &geo.width, &geo.height);
+
     if (entry_name)
     {
-      owner_->ActivateEntry(entry_name);
+      owner_->ActivateEntry(entry_name.Str(), geo);
     }
   }
   else if (signal_name == "EntryActivateRequest")
@@ -499,8 +518,7 @@
 
 DBusIndicators::DBusIndicators()
   : pimpl(new Impl(this))
-{
-}
+{}
 
 DBusIndicators::~DBusIndicators()
 {
@@ -519,9 +537,10 @@
 }
 
 void DBusIndicators::OnEntryShowMenu(std::string const& entry_id,
-                                     int x, int y, int timestamp, int button)
+                                     unsigned int xid, int x, int y,
+                                     unsigned int button, unsigned int timestamp)
 {
-  pimpl->OnEntryShowMenu(entry_id, x, y, timestamp, button);
+  pimpl->OnEntryShowMenu(entry_id, xid, x, y, button, timestamp);
 }
 
 void DBusIndicators::OnEntrySecondaryActivate(std::string const& entry_id,
@@ -530,6 +549,12 @@
   pimpl->OnEntrySecondaryActivate(entry_id, timestamp);
 }
 
+void DBusIndicators::OnShowAppMenu(unsigned int xid, int x, int y,
+                                   unsigned int timestamp)
+{
+  pimpl->OnShowAppMenu(xid, x, y, timestamp);
+}
+
 std::string DBusIndicators::name() const
 {
   return pimpl->name();
@@ -651,21 +676,45 @@
 bool send_show_entry(ShowEntryData* data)
 {
   g_return_val_if_fail(data != NULL, FALSE);
-  g_return_val_if_fail(G_IS_DBUS_PROXY(data->proxy), FALSE);
+  g_return_val_if_fail(G_IS_DBUS_PROXY(data->proxy.RawPtr()), FALSE);
 
   g_dbus_proxy_call(data->proxy,
                     "ShowEntry",
-                    g_variant_new("(suiii)",
+                    g_variant_new("(suiiuu)",
                                   data->entry_id.c_str(),
-                                  0,
-                                  data->x,
-                                  data->y,
-                                  data->button),
-                    G_DBUS_CALL_FLAGS_NONE,
-                    -1,
-                    NULL,
-                    NULL,
-                    NULL);
+                                  data->xid,
+                                  data->x,
+                                  data->y,
+                                  data->button,
+                                  data->timestamp),
+                    G_DBUS_CALL_FLAGS_NONE,
+                    -1,
+                    NULL,
+                    NULL,
+                    NULL);
+
+  delete data;
+  return FALSE;
+}
+
+bool send_show_appmenu(ShowEntryData* data)
+{
+  g_return_val_if_fail(data != NULL, FALSE);
+  g_return_val_if_fail(G_IS_DBUS_PROXY(data->proxy.RawPtr()), FALSE);
+
+  g_dbus_proxy_call(data->proxy,
+                    "ShowAppMenu",
+                    g_variant_new("(uiiu)",
+                                  data->xid,
+                                  data->x,
+                                  data->y,
+                                  data->timestamp),
+                    G_DBUS_CALL_FLAGS_NONE,
+                    -1,
+                    NULL,
+                    NULL,
+                    NULL);
+
   delete data;
   return FALSE;
 }

=== modified file 'UnityCore/DBusIndicators.h'
--- UnityCore/DBusIndicators.h	2011-07-29 07:15:54 +0000
+++ UnityCore/DBusIndicators.h	2012-03-20 10:02:20 +0000
@@ -42,10 +42,13 @@
                       EntryLocationMap const& locations);
 
   virtual void OnEntryScroll(std::string const& entry_id, int delta);
-  virtual void OnEntryShowMenu(std::string const& entry_id,
-                               int x, int y, int timestamp, int button);
+  virtual void OnEntryShowMenu(std::string const& entry_id, unsigned int xid,
+                               int x, int y, unsigned int button,
+                               unsigned int timestamp);
   virtual void OnEntrySecondaryActivate(std::string const& entry_id,
                                         unsigned int timestamp);
+  virtual void OnShowAppMenu(unsigned int xid, int x, int y,
+                             unsigned int timestamp);
 
   std::string name() const;
   std::string owner_name() const;

=== modified file 'UnityCore/Indicator.cpp'
--- UnityCore/Indicator.cpp	2011-09-06 00:19:51 +0000
+++ UnityCore/Indicator.cpp	2012-03-20 10:02:20 +0000
@@ -35,11 +35,18 @@
 
 Indicator::~Indicator()
 {
-  for (auto entry : entries_) {
-    on_entry_removed(entry->id());
-  }
+  for (auto entry : entries_)
+    on_entry_removed.emit(entry->id());
 
   entries_.clear();
+
+  for (auto it : entries_connections_)
+  {
+    for (auto conn : it.second)
+      conn.disconnect();
+
+    it.second.clear();
+  }
 }
 
 std::string const& Indicator::name() const
@@ -72,7 +79,12 @@
   }
 
   for (auto entry : to_rm) {
-    on_entry_removed(entry->id());
+    for (auto conn : entries_connections_[entry])
+      conn.disconnect();
+
+    entries_connections_[entry].clear();
+
+    on_entry_removed.emit(entry->id());
     entries_.remove(entry);
   }
 
@@ -82,10 +94,19 @@
       continue;
 
     // Just add the new entry, and connect it up.
-    new_entry->on_show_menu.connect(sigc::mem_fun(this, &Indicator::OnEntryShowMenu));
-    new_entry->on_secondary_activate.connect(sigc::mem_fun(this, &Indicator::OnEntrySecondaryActivate));
-    new_entry->on_scroll.connect(sigc::mem_fun(this, &Indicator::OnEntryScroll));
+    sigc::connection conn;
+    std::vector<sigc::connection>& new_entry_connections = entries_connections_[new_entry];
+
+    conn = new_entry->on_show_menu.connect(sigc::mem_fun(this, &Indicator::OnEntryShowMenu));
+    new_entry_connections.push_back(conn);
+
+    conn = new_entry->on_secondary_activate.connect(sigc::mem_fun(this, &Indicator::OnEntrySecondaryActivate));
+    new_entry_connections.push_back(conn);
+
+    conn = new_entry->on_scroll.connect(sigc::mem_fun(this, &Indicator::OnEntryScroll));
     entries_.push_back(new_entry);
+    new_entry_connections.push_back(conn);
+
     on_entry_added.emit(new_entry);
   }
 }
@@ -99,11 +120,10 @@
   return Entry::Ptr();
 }
 
-void Indicator::OnEntryShowMenu(std::string const& entry_id,
-                                int x, int y,
-                                int timestamp, int button)
+void Indicator::OnEntryShowMenu(std::string const& entry_id, unsigned int xid,
+                                int x, int y, unsigned int button, unsigned int timestamp)
 {
-  on_show_menu.emit(entry_id, x, y, timestamp, button);
+  on_show_menu.emit(entry_id, xid, x, y, button, timestamp);
 }
 
 void Indicator::OnEntrySecondaryActivate(std::string const& entry_id,

=== modified file 'UnityCore/Indicator.h'
--- UnityCore/Indicator.h	2011-09-06 00:19:51 +0000
+++ UnityCore/Indicator.h	2012-03-20 10:02:20 +0000
@@ -23,6 +23,7 @@
 
 #include <vector>
 #include <boost/utility.hpp>
+#include <sigc++/connection.h>
 
 #include "IndicatorEntry.h"
 
@@ -39,28 +40,31 @@
   typedef std::list<Entry::Ptr> Entries;
 
   Indicator(std::string const& name);
-  ~Indicator();
+  virtual ~Indicator();
 
   std::string const& name() const;
 
+  virtual bool IsAppmenu() const { return false; }
+
   void Sync(Entries const& new_entries);
   Entry::Ptr GetEntry(std::string const& entry_id) const;
   Entries GetEntries() const;
 
-  void OnEntryShowMenu(std::string const& entry_id, int x, int y, int timestamp, int button);
-  void OnEntrySecondaryActivate(std::string const& entry_id, unsigned int timestamp);
-  void OnEntryScroll(std::string const& entry_id, int delta);
-
   // Signals
   sigc::signal<void, Entry::Ptr const&> on_entry_added;
   sigc::signal<void, std::string const&> on_entry_removed;
-  sigc::signal<void, std::string const&, int, int, int, int> on_show_menu;
+  sigc::signal<void, std::string const&, unsigned int, int, int, unsigned int, unsigned int> on_show_menu;
   sigc::signal<void, std::string const&, unsigned int> on_secondary_activate;
   sigc::signal<void, std::string const&, int> on_scroll;
 
-private:
+protected:
+  void OnEntryShowMenu(std::string const& entry_id, unsigned int xid, int x, int y, unsigned int button, unsigned int timestamp);
+  void OnEntrySecondaryActivate(std::string const& entry_id, unsigned int timestamp);
+  void OnEntryScroll(std::string const& entry_id, int delta);
+
   Entries entries_;
   std::string name_;
+  std::map<Entry::Ptr, std::vector<sigc::connection>> entries_connections_;
 
   friend std::ostream& operator<<(std::ostream& out, Indicator const& i);
 };

=== modified file 'UnityCore/IndicatorEntry.cpp'
--- UnityCore/IndicatorEntry.cpp	2011-08-09 12:21:18 +0000
+++ UnityCore/IndicatorEntry.cpp	2012-03-20 10:02:20 +0000
@@ -27,8 +27,6 @@
 namespace indicator
 {
 
-std::string const Entry::UNUSED_ID("|");
-
 Entry::Entry(std::string const& id,
              std::string const &name_hint,
              std::string const& label,
@@ -104,6 +102,12 @@
   return priority_;
 }
 
+bool Entry::visible() const
+{
+  return ((label_visible_ && !label_.empty()) ||
+          (image_type_ != 0 && image_visible_ && !image_data_.empty()));
+}
+
 void Entry::set_active(bool active)
 {
   if (active_ == active)
@@ -114,6 +118,16 @@
   updated.emit();
 }
 
+void Entry::set_geometry(nux::Rect const& geometry)
+{
+  if (geometry_ == geometry)
+    return;
+
+  geometry_ = geometry;
+  geometry_changed.emit(geometry);
+  updated.emit();
+}
+
 void Entry::setLabel(std::string const& label, bool sensitive, bool visible)
 {
   if (label_ == label && sensitive == label_sensitive_ && visible == label_visible_)
@@ -154,6 +168,11 @@
   return active_;
 }
 
+nux::Rect const& Entry::geometry() const
+{
+  return geometry_;
+}
+
 Entry& Entry::operator=(Entry const& rhs)
 {
   id_ = rhs.id_;
@@ -186,28 +205,14 @@
   updated.emit();
 }
 
-void Entry::MarkUnused()
-{
-  id_ = UNUSED_ID;
-  name_hint_ = "";
-  label_ = "";
-  label_sensitive_ = false;
-  label_visible_ = false;
-  image_type_ = 0;
-  image_data_ = "";
-  image_sensitive_ = false;
-  image_visible_ = false;
-  updated.emit();
-}
-
-bool Entry::IsUnused() const
-{
-  return id_ == UNUSED_ID;
-}
-
-void Entry::ShowMenu(int x, int y, int timestamp, int button)
-{
-  on_show_menu.emit(id_, x, y, timestamp, button);
+void Entry::ShowMenu(int x, int y, unsigned int button, unsigned int timestamp)
+{
+  ShowMenu(0, x, y, button, timestamp);
+}
+
+void Entry::ShowMenu(unsigned int xid, int x, int y, unsigned int button, unsigned int timestamp)
+{
+  on_show_menu.emit(id_, xid, x, y, button, timestamp);
 }
 
 void Entry::SecondaryActivate(unsigned int timestamp)

=== modified file 'UnityCore/IndicatorEntry.h'
--- UnityCore/IndicatorEntry.h	2011-08-09 12:21:18 +0000
+++ UnityCore/IndicatorEntry.h	2012-03-20 10:02:20 +0000
@@ -28,7 +28,7 @@
 #include <sigc++/signal.h>
 #include <boost/shared_ptr.hpp>
 
-#include "NuxCore/Rect.h"
+#include <NuxCore/Rect.h>
 
 namespace unity
 {
@@ -43,7 +43,6 @@
 {
 public:
   typedef boost::shared_ptr<Entry> Ptr;
-  static std::string const UNUSED_ID;
 
   Entry(std::string const& id,
         std::string const& name_hint,
@@ -72,8 +71,13 @@
   void set_active(bool active);
   bool active() const;
 
+  void set_geometry(nux::Rect const& geometry);
+  nux::Rect const& geometry() const;
+
   int priority() const;
 
+  bool visible() const;
+
   /**
    * Whether this entry should be shown to the user.
    * Example uses: Application menubar items are only shown when the user
@@ -82,10 +86,8 @@
   bool show_now() const;
   void set_show_now(bool show_now);
 
-  void MarkUnused();
-  bool IsUnused() const;
-
-  void ShowMenu(int x, int y, int timestamp, int button);
+  void ShowMenu(int x, int y, unsigned int button, unsigned int timestamp);
+  void ShowMenu(unsigned int xid, int x, int y, unsigned int button, unsigned int timestamp);
   void SecondaryActivate(unsigned int timestamp);
   void Scroll(int delta);
 
@@ -96,9 +98,10 @@
   // Signals
   sigc::signal<void> updated;
   sigc::signal<void, bool> active_changed;
+  sigc::signal<void, nux::Rect const&> geometry_changed;
   sigc::signal<void, bool> show_now_changed;
 
-  sigc::signal<void, std::string const&, int, int, int, int> on_show_menu;
+  sigc::signal<void, std::string const&, unsigned int, int, int, unsigned int, unsigned int> on_show_menu;
   sigc::signal<void, std::string const&, unsigned int> on_secondary_activate;
   sigc::signal<void, std::string const&, int> on_scroll;
 
@@ -118,6 +121,8 @@
 
   bool show_now_;
   bool active_;
+
+  nux::Rect geometry_;
 };
 
 std::ostream& operator<<(std::ostream& out, Entry const& e);

=== modified file 'UnityCore/Indicators.cpp'
--- UnityCore/Indicators.cpp	2011-09-05 13:21:08 +0000
+++ UnityCore/Indicators.cpp	2012-03-20 10:02:20 +0000
@@ -17,8 +17,9 @@
  * Authored by: Neil Jagdish Patel <[e-mail]>
  *              Tim Penhey <[e-mail]>
  */
+
 #include "Indicators.h"
-
+#include "AppmenuIndicator.h"
 
 namespace unity
 {
@@ -35,7 +36,7 @@
     : owner_(owner)
   {}
 
-  void ActivateEntry(std::string const& entry_id);
+  void ActivateEntry(std::string const& entry_id, nux::Rect const& geometry);
   void SetEntryShowNow(std::string const& entry_id, bool show_now);
 
   IndicatorsList GetIndicators() const;
@@ -65,10 +66,10 @@
   delete pimpl;
 }
 
-void Indicators::ActivateEntry(std::string const& entry_id)
+void Indicators::ActivateEntry(std::string const& entry_id, nux::Rect const& geometry)
 {
-  pimpl->ActivateEntry(entry_id);
-  on_entry_activated.emit(entry_id);
+  pimpl->ActivateEntry(entry_id, geometry);
+  on_entry_activated.emit(entry_id, geometry);
 }
 
 void Indicators::SetEntryShowNow(std::string const& entry_id, bool show_now)
@@ -96,13 +97,19 @@
   return pimpl->RemoveIndicator(name);
 }
 
-void Indicators::Impl::ActivateEntry(std::string const& entry_id)
+void Indicators::Impl::ActivateEntry(std::string const& entry_id, nux::Rect const& geometry)
 {
   if (active_entry_)
+  {
+    active_entry_->set_geometry(nux::Rect());
     active_entry_->set_active(false);
+  }
+
   active_entry_ = GetEntry(entry_id);
+
   if (active_entry_)
   {
+    active_entry_->set_geometry(geometry);
     active_entry_->set_active(true);
   }
 }
@@ -131,12 +138,26 @@
 
 Indicator::Ptr Indicators::Impl::AddIndicator(std::string const& name)
 {
-  Indicator::Ptr indicator(new Indicator(name));
+  Indicator* indptr;
+
+  if (name == "libappmenu.so")
+  {
+    AppmenuIndicator *appmenu = new AppmenuIndicator(name);
+    appmenu->on_show_appmenu.connect(sigc::mem_fun(owner_, &Indicators::OnShowAppMenu));
+    indptr = appmenu;
+  }
+  else
+  {
+    indptr = new Indicator(name);
+  }
+
+  Indicator::Ptr indicator(indptr);
 
   // The owner Indicators class is interested in the other events.
   indicator->on_show_menu.connect(sigc::mem_fun(owner_, &Indicators::OnEntryShowMenu));
   indicator->on_secondary_activate.connect(sigc::mem_fun(owner_, &Indicators::OnEntrySecondaryActivate));
   indicator->on_scroll.connect(sigc::mem_fun(owner_, &Indicators::OnEntryScroll));
+
   indicators_[name] = indicator;
   owner_->on_object_added.emit(indicator);
 

=== modified file 'UnityCore/Indicators.h'
--- UnityCore/Indicators.h	2011-09-05 13:21:08 +0000
+++ UnityCore/Indicators.h	2012-03-20 10:02:20 +0000
@@ -1,6 +1,6 @@
 // -*- Mode: C++; indent-tabs-mode: nil; tab-width: 2 -*-
 /*
- * Copyright (C) 2010 Canonical Ltd
+ * Copyright (C) 2010-2012 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
@@ -43,7 +43,7 @@
   /**
    * internal
    */
-  void ActivateEntry(std::string const& entry_id);
+  void ActivateEntry(std::string const& entry_id, nux::Rect const& geometry);
 
   /**
    * internal
@@ -58,11 +58,22 @@
   /**
    * internal
    */
-  virtual void OnEntryShowMenu(std::string const& entry_id,
-                               int x, int y, int timestamp, int button) = 0;
+  virtual void OnEntryShowMenu(std::string const& entry_id, unsigned int xid,
+                               int x, int y, unsigned int button,
+                               unsigned int timestamp) = 0;
+
+  /**
+   * internal
+   */
   virtual void OnEntrySecondaryActivate(std::string const& entry_id,
                                         unsigned int timestamp) = 0;
 
+  /**
+   * internal
+   */
+  virtual void OnShowAppMenu(unsigned int xid, int x, int y,
+                             unsigned int timestamp) = 0;
+
   // Signals
   sigc::signal<void, Indicator::Ptr const&> on_object_added;
   sigc::signal<void, Indicator::Ptr const&> on_object_removed;
@@ -79,7 +90,7 @@
    * An entry just got activated. View needs to repaint it.
    * @param entry_id entry id
    */
-  sigc::signal<void, std::string const&> on_entry_activated;
+  sigc::signal<void, std::string const&, nux::Rect const&> on_entry_activated;
 
   /**
    * internal
@@ -89,12 +100,22 @@
   /**
    * The service is about to show a menu.
    * @param entry_id entry id
-   * @param x x coordinate
-   * @param y y coordinate
-   * @param timestamp current time
+   * @param xid window xid
+   * @param x coordinate
+   * @param y coordinate
    * @param button pressed button
-   */
-  sigc::signal<void, std::string const&, int, int, int, int> on_entry_show_menu;
+   * @param timestamp current time
+   */
+  sigc::signal<void, std::string const&, unsigned int, int, int, unsigned int, unsigned int> on_entry_show_menu;
+
+  /**
+   * The service is about to show an appmenu.
+   * @param xid window xid
+   * @param x coordinate
+   * @param y coordinate
+   * @param timestamp current time
+   */
+  sigc::signal<void, unsigned int, int, int, unsigned int> on_show_appmenu;
 
 protected:
   Indicator::Ptr GetIndicator(std::string const& name);

=== modified file 'plugins/unityshell/src/PanelIndicatorEntryView.cpp'
--- plugins/unityshell/src/PanelIndicatorEntryView.cpp	2012-03-16 01:48:22 +0000
+++ plugins/unityshell/src/PanelIndicatorEntryView.cpp	2012-03-20 10:02:20 +0000
@@ -110,8 +110,8 @@
 {
   proxy_->ShowMenu(GetAbsoluteX(),
                    GetAbsoluteY() + PANEL_HEIGHT,
-                   time(NULL),
-                   button);
+                   button,
+                   time(NULL));
 }
 
 void PanelIndicatorEntryView::OnMouseDown(int x, int y, long button_flags, long key_flags)
@@ -193,6 +193,14 @@
 // 3. Paint something
 void PanelIndicatorEntryView::Refresh()
 {
+  if (!IsVisible())
+  {
+    SetVisible(false);
+    return;
+  }
+
+  SetVisible(true);
+
   PangoLayout*          layout = NULL;
   PangoFontDescription* desc = NULL;
   PangoAttrList*        attrs = NULL;
@@ -472,10 +480,7 @@
 
 std::string PanelIndicatorEntryView::GetName() const
 {
-  if (proxy_->IsUnused())
-    return "";
-  else
-    return proxy_->id();
+  return proxy_->id();
 }
 
 void PanelIndicatorEntryView::AddProperties(GVariantBuilder* builder)
@@ -498,7 +503,7 @@
 
 void PanelIndicatorEntryView::GetGeometryForSync(indicator::EntryLocationMap& locations)
 {
-  if (proxy_->IsUnused())
+  if (!IsVisible())
     return;
 
   locations[proxy_->id()] = GetAbsoluteGeometry();
@@ -546,6 +551,15 @@
   return (disabled_ || !proxy_.get() || !IsSensitive());
 }
 
+bool PanelIndicatorEntryView::IsVisible()
+{
+  if (proxy_.get())
+  {
+    return proxy_->visible();
+  }
+  return false;
+}
+
 void PanelIndicatorEntryView::OnFontChanged(GObject* gobject, GParamSpec* pspec,
                                             gpointer data)
 {

=== modified file 'plugins/unityshell/src/PanelIndicatorEntryView.h'
--- plugins/unityshell/src/PanelIndicatorEntryView.h	2012-02-07 23:19:01 +0000
+++ plugins/unityshell/src/PanelIndicatorEntryView.h	2012-03-20 10:02:20 +0000
@@ -60,6 +60,7 @@
   bool IsEntryValid() const;
   bool IsSensitive() const;
   bool IsActive() const;
+  bool IsVisible();
   int  GetEntryPriority() const;
 
   void DashShown();

=== modified file 'plugins/unityshell/src/PanelIndicatorsView.cpp'
--- plugins/unityshell/src/PanelIndicatorsView.cpp	2012-02-04 05:28:23 +0000
+++ plugins/unityshell/src/PanelIndicatorsView.cpp	2012-03-20 10:02:20 +0000
@@ -180,7 +180,7 @@
   {
     PanelIndicatorEntryView* view = entry.second;
 
-    if (!target && view->GetAbsoluteGeometry().IsPointInside(x, y))
+    if (!target && view->IsVisible() && view->GetAbsoluteGeometry().IsPointInside(x, y))
     {
       view->Activate(0);
       target = view;

=== modified file 'plugins/unityshell/src/PanelIndicatorsView.h'
--- plugins/unityshell/src/PanelIndicatorsView.h	2012-02-04 05:28:23 +0000
+++ plugins/unityshell/src/PanelIndicatorsView.h	2012-03-20 10:02:20 +0000
@@ -83,7 +83,7 @@
   Entries entries_;
 
   std::string GetName() const;
-  void         AddProperties(GVariantBuilder* builder);
+  void        AddProperties(GVariantBuilder* builder);
 
 private:
   typedef std::vector<indicator::Indicator::Ptr> Indicators;

=== modified file 'plugins/unityshell/src/PanelView.cpp'
--- plugins/unityshell/src/PanelView.cpp	2012-03-13 02:52:00 +0000
+++ plugins/unityshell/src/PanelView.cpp	2012-03-20 10:02:20 +0000
@@ -483,7 +483,7 @@
 {
   // Appmenu is treated differently as it needs to expand
   // We could do this in a more special way, but who has the time for special?
-  if (proxy->name().find("appmenu") != std::string::npos)
+  if (proxy->IsAppmenu())
   {
     _menu_view->AddIndicator(proxy);
   }
@@ -500,7 +500,7 @@
 
 void PanelView::OnObjectRemoved(indicator::Indicator::Ptr const& proxy)
 {
-  if (proxy->name().find("appmenu") != std::string::npos)
+  if (proxy->IsAppmenu())
   {
     _menu_view->RemoveIndicator(proxy);
   }
@@ -569,7 +569,7 @@
   return TRUE;
 }
 
-void PanelView::OnEntryActivated(std::string const& entry_id)
+void PanelView::OnEntryActivated(std::string const& entry_id, nux::Rect const& geo)
 {
   bool active = (entry_id.size() > 0);
   if (active && !_track_menu_pointer_id)
@@ -605,8 +605,9 @@
   _needs_geo_sync = true;
 }
 
-void PanelView::OnEntryShowMenu(std::string const& entry_id,
-                                int x, int y, int timestamp, int button)
+void PanelView::OnEntryShowMenu(std::string const& entry_id, unsigned int xid,
+                                int x, int y, unsigned int button,
+                                unsigned int timestamp)
 {
   Display* d = nux::GetGraphicsDisplay()->GetX11Display();
   XUngrabPointer(d, CurrentTime);

=== modified file 'plugins/unityshell/src/PanelView.h'
--- plugins/unityshell/src/PanelView.h	2012-02-04 05:28:23 +0000
+++ plugins/unityshell/src/PanelView.h	2012-03-20 10:02:20 +0000
@@ -57,10 +57,10 @@
   void OnIndicatorViewUpdated(PanelIndicatorEntryView* view);
   void OnMenuPointerMoved(int x, int y);
   void OnEntryActivateRequest(std::string const& entry_id);
-  void OnEntryActivated(std::string const& entry_id);
+  void OnEntryActivated(std::string const& entry_id, nux::Rect const& geo);
   void OnSynced();
-  void OnEntryShowMenu(std::string const& entry_id,
-                       int x, int y, int timestamp, int button);
+  void OnEntryShowMenu(std::string const& entry_id, unsigned int xid, int x, int y,
+                       unsigned int button, unsigned int timestamp);
 
   void SetPrimary(bool primary);
   bool GetPrimary();

=== modified file 'services/CMakeLists.txt'
--- services/CMakeLists.txt	2012-02-04 05:28:23 +0000
+++ services/CMakeLists.txt	2012-03-20 10:02:20 +0000
@@ -2,7 +2,7 @@
 # Panel Service
 #
 find_package(PkgConfig)
-pkg_check_modules(SERVICE_DEPS REQUIRED gtk+-3.0>=3.3 gobject-2.0 gio-2.0 gthread-2.0 indicator3-0.4 x11 gconf-2.0)
+pkg_check_modules(SERVICE_DEPS REQUIRED gtk+-3.0>=3.3 gobject-2.0 gio-2.0 gthread-2.0 indicator3-0.4>=0.4.90 x11 gconf-2.0)
 
 execute_process (COMMAND ${PKG_CONFIG_EXECUTABLE} indicator3-0.4 --variable indicatordir OUTPUT_VARIABLE _indicatordir OUTPUT_STRIP_TRAILING_WHITESPACE)
 execute_process (COMMAND ${PKG_CONFIG_EXECUTABLE} indicator3-0.4 --variable iconsdir OUTPUT_VARIABLE _iconsdir OUTPUT_STRIP_TRAILING_WHITESPACE)

=== modified file 'services/panel-indicator-entry-accessible.c'
--- services/panel-indicator-entry-accessible.c	2011-11-11 10:44:50 +0000
+++ services/panel-indicator-entry-accessible.c	2012-03-20 10:02:20 +0000
@@ -46,7 +46,8 @@
 #define GET_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE ((o), PANEL_TYPE_INDICATOR_ENTRY_ACCESSIBLE, PanelIndicatorEntryAccessiblePrivate))
 
 static void
-on_entry_activated_cb (PanelService *service, const gchar *entry_id, gpointer user_data)
+on_entry_activated_cb (PanelService *service, const gchar *entry_id,
+                       gint x, gint y, guint w, guint h, gpointer user_data)
 {
   gchar *s;
   gboolean adding = FALSE;

=== modified file 'services/panel-main.c'
--- services/panel-main.c	2012-01-12 16:47:05 +0000
+++ services/panel-main.c	2012-03-20 10:02:20 +0000
@@ -58,13 +58,21 @@
   "    <method name='SyncGeometries'>"
   "      <arg type='a(ssiiii)' name='geometries' direction='in'/>"
   "    </method>"
-	""
+  ""
   "    <method name='ShowEntry'>"
   "      <arg type='s' name='entry_id' direction='in'/>"
-  "      <arg type='u' name='timestamp' direction='in'/>"
-  "      <arg type='i' name='x' direction='in'/>"
-  "      <arg type='i' name='y' direction='in'/>"
-  "      <arg type='i' name='button' direction='in'/>"
+  "      <arg type='u' name='xid' direction='in'/>"
+  "      <arg type='i' name='x' direction='in'/>"
+  "      <arg type='i' name='y' direction='in'/>"
+  "      <arg type='u' name='button' direction='in'/>"
+  "      <arg type='u' name='timestamp' direction='in'/>"
+  "    </method>"
+  ""
+  "    <method name='ShowAppMenu'>"
+  "      <arg type='u' name='xid' direction='in'/>"
+  "      <arg type='i' name='x' direction='in'/>"
+  "      <arg type='i' name='y' direction='in'/>"
+  "      <arg type='u' name='timestamp' direction='in'/>"
   "    </method>"
   ""
   "    <method name='SecondaryActivateEntry'>"
@@ -79,6 +87,7 @@
   ""
   "    <signal name='EntryActivated'>"
   "     <arg type='s' name='entry_id' />"
+  "     <arg type='(iiuu)' name='entry_geometry' />"
   "    </signal>"
   ""
   "    <signal name='ReSync'>"
@@ -174,18 +183,31 @@
     }
   else if (g_strcmp0 (method_name, "ShowEntry") == 0)
     {
+      guint32 xid;
       gchar  *entry_id;
-      guint32 timestamp;
       gint32  x;
       gint32  y;
-      gint32  button;
-      g_variant_get (parameters, "(suiii)", &entry_id, &timestamp, &x, &y, &button, NULL);
+      guint32 button;
+      guint32 timestamp;
+      g_variant_get (parameters, "(suiiuu)", &entry_id, &xid, &x, &y, &button, &timestamp, NULL);
 
-      panel_service_show_entry (service, entry_id, timestamp, x, y, button);
+      panel_service_show_entry (service, entry_id, xid, x, y, button, timestamp);
 
       g_dbus_method_invocation_return_value (invocation, NULL);
       g_free (entry_id);
     }
+  else if (g_strcmp0 (method_name, "ShowAppMenu") == 0)
+    {
+      guint32 xid;
+      gint32  x;
+      gint32  y;
+      guint32 timestamp;
+      g_variant_get (parameters, "(uiiu)", &xid, &x, &y, &timestamp, NULL);
+
+      panel_service_show_app_menu (service, xid, x, y, timestamp);
+
+      g_dbus_method_invocation_return_value (invocation, NULL);
+    }
   else if (g_strcmp0 (method_name, "SecondaryActivateEntry") == 0)
     {
       gchar  *entry_id;
@@ -229,6 +251,7 @@
 static void
 on_service_entry_activated (PanelService    *service,
                             const gchar     *entry_id,
+                            gint x, gint y, guint w, guint h,
                             GDBusConnection *connection)
 {
   GError *error = NULL;
@@ -237,7 +260,7 @@
                                  S_PATH,
                                  S_IFACE,
                                  "EntryActivated",
-                                 g_variant_new ("(s)", entry_id),
+                                 g_variant_new ("(s(iiuu))", entry_id, x, y, w, h),
                                  &error);
 
   if (error)

=== modified file 'services/panel-marshal.list'
--- services/panel-marshal.list	2011-03-17 14:16:13 +0000
+++ services/panel-marshal.list	2012-03-20 10:02:20 +0000
@@ -1,2 +1,3 @@
 NONE:OBJECT,POINTER,INT,INT,INT,INT
 VOID:STRING,BOOLEAN
+VOID:STRING,INT,INT,UINT,UINT

=== modified file 'services/panel-root-accessible.c'
--- services/panel-root-accessible.c	2011-11-11 10:44:50 +0000
+++ services/panel-root-accessible.c	2012-03-20 10:02:20 +0000
@@ -134,16 +134,16 @@
       if (INDICATOR_IS_OBJECT (indicator))
         {
           AtkObject *child;
-          gpointer *data;
+          gpointer *weak_data;
 
           child = panel_indicator_accessible_new (indicator);
           /* FIXME: use proper signals once we support dynamic adding/removing
            * of indicators */
-          data = g_new0 (gpointer, 2);
-          data[0] = root;
-          data[1] = child;
+          weak_data = g_new0 (gpointer, 2);
+          weak_data[0] = root;
+          weak_data[1] = child;
           g_object_weak_ref (G_OBJECT (indicator), 
-              (GWeakNotify) on_indicator_removed, data);
+              (GWeakNotify) on_indicator_removed, weak_data);
 
           atk_object_set_parent (child, accessible);
           root->priv->a11y_children = g_slist_append (root->priv->a11y_children, child);

=== modified file 'services/panel-service.c'
--- services/panel-service.c	2012-02-22 13:59:25 +0000
+++ services/panel-service.c	2012-03-20 10:02:20 +0000
@@ -1,6 +1,6 @@
 // -*- Mode: C; indent-tabs-mode: nil; tab-width: 2 -*-
 /*
- * Copyright (C) 2010 Canonical Ltd
+ * Copyright (C) 2010-2012 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
@@ -27,8 +27,10 @@
 #include "panel-service.h"
 
 #include <stdlib.h>
+#include <string.h>
 #include <gtk/gtk.h>
 #include <gdk/gdkx.h>
+#include <gconf/gconf-client.h>
 
 #include <X11/extensions/XInput2.h>
 #include <X11/XKBlib.h>
@@ -44,12 +46,15 @@
 #define N_TIMEOUT_SLOTS 50
 #define MAX_INDICATOR_ENTRIES 50
 
+#define COMPIZ_OPTIONS_PATH "/apps/compiz-1/plugins/unityshell/screen0/options"
+#define MENU_TOGGLE_KEYBINDING_PATH COMPIZ_OPTIONS_PATH"/panel_first_menu"
+
 static PanelService *static_service = NULL;
 
 struct _PanelServicePrivate
 {
   GSList     *indicators;
-  GHashTable *entry2indicator_hash;
+  GHashTable *id2entry_hash;
   GHashTable *panel2entries_hash;
 
   guint  initial_sync_id;
@@ -69,6 +74,10 @@
   gint     last_bottom;
   guint32  last_menu_button;
 
+  KeyCode toggle_key;
+  guint32 toggle_modifiers;
+  guint32 key_monitor_id;
+
   IndicatorObjectEntry *pressed_entry;
   gboolean use_event;
 };
@@ -121,7 +130,6 @@
 static void sort_indicators (PanelService    *self);
 
 static void notify_object (IndicatorObject *object);
-static IndicatorObjectEntry *get_entry_at (PanelService *self, gint x, gint y);
 
 static GdkFilterReturn event_filter (GdkXEvent    *ev,
                                      GdkEvent     *gev,
@@ -137,24 +145,27 @@
   PanelServicePrivate *priv = PANEL_SERVICE (object)->priv;
   gint i;
 
-  g_hash_table_destroy (priv->entry2indicator_hash);
+  g_hash_table_destroy (priv->id2entry_hash);
   g_hash_table_destroy (priv->panel2entries_hash);
 
   gdk_window_remove_filter (NULL, (GdkFilterFunc)event_filter, object);
 
-  if (G_IS_OBJECT (priv->menubar))
+  if (GTK_IS_WIDGET (priv->menubar) &&
+      gtk_widget_get_realized (GTK_WIDGET (priv->menubar)))
     {
       g_object_unref (priv->menubar);
       priv->menubar = NULL;
     }
 
-  if (G_IS_OBJECT (priv->offscreen_window))
+  if (GTK_IS_WIDGET (priv->offscreen_window) &&
+      gtk_widget_get_realized (GTK_WIDGET (priv->offscreen_window)))
     {
       g_object_unref (priv->offscreen_window);
       priv->offscreen_window = NULL;
     }
 
-  if (G_IS_OBJECT (priv->last_menu))
+  if (GTK_IS_WIDGET (priv->last_menu) &&
+      gtk_widget_get_realized (GTK_WIDGET (priv->last_menu)))
     {
       g_object_unref (priv->last_menu);
       priv->last_menu = NULL;
@@ -175,15 +186,28 @@
         }
     }
 
+  if (priv->key_monitor_id)
+    {
+      gconf_client_notify_remove (gconf_client_get_default(), priv->key_monitor_id);
+      priv->key_monitor_id = 0;
+    }
+
   G_OBJECT_CLASS (panel_service_parent_class)->dispose (object);
 }
 
 static void
+panel_service_class_finalize (GObject *object)
+{
+  static_service = NULL;
+}
+
+static void
 panel_service_class_init (PanelServiceClass *klass)
 {
   GObjectClass *obj_class = G_OBJECT_CLASS (klass);
 
   obj_class->dispose = panel_service_class_dispose;
+  obj_class->finalize = panel_service_class_finalize;
 
   /* Signals */
   _service_signals[ENTRY_ACTIVATED] =
@@ -192,8 +216,9 @@
                   G_SIGNAL_RUN_LAST,
                   0,
                   NULL, NULL,
-                  g_cclosure_marshal_VOID__STRING,
-                  G_TYPE_NONE, 1, G_TYPE_STRING);
+                  panel_marshal_VOID__STRING_INT_INT_UINT_UINT,
+                  G_TYPE_NONE, 5, G_TYPE_STRING, G_TYPE_INT, G_TYPE_INT,
+                  G_TYPE_UINT, G_TYPE_UINT);
 
   _service_signals[RE_SYNC] =
     g_signal_new ("re-sync",
@@ -212,6 +237,7 @@
                   NULL, NULL,
                   g_cclosure_marshal_VOID__STRING,
                   G_TYPE_NONE, 1, G_TYPE_STRING);
+
  _service_signals[GEOMETRIES_CHANGED] =
     g_signal_new ("geometries-changed",
       G_OBJECT_CLASS_TYPE (obj_class),
@@ -245,6 +271,82 @@
   g_type_class_add_private (obj_class, sizeof (PanelServicePrivate));
 }
 
+static IndicatorObjectEntry *
+get_entry_at (PanelService *self, gint x, gint y)
+{
+  GHashTableIter panel_iter, entries_iter;
+  gpointer key, value, k, v;
+
+  g_hash_table_iter_init (&panel_iter, self->priv->panel2entries_hash);
+  while (g_hash_table_iter_next (&panel_iter, &key, &value))
+    {
+      GHashTable *entry2geometry_hash = value;
+      g_hash_table_iter_init (&entries_iter, entry2geometry_hash);
+
+      while (g_hash_table_iter_next (&entries_iter, &k, &v))
+        {
+          IndicatorObjectEntry *entry = k;
+          GdkRectangle *geo = v;
+
+          if (x >= geo->x && x <= (geo->x + geo->width) &&
+              y >= geo->y && y <= (geo->y + geo->height))
+            {
+              return entry;
+            }
+        }
+    }
+
+  return NULL;
+}
+
+static IndicatorObject *
+get_entry_parent_indicator (IndicatorObjectEntry *entry)
+{
+  if (!entry || !INDICATOR_IS_OBJECT (entry->parent_object))
+    return NULL;
+
+  return INDICATOR_OBJECT (entry->parent_object);
+}
+
+static gchar *
+get_indicator_entry_id_by_entry (IndicatorObjectEntry *entry)
+{
+  gchar *entry_id = NULL;
+
+  if (entry)
+    entry_id = g_strdup_printf ("%p", entry);
+
+  return entry_id;
+}
+
+static IndicatorObjectEntry *
+get_indicator_entry_by_id (PanelService *self, const gchar *entry_id)
+{
+  IndicatorObjectEntry *entry;
+
+  entry = g_hash_table_lookup (self->priv->id2entry_hash, entry_id);
+
+  if (entry)
+    {
+      if (g_slist_find (self->priv->indicators, entry->parent_object))
+        {
+          if (!INDICATOR_IS_OBJECT (entry->parent_object))
+            entry = NULL;
+        }
+      else
+        {
+          entry = NULL;
+        }
+
+      if (!entry)
+        {
+          g_warning("The entry id '%s' you're trying to lookup is not a valid IndicatorObjectEntry!", entry_id);
+        }
+    }
+
+  return entry;
+}
+
 static GdkFilterReturn
 event_filter (GdkXEvent *ev, GdkEvent *gev, PanelService *self)
 {
@@ -269,9 +371,9 @@
       if (!event)
         return ret;
 
-      if (event->evtype == XI_KeyRelease)
+      if (event->evtype == XI_KeyPress)
         {
-          if (XkbKeycodeToKeysym(event->display, event->detail, 0, 0) == GDK_KEY_F10)
+          if (event->mods.base == priv->toggle_modifiers && event->detail == priv->toggle_key)
           {
             if (GTK_IS_MENU (priv->last_menu))
               gtk_menu_popdown (GTK_MENU (priv->last_menu));
@@ -293,16 +395,16 @@
           gboolean event_is_a_click = FALSE;
           entry = get_entry_at (self, event->root_x, event->root_y);
 
-          if (XIMaskIsSet (event->buttons.mask, 1) || XIMaskIsSet (event->buttons.mask, 3))
+          if (event->detail == 1 || event->detail == 3)
             {
               /* Consider only right and left clicks over the indicators entries */
               event_is_a_click = TRUE;
             }
-          else if (XIMaskIsSet (event->buttons.mask, 2) && entry)
+          else if (entry && event->detail == 2)
             {
               /* Middle clicks over an appmenu entry are considered just like
                * all other clicks */
-              IndicatorObject *obj = g_hash_table_lookup (priv->entry2indicator_hash, entry);
+              IndicatorObject *obj = get_entry_parent_indicator (entry);
 
               if (g_strcmp0 (g_object_get_data (G_OBJECT (obj), "id"), "libappmenu.so") == 0)
                 {
@@ -330,25 +432,23 @@
                           /* If we were navigating over indicators using the keyboard
                            * and now we click over the indicator under the mouse, we
                            * must force it to show back again, not make it close */
-                          gchar *entry_id = g_strdup_printf ("%p", entry);
+                          gchar *entry_id = get_indicator_entry_id_by_entry (entry);
                           g_signal_emit (self, _service_signals[ENTRY_ACTIVATE_REQUEST], 0, entry_id);
                           g_free (entry_id);
                         }
                     }
                 }
             }
-          else if ((XIMaskIsSet (event->buttons.mask, 2) ||
-                    XIMaskIsSet (event->buttons.mask, 4) ||
-                    XIMaskIsSet (event->buttons.mask, 5)) && entry)
+          else if (entry && (event->detail == 2 || event->detail == 4 || event->detail == 5))
             {
               /* If we're scrolling or middle-clicking over an indicator
                * (which is not an appmenu entry) then we need to send the
                * event to the indicator itself, and avoid it to close */
-              gchar *entry_id = g_strdup_printf ("%p", entry);
+              gchar *entry_id = get_indicator_entry_id_by_entry (entry);
 
-              if (XIMaskIsSet (event->buttons.mask, 4) || XIMaskIsSet (event->buttons.mask, 5))
+              if (event->detail == 4 || event->detail == 5)
                 {
-                  gint32 delta = XIMaskIsSet (event->buttons.mask, 4) ? 120 : -120;
+                  gint32 delta = (event->detail == 4) ? 120 : -120;
                   panel_service_scroll_entry (self, entry_id, delta);
                 }
               else if (entry == priv->pressed_entry)
@@ -365,60 +465,6 @@
   return ret;
 }
 
-static IndicatorObjectEntry *
-get_entry_at (PanelService *self, gint x, gint y)
-{
-  GHashTableIter panel_iter, entries_iter;
-  gpointer key, value, k, v;
-
-  g_hash_table_iter_init (&panel_iter, self->priv->panel2entries_hash);
-  while (g_hash_table_iter_next (&panel_iter, &key, &value))
-    {
-      GHashTable *entry2geometry_hash = value;
-      g_hash_table_iter_init (&entries_iter, entry2geometry_hash);
-
-      while (g_hash_table_iter_next (&entries_iter, &k, &v))
-        {
-          IndicatorObjectEntry *entry = k;
-          GdkRectangle *geo = v;
-
-          if (x >= geo->x && x <= (geo->x + geo->width) &&
-              y >= geo->y && y <= (geo->y + geo->height))
-            {
-              return entry;
-            }
-        }
-    }
-
-  return NULL;
-}
-
-static void
-panel_service_get_indicator_entry_by_id (PanelService *self, 
-                                         const gchar *entry_id,
-                                         IndicatorObjectEntry **entry,
-                                         IndicatorObject **object)
-{
-  IndicatorObject *indicator;
-  IndicatorObjectEntry *probably_entry;
-  PanelServicePrivate *priv = self->priv;
-
-  /* FIXME: eeek, why do we even do this? */
-  if (sscanf (entry_id, "%p", &probably_entry) == 1)
-  {
-    /* check that there really is such IndicatorObjectEntry */
-    indicator = g_hash_table_lookup (priv->entry2indicator_hash,
-                                     probably_entry);
-    if (object) *object = indicator;
-    if (entry) *entry = indicator != NULL ? probably_entry : NULL;
-  }
-  else
-  {
-    if (object) *object = NULL;
-    if (entry) *entry = NULL;
-  }
-}
-
 static gboolean
 initial_resync (PanelService *self)
 {
@@ -431,6 +477,73 @@
 }
 
 static void
+panel_service_update_menu_keybinding (PanelService *self)
+{
+  GConfClient *client = gconf_client_get_default ();
+  gchar *binding = gconf_client_get_string (client, MENU_TOGGLE_KEYBINDING_PATH, NULL);
+
+  KeyCode keycode = 0;
+  KeySym keysym = NoSymbol;
+  guint32 modifiers = 0;
+
+  gchar *keystart = (binding) ? strrchr(binding, '>') : NULL;
+
+  if (!keystart)
+    keystart = binding;
+
+  while (keystart && !g_ascii_isalnum (*keystart))
+    keystart++;
+
+  gchar *keyend = keystart;
+
+  while (keyend && g_ascii_isalnum (*keyend))
+    keyend++;
+
+  if (keystart != keyend)
+  {
+    gchar *keystr = g_strndup (keystart, keyend-keystart);
+    keysym = XStringToKeysym (keystr);
+  }
+
+  if (keysym != NoSymbol)
+    {
+      Display *dpy = GDK_DISPLAY_XDISPLAY (gdk_display_get_default ());
+      keycode = XKeysymToKeycode(dpy, keysym);
+
+      if (g_strrstr (binding, "<Shift>"))
+        {
+          modifiers |= GDK_SHIFT_MASK;
+        }
+      if (g_strrstr (binding, "<Control>"))
+        {
+          modifiers |= GDK_CONTROL_MASK;
+        }
+      if (g_strrstr (binding, "<Alt>") || g_strrstr (binding, "<Mod1>"))
+        {
+          modifiers |= GDK_MOD1_MASK;
+        }
+      if (g_strrstr (binding, "<Super>"))
+        {
+          modifiers |= GDK_SUPER_MASK;
+        }
+    }
+
+  self->priv->toggle_key = keycode;
+  self->priv->toggle_modifiers = modifiers;
+
+  g_free (binding);
+}
+
+void
+on_keybinding_changed (GConfClient* client, guint id, GConfEntry* entry, gpointer data)
+{
+  PanelService *self = data;
+  g_return_if_fail (PANEL_IS_SERVICE (data));
+
+  panel_service_update_menu_keybinding (self);
+}
+
+static void
 panel_service_init (PanelService *self)
 {
   PanelServicePrivate *priv;
@@ -442,7 +555,7 @@
 
   gdk_window_add_filter (NULL, (GdkFilterFunc)event_filter, self);
 
-  priv->entry2indicator_hash = g_hash_table_new (g_direct_hash, g_direct_equal);
+  priv->id2entry_hash = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, NULL);
   priv->panel2entries_hash = g_hash_table_new_full (g_str_hash, g_str_equal,
                                                     g_free,
                                                     (GDestroyNotify) g_hash_table_destroy);
@@ -452,6 +565,14 @@
   sort_indicators (self);
   suppress_signals = FALSE;
 
+  panel_service_update_menu_keybinding (self);
+
+  GConfClient *client = gconf_client_get_default ();
+  gconf_client_add_dir (client, COMPIZ_OPTIONS_PATH, GCONF_CLIENT_PRELOAD_NONE, NULL);
+  priv->key_monitor_id = gconf_client_notify_add (client, MENU_TOGGLE_KEYBINDING_PATH,
+                                                  on_keybinding_changed, self,
+                                                  NULL, NULL);
+
   priv->initial_sync_id = g_idle_add ((GSourceFunc)initial_resync, self);
 }
 
@@ -481,17 +602,23 @@
     {
       for (l = entries; l; l = l->next)
         {
-          g_hash_table_remove (self->priv->entry2indicator_hash, l->data);
-
+          IndicatorObjectEntry *entry;
+          gchar *entry_id;
           GHashTableIter iter;
           gpointer key, value;
+
+          entry = l->data;
+          entry_id = get_indicator_entry_id_by_entry (entry);
+          g_hash_table_remove (self->priv->id2entry_hash, entry_id);
+          g_free (entry_id);
+
           g_hash_table_iter_init (&iter, self->priv->panel2entries_hash);
           while (g_hash_table_iter_next (&iter, &key, &value))
             {
               GHashTable *entry2geometry_hash = value;
 
               if (g_hash_table_size (entry2geometry_hash) > 1)
-                g_hash_table_remove (entry2geometry_hash, l->data);
+                g_hash_table_remove (entry2geometry_hash, entry);
               else
                 g_hash_table_iter_remove (&iter);
             }
@@ -586,7 +713,7 @@
   gpointer timeout = g_object_get_data (G_OBJECT (indicator), "remove-timeout");
 
   if (timeout)
-      g_source_remove (GPOINTER_TO_UINT (timeout));
+    g_source_remove (GPOINTER_TO_UINT (timeout));
 
   g_object_set_data (G_OBJECT (indicator), "remove", GINT_TO_POINTER (TRUE));
   notify_object (indicator);
@@ -695,13 +822,11 @@
                 IndicatorObjectEntry *entry,
                 PanelService         *self)
 {
-  PanelServicePrivate *priv;
-
   g_return_if_fail (PANEL_IS_SERVICE (self));
   g_return_if_fail (entry != NULL);
-  priv = self->priv;
 
-  g_hash_table_insert (priv->entry2indicator_hash, entry, object);
+  gchar *entry_id = get_indicator_entry_id_by_entry (entry);
+  g_hash_table_insert (self->priv->id2entry_hash, entry_id, entry);
 
   if (GTK_IS_LABEL (entry->label))
     {
@@ -748,18 +873,18 @@
                   IndicatorObjectEntry *entry,
                   PanelService         *self)
 {
-  PanelServicePrivate *priv;
   g_return_if_fail (PANEL_IS_SERVICE (self));
   g_return_if_fail (entry != NULL);
 
-  priv = self->priv;
-
-  g_hash_table_remove (priv->entry2indicator_hash, entry);
-  /* Don't remove here the value from priv->panel2entries_hash, this should
-   * be done in during the sync, to avoid false positive.
+  /* Don't remove here the value from panel2entries_hash, this should be
+   * done during the geometries sync, to avoid false positive.
    * FIXME this in libappmenu.so to avoid to send an "entry-removed" signal
    * when switching the focus from a window to one of its dialog children */
 
+  gchar *entry_id = get_indicator_entry_id_by_entry (entry);
+  g_hash_table_remove (self->priv->id2entry_hash, entry_id);
+  g_free (entry_id);
+
   notify_object (object);
 }
 
@@ -786,10 +911,8 @@
       return;
     }
 
-  entry_id = g_strdup_printf ("%p", entry);
-
+  entry_id = get_indicator_entry_id_by_entry (entry);
   g_signal_emit (self, _service_signals[ENTRY_ACTIVATE_REQUEST], 0, entry_id);
-
   g_free (entry_id);
 }
 
@@ -808,10 +931,8 @@
       return;
     }
 
-  entry_id = g_strdup_printf ("%p", entry);
-
+  entry_id = get_indicator_entry_id_by_entry (entry);
   g_signal_emit (self, _service_signals[ENTRY_SHOW_NOW_CHANGED], 0, entry_id, show_now_changed);
-
   g_free (entry_id);
 }
 
@@ -829,7 +950,7 @@
   gchar *name;
   GList *entries, *entry;
 
-  indicator_object_set_environment(object, (const GStrv)indicator_environment);
+  indicator_object_set_environment(object, (GStrv)indicator_environment);
 
   if (_name != NULL)
     name = g_strdup (_name);
@@ -1079,7 +1200,7 @@
         {
           gint prio = -1;
           IndicatorObjectEntry *entry = e->data;
-          gchar *id = g_strdup_printf ("%p", entry);
+          gchar *id = get_indicator_entry_id_by_entry (entry);
 
           if (entry->name_hint)
             {
@@ -1095,13 +1216,14 @@
           indicator_entry_to_variant (entry, id, indicator_id, b, prio);
           g_free (id);
         }
+
+      g_list_free (entries);
     }
   else
     {
       /* Add a null entry to indicate that there is an indicator here, it's just empty */
       indicator_entry_null_to_variant (indicator_id, b);
     }
-  g_list_free (entries);
 }
 
 static void
@@ -1147,7 +1269,7 @@
   priv->use_event = FALSE;
   priv->pressed_entry = NULL;
 
-  g_signal_emit (self, _service_signals[ENTRY_ACTIVATED], 0, "");
+  g_signal_emit (self, _service_signals[ENTRY_ACTIVATED], 0, "", 0, 0, 0, 0);
 }
 
 /*
@@ -1226,15 +1348,29 @@
 {
   IndicatorObject      *object;
   IndicatorObjectEntry *entry;
+  gboolean valid_entry = TRUE;
   PanelServicePrivate  *priv = self->priv;
 
-  panel_service_get_indicator_entry_by_id (self, entry_id, &entry, &object);
+  entry = get_indicator_entry_by_id (self, entry_id);
+
+  /* If the entry we read is not valid, maybe it has already been removed
+   * or unparented, so we need to make sure that the related key on the
+   * entry2geometry_hash is correctly removed and the value is free'd */
+  if (!entry)
+    {
+      IndicatorObjectEntry *invalid_entry;
+      if (sscanf (entry_id, "%p", &invalid_entry) == 1)
+        {
+          entry = invalid_entry;
+          valid_entry = FALSE;
+        }
+    }
 
   if (entry)
     {
       GHashTable *entry2geometry_hash = g_hash_table_lookup (priv->panel2entries_hash, panel_id);
 
-      if (width < 0 || height < 0)
+      if (width < 0 || height < 0 || !valid_entry)
         {
           if (entry2geometry_hash)
             {
@@ -1247,63 +1383,110 @@
                   g_hash_table_remove (priv->panel2entries_hash, panel_id);
                 }
             }
+
+          /* If the entry has been removed let's make sure that its menu is closed */
+          if (valid_entry && GTK_IS_MENU (priv->last_menu) && priv->last_menu == entry->menu)
+            {
+              gtk_menu_popdown (entry->menu);
+            }
         }
       else
         {
           GdkRectangle *geo = NULL;
 
           if (entry2geometry_hash == NULL)
-          {
-            //FIXME - this leaks memory but i'm not 100% on the logic,
-            // using g_free as the keys destructor function causes all
-            // kinds of problems 
-            entry2geometry_hash = g_hash_table_new_full (g_direct_hash, g_direct_equal,
-                                                        NULL, g_free);
-            g_hash_table_insert (priv->panel2entries_hash, g_strdup (panel_id),
-                                 entry2geometry_hash);
-          }
+            {
+              entry2geometry_hash = g_hash_table_new_full (g_direct_hash, g_direct_equal,
+                                                           NULL, g_free);
+              g_hash_table_insert (priv->panel2entries_hash, g_strdup (panel_id),
+                                   entry2geometry_hash);
+            }
           else
-          {
-            geo = g_hash_table_lookup (entry2geometry_hash, entry);
-          }
+            {
+              geo = g_hash_table_lookup (entry2geometry_hash, entry);
+            }
 
           if (geo == NULL)
             {
-              geo = g_new (GdkRectangle, 1);
+              geo = g_new0 (GdkRectangle, 1);
               g_hash_table_insert (entry2geometry_hash, entry, geo);
             }
 
+          /* If the current entry geometry has changed, we need to move the menu
+           * accordingly to the change we recorded! */
+          if (GTK_IS_MENU (priv->last_menu) && priv->last_menu == entry->menu)
+            {
+              GtkWidget *top_widget = gtk_widget_get_toplevel (GTK_WIDGET (priv->last_menu));
+
+              if (GTK_IS_WINDOW (top_widget))
+                {
+                  GtkWindow *top_win = GTK_WINDOW (top_widget);
+                  gint old_x, old_y;
+
+                  gtk_window_get_position (top_win, &old_x, &old_y);
+                  gtk_window_move (top_win, old_x - (geo->x - x), old_y - (geo->y - y));
+                }
+            }
+
           geo->x = x;
           geo->y = y;
           geo->width = width;
           geo->height = height;
         }
 
-      g_signal_emit (self, _service_signals[GEOMETRIES_CHANGED], 0, object, entry, x, y, width, height);
+      if (valid_entry)
+        {
+          object = get_entry_parent_indicator (entry);
+          g_signal_emit (self, _service_signals[GEOMETRIES_CHANGED], 0, object, entry, x, y, width, height);
+        }
     }
 }
 
 static gboolean
-should_skip_menu (IndicatorObjectEntry *entry)
+panel_service_entry_is_visible (PanelService *self, IndicatorObjectEntry *entry)
 {
-  gboolean label_ok = FALSE;
-  gboolean image_ok = FALSE;
-
-  g_return_val_if_fail (entry != NULL, TRUE);
+  GHashTableIter panel_iter;
+  gpointer key, value;
+  gboolean found_geo;
+
+  g_return_val_if_fail (PANEL_IS_SERVICE (self), FALSE);
+  g_return_val_if_fail (entry != NULL, FALSE);
+
+  found_geo = FALSE;
+  g_hash_table_iter_init (&panel_iter, self->priv->panel2entries_hash);
+
+  while (g_hash_table_iter_next (&panel_iter, &key, &value) && !found_geo)
+    {
+      GHashTable *entry2geometry_hash = value;
+
+      if (g_hash_table_lookup (entry2geometry_hash, entry))
+        {
+          found_geo = TRUE;
+        }
+    }
+
+  if (!found_geo)
+    return FALSE;
 
   if (GTK_IS_LABEL (entry->label))
     {
-      label_ok = gtk_widget_get_visible (GTK_WIDGET (entry->label))
-        && gtk_widget_is_sensitive (GTK_WIDGET (entry->label));
+      if (gtk_widget_get_visible (GTK_WIDGET (entry->label)) &&
+          gtk_widget_is_sensitive (GTK_WIDGET (entry->label)))
+        {
+          return TRUE;
+        }
     }
 
   if (GTK_IS_IMAGE (entry->image))
     {
-      image_ok = gtk_widget_get_visible (GTK_WIDGET (entry->image))
-        && gtk_widget_is_sensitive (GTK_WIDGET (entry->image));
+      if (gtk_widget_get_visible (GTK_WIDGET (entry->image)) &&
+          gtk_widget_is_sensitive (GTK_WIDGET (entry->image)))
+        {
+          return TRUE;
+        }
     }
 
-  return !label_ok && !image_ok;
+  return TRUE;
 }
 
 static int
@@ -1341,7 +1524,7 @@
               gint prio = -1;
               new_entry = ll->data;
 
-              if (should_skip_menu (new_entry))
+              if (!panel_service_entry_is_visible (self, new_entry))
                 continue;
 
               if (new_entry->name_hint)
@@ -1388,7 +1571,7 @@
 
   if (new_entry)
     {
-      id = g_strdup_printf ("%p", new_entry);
+      id = get_indicator_entry_id_by_entry (new_entry);
       g_signal_emit (self, _service_signals[ENTRY_ACTIVATE_REQUEST], 0, id);
       g_free (id);
     }
@@ -1433,7 +1616,7 @@
     }
 
   /* Find the next/prev indicator */
-  object = g_hash_table_lookup (priv->entry2indicator_hash, priv->last_entry);
+  object = get_entry_parent_indicator(priv->last_entry);
   if (object == NULL)
     {
       g_warning ("Unable to find IndicatorObject for entry");
@@ -1450,23 +1633,25 @@
   gtk_widget_destroy (menu);
 }
 
-void
-panel_service_show_entry (PanelService *self,
-                          const gchar  *entry_id,
-                          guint32       timestamp,
-                          gint32        x,
-                          gint32        y,
-                          gint32        button)
+static void
+panel_service_show_entry_common (PanelService *self,
+                                 IndicatorObject *object,
+                                 IndicatorObjectEntry *entry,
+                                 guint32       xid,
+                                 gint32        x,
+                                 gint32        y,
+                                 guint32       button,
+                                 guint32       timestamp)
 {
-  IndicatorObject      *object;
-  IndicatorObjectEntry *entry;
-  GtkWidget            *last_menu;
-  PanelServicePrivate  *priv = self->priv;
-
-  panel_service_get_indicator_entry_by_id (self, entry_id, &entry, &object);
-
+  PanelServicePrivate *priv;
+  GtkWidget           *last_menu;
+
+  g_return_if_fail (PANEL_IS_SERVICE (self));
+  g_return_if_fail (INDICATOR_IS_OBJECT (object));
   g_return_if_fail (entry);
 
+  priv = self->priv;
+
   if (priv->last_entry == entry)
     return;
 
@@ -1489,7 +1674,14 @@
 
   if (entry != NULL)
     {
-      g_signal_emit (self, _service_signals[ENTRY_ACTIVATED], 0, entry_id);
+      if (xid > 0)
+        {
+          indicator_object_entry_activate_window (object, entry, xid, CurrentTime);
+        }
+      else
+        {
+          indicator_object_entry_activate (object, entry, CurrentTime);
+        }
 
       if (GTK_IS_MENU (entry->menu))
         {
@@ -1526,29 +1718,33 @@
       priv->last_menu_move_id = g_signal_connect_after (priv->last_menu, "move-current",
                                                         G_CALLBACK (on_active_menu_move_current), self);
 
-      indicator_object_entry_activate (object, entry, CurrentTime);
-
       gtk_menu_popup (priv->last_menu, NULL, NULL, positon_menu, self, 0, CurrentTime);
       GdkWindow *gdkwin = gtk_widget_get_window (GTK_WIDGET (priv->last_menu));
+
       if (gdkwin != NULL)
-      {
-        gint left=0, top=0, width=0, height=0;
-
-        gdk_window_get_geometry (gdkwin, NULL, NULL, &width, &height);
-        gdk_window_get_origin (gdkwin, &left, &top);
-
-        priv->last_left = left;
-        priv->last_right = left + width -1;
-        priv->last_top = top;
-        priv->last_bottom = top + height -1;
-      }
+        {
+          gint left=0, top=0, width=0, height=0;
+
+          gdk_window_get_geometry (gdkwin, NULL, NULL, &width, &height);
+          gdk_window_get_origin (gdkwin, &left, &top);
+
+          gchar *entry_id = get_indicator_entry_id_by_entry (entry);
+          g_signal_emit (self, _service_signals[ENTRY_ACTIVATED], 0, entry_id,
+                         left, top, width, height);
+          g_free (entry_id);
+
+          priv->last_left = left;
+          priv->last_right = left + width -1;
+          priv->last_top = top;
+          priv->last_bottom = top + height -1;
+        }
       else
-      {
-        priv->last_left = 0;
-        priv->last_right = 0;
-        priv->last_top = 0;
-        priv->last_bottom = 0;
-      }
+        {
+          priv->last_left = 0;
+          priv->last_right = 0;
+          priv->last_top = 0;
+          priv->last_bottom = 0;
+        }
     }
 
   /* We popdown the old one last so we don't accidently send key focus back to the
@@ -1560,6 +1756,53 @@
 }
 
 void
+panel_service_show_entry (PanelService *self,
+                          const gchar  *entry_id,
+                          guint32       xid,
+                          gint32        x,
+                          gint32        y,
+                          guint32       button,
+                          guint32       timestamp)
+{
+  IndicatorObject      *object;
+  IndicatorObjectEntry *entry;
+
+  g_return_if_fail (PANEL_IS_SERVICE (self));
+
+  entry = get_indicator_entry_by_id (self, entry_id);
+  object = get_entry_parent_indicator (entry);
+
+  panel_service_show_entry_common (self, object, entry, xid, x, y, button, timestamp);
+}
+
+void
+panel_service_show_app_menu (PanelService *self,
+                             guint32       xid,
+                             gint32        x,
+                             gint32        y,
+                             guint32       timestamp)
+{
+  IndicatorObject      *object;
+  IndicatorObjectEntry *entry;
+  GList                *entries;
+
+  g_return_if_fail (PANEL_IS_SERVICE (self));
+
+  object = panel_service_get_indicator (self, "libappmenu.so");
+  g_return_if_fail (INDICATOR_IS_OBJECT (object));
+
+  entries = indicator_object_get_entries (object);
+
+  if (entries)
+    {
+      entry = entries->data;
+      g_list_free (entries);
+
+      panel_service_show_entry_common (self, object, entry, xid, x, y, 1, timestamp);
+    }
+}
+
+void
 panel_service_secondary_activate_entry (PanelService *self,
                                         const gchar  *entry_id,
                                         guint32       timestamp)
@@ -1567,9 +1810,10 @@
   IndicatorObject      *object;
   IndicatorObjectEntry *entry;
 
-  panel_service_get_indicator_entry_by_id (self, entry_id, &entry, &object);
+  entry = get_indicator_entry_by_id (self, entry_id);
   g_return_if_fail (entry);
 
+  object = get_entry_parent_indicator (entry);
   g_signal_emit_by_name(object, INDICATOR_OBJECT_SIGNAL_SECONDARY_ACTIVATE, entry,
                         timestamp);
 }
@@ -1582,11 +1826,12 @@
   IndicatorObject      *object;
   IndicatorObjectEntry *entry;
 
-  panel_service_get_indicator_entry_by_id (self, entry_id, &entry, &object);
+  entry = get_indicator_entry_by_id (self, entry_id);
   g_return_if_fail (entry);
 
   GdkScrollDirection direction = delta < 0 ? GDK_SCROLL_DOWN : GDK_SCROLL_UP;
 
+  object = get_entry_parent_indicator (entry);
   g_signal_emit_by_name(object, INDICATOR_OBJECT_SIGNAL_ENTRY_SCROLLED, entry,
                         abs(delta/120), direction);
 }

=== modified file 'services/panel-service.h'
--- services/panel-service.h	2011-09-04 12:11:22 +0000
+++ services/panel-service.h	2012-03-20 10:02:20 +0000
@@ -95,10 +95,17 @@
 
 void              panel_service_show_entry    (PanelService *self,
 					       const gchar  *entry_id,
-					       guint32       timestamp,
-					       gint32        x,
-					       gint32        y,
-					       gint32        button);
+					       guint32       xid,
+					       gint32        x,
+					       gint32        y,
+					       guint32       button,
+					       guint32       timestamp);
+
+void              panel_service_show_app_menu (PanelService *self,
+					       guint32       xid,
+					       gint32        x,
+					       gint32        y,
+					       guint32       timestamp);
 
 void              panel_service_secondary_activate_entry (PanelService *self,
 					       const gchar  *entry_id,

=== modified file 'tests/CMakeLists.txt'
--- tests/CMakeLists.txt	2012-03-13 01:59:15 +0000
+++ tests/CMakeLists.txt	2012-03-20 10:02:20 +0000
@@ -45,9 +45,18 @@
 # We can't have convenience libs so we need to rebuild with what we need
 # Please keep actual test files alphabetically at top and then files
 # from ../${UNITY_SRC} or ../../services in alphabetically after that
+find_program(GLIB_GENMARSHAL glib-genmarshal)
+add_custom_command(OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/panel-marshal.c
+  COMMAND ${GLIB_GENMARSHAL} ARGS ../services/panel-marshal.list --body --prefix=panel_marshal > ${CMAKE_CURRENT_BINARY_DIR}/panel-marshal.c
+  COMMAND ${GLIB_GENMARSHAL} ARGS ../services/panel-marshal.list --header --prefix=panel_marshal > ${CMAKE_CURRENT_BINARY_DIR}/panel-marshal.h
+  WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
+  DEPENDS ../services/panel-marshal.list
+  COMMENT "Generating marshallers for panel-service")
+
 add_executable (test-unit
+                unit/TestMain.cpp
                 unit/TestQuicklistMenuitems.cpp
-                unit/TestMain.cpp
+                unit/TestPanelService.cpp
                 unit/TestUBus.cpp
                 unit/TestStaticCairoText.cpp
                 ${UNITY_SRC}/CairoBaseWindow.cpp
@@ -59,6 +68,8 @@
                 ${UNITY_SRC}/QuicklistMenuItemSeparator.cpp
                 ${UNITY_SRC}/QuicklistView.cpp
                 ${UNITY_SRC}/StaticCairoText.cpp
+                ../services/panel-service.c
+                ${CMAKE_CURRENT_BINARY_DIR}/panel-marshal.c
                 ${UNITY_SRC}/ubus-server.cpp
                 )
 add_dependencies (test-unit unity-core-${UNITY_API_VERSION})
@@ -114,6 +125,9 @@
                  test_glib_signals_utils.h
                  test_glib_variant.cpp
                  ${CMAKE_CURRENT_BINARY_DIR}/test_glib_signals_utils_marshal.cpp
+                 test_indicator.cpp
+                 test_indicator_appmenu.cpp
+                 test_indicator_entry.cpp
                  test_favorite_store_gsettings.cpp
                  test_favorite_store_private.cpp
                  test_home_lens.cpp
@@ -180,7 +194,6 @@
                  test_filter.cpp
                  test_gdbus_proxy.cpp
                  test_hud.cpp
-                 test_indicator_entry.cpp
                  test_lens.cpp
                  test_main_dbus.cpp
                  test_model.cpp

=== added file 'tests/test_indicator.cpp'
--- tests/test_indicator.cpp	1970-01-01 00:00:00 +0000
+++ tests/test_indicator.cpp	2012-03-20 10:02:20 +0000
@@ -0,0 +1,207 @@
+// -*- Mode: C++; indent-tabs-mode: nil; tab-width: 2 -*-
+/*
+ * Copyright (C) 2012 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 warranty of
+ * MERCHANTABILITY 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/>.
+ *
+ * Authored by: Marco Trevisan (Treviño) <[e-mail]>
+ */
+
+#include <UnityCore/Indicator.h>
+
+#include <gtest/gtest.h>
+
+using namespace std;
+using namespace unity;
+using namespace indicator;
+
+namespace
+{
+
+TEST(TestIndicator, Construction)
+{
+  Indicator indicator("indicator-test");
+
+  EXPECT_EQ(indicator.name(), "indicator-test");
+  EXPECT_FALSE(indicator.IsAppmenu());
+  EXPECT_EQ(indicator.GetEntry("test-entry"), nullptr);
+  EXPECT_TRUE(indicator.GetEntries().empty());
+}
+
+TEST(TestIndicator, Syncing)
+{
+  Indicator::Entries sync_data;
+  Entry* entry;
+
+  Indicator indicator("indicator-test");
+
+  // Connecting to signals
+  Indicator::Entries added;
+  indicator.on_entry_added.connect([&added] (Entry::Ptr const& e) {
+    added.push_back(e);
+  });
+
+  std::vector<std::string> removed;
+  sigc::connection removed_connection = indicator.on_entry_removed.connect([&removed]
+  (std::string const& en_id) {
+    removed.push_back(en_id);
+  });
+
+  entry = new Entry("test-entry-1", "name-hint", "label", true, true, 0, "icon",
+                    true, true, -1);
+  Entry::Ptr entry1(entry);
+  sync_data.push_back(entry1);
+
+  entry = new Entry("test-entry-2", "name-hint", "label", true, true, 0, "icon",
+                    true, true, -1);
+  Entry::Ptr entry2(entry);
+  sync_data.push_back(entry2);
+
+  entry = new Entry("test-entry-3", "name-hint", "label", true, true, 0, "icon",
+                    true, true, -1);
+  Entry::Ptr entry3(entry);
+  sync_data.push_back(entry3);
+
+  // Sync the indicator, adding 3 entries
+  indicator.Sync(sync_data);
+  EXPECT_EQ(indicator.GetEntries().size(), 3);
+  EXPECT_EQ(indicator.GetEntry("test-entry-2"), entry2);
+  EXPECT_EQ(added.size(), 3);
+  EXPECT_EQ(added.front()->id(), "test-entry-1");
+  EXPECT_EQ(added.back()->id(), "test-entry-3");
+  EXPECT_EQ(removed.size(), 0);
+
+  added.clear();
+  removed.clear();
+
+  // Sync the indicator removing an entry
+  sync_data.remove(entry2);
+  EXPECT_EQ(sync_data.size(), 2);
+
+  indicator.Sync(sync_data);
+  EXPECT_EQ(indicator.GetEntries().size(), 2);
+  EXPECT_EQ(indicator.GetEntry("test-entry-2"), nullptr);
+  EXPECT_EQ(added.size(), 0);
+  EXPECT_EQ(removed.size(), 1);
+  EXPECT_EQ(removed.front(), entry2->id());
+
+  // Sync the indicator removing an entry and adding a new one
+  entry = new Entry("test-entry-4", "name-hint", "label", true, true, 0, "icon",
+                    true, true, -1);
+  Entry::Ptr entry4(entry);
+  sync_data.push_back(entry4);
+  sync_data.remove(entry3);
+  EXPECT_EQ(sync_data.size(), 2);
+
+  added.clear();
+  removed.clear();
+
+  indicator.Sync(sync_data);
+  EXPECT_EQ(indicator.GetEntries().size(), 2);
+  EXPECT_EQ(added.size(), 1);
+  EXPECT_EQ(added.front(), entry4);
+  EXPECT_EQ(removed.size(), 1);
+  EXPECT_EQ(removed.front(), entry3->id());
+
+  // Remove all the indicators
+  added.clear();
+  removed.clear();
+
+  sync_data.clear();
+  indicator.Sync(sync_data);
+  EXPECT_EQ(indicator.GetEntries().size(), 0);
+  EXPECT_EQ(added.size(), 0);
+  EXPECT_EQ(removed.size(), 2);
+
+  removed_connection.disconnect();
+}
+
+TEST(TestIndicator, ChildrenSignals)
+{
+  Indicator::Entries sync_data;
+  Entry* entry;
+
+  Indicator indicator("indicator-test");
+
+  entry = new Entry("test-entry-1", "name-hint", "label", true, true, 0, "icon",
+                    true, true, -1);
+  Entry::Ptr entry1(entry);
+  sync_data.push_back(entry1);
+
+  indicator.Sync(sync_data);
+
+  std::string show_entry;
+  int show_x, show_y;
+  unsigned int show_xid, show_button, show_timestamp;
+
+  // Connecting to signals
+  indicator.on_show_menu.connect([&] (std::string const& eid, unsigned int xid,
+                                      int x, int y, unsigned int button,
+                                      unsigned int timestamp) {
+    show_entry = eid;
+    show_xid = xid;
+    show_x = x;
+    show_y = y;
+    show_button = button;
+    show_timestamp = timestamp;
+  });
+
+  entry1->ShowMenu(123456789, 50, 100, 2, 1328058770);
+  EXPECT_EQ(show_entry, "test-entry-1");
+  EXPECT_EQ(show_xid, 123456789);
+  EXPECT_EQ(show_x, 50);
+  EXPECT_EQ(show_y, 100);
+  EXPECT_EQ(show_button, 2);
+  EXPECT_EQ(show_timestamp, 1328058770);
+
+  // Check if a removed entry still emits a signal to the old indicator
+  show_entry = "invalid-entry";
+  sync_data.remove(entry1);
+  indicator.Sync(sync_data);
+
+  entry1->ShowMenu(123456789, 50, 100, 2, 1328058770);
+  EXPECT_EQ(show_entry, "invalid-entry");
+
+  // Checking secondary activate signal
+  entry = new Entry("test-entry-2", "name-hint", "label", true, true, 0, "icon",
+                      true, true, -1);
+  sync_data.push_back(Entry::Ptr(entry));
+  indicator.Sync(sync_data);
+
+  std::string secondary_activated;
+  unsigned int secondary_timestamp;
+  indicator.on_secondary_activate.connect([&] (std::string const& eid,
+                                               unsigned int timestamp) {
+    secondary_activated = eid;
+    secondary_timestamp = timestamp;
+  });
+
+  entry->SecondaryActivate(1328060717);
+  EXPECT_EQ(secondary_activated, "test-entry-2");
+  EXPECT_EQ(secondary_timestamp, 1328060717);
+
+  // Checking scroll signal
+  std::string scrolled_entry;
+  int scrolled_delta;
+
+  indicator.on_scroll.connect([&] (std::string const& eid, int delta) {
+    scrolled_entry = eid;
+    scrolled_delta = delta;
+  });
+
+  entry->Scroll(-5);
+  EXPECT_EQ(scrolled_entry, "test-entry-2");
+  EXPECT_EQ(scrolled_delta, -5);
+}
+
+}

=== added file 'tests/test_indicator_appmenu.cpp'
--- tests/test_indicator_appmenu.cpp	1970-01-01 00:00:00 +0000
+++ tests/test_indicator_appmenu.cpp	2012-03-20 10:02:20 +0000
@@ -0,0 +1,66 @@
+// -*- Mode: C++; indent-tabs-mode: nil; tab-width: 2 -*-
+/*
+ * Copyright (C) 2012 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 warranty of
+ * MERCHANTABILITY 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/>.
+ *
+ * Authored by: Marco Trevisan (Treviño) <[e-mail]>
+ */
+
+#include <UnityCore/AppmenuIndicator.h>
+
+#include <gtest/gtest.h>
+
+using namespace std;
+using namespace unity;
+using namespace indicator;
+
+namespace
+{
+
+TEST(TestAppmenuIndicator, Construction)
+{
+  AppmenuIndicator indicator("indicator-appmenu");
+
+  EXPECT_EQ(indicator.name(), "indicator-appmenu");
+  EXPECT_TRUE(indicator.IsAppmenu());
+}
+
+TEST(TestAppmenuIndicator, ShowAppmenu)
+{
+  AppmenuIndicator indicator("indicator-appmenu");
+
+  bool signal_emitted = false;
+  int show_x, show_y;
+  unsigned int show_xid, show_timestamp;
+
+  // Connecting to signals
+  indicator.on_show_appmenu.connect([&] (unsigned int xid, int x, int y,
+                                         unsigned int timestamp) {
+    signal_emitted = true;
+    show_xid = xid;
+    show_x = x;
+    show_y = y;
+    show_timestamp = timestamp;
+  });
+
+  indicator.ShowAppmenu(123456789, 50, 100, 1328308554);
+  EXPECT_TRUE(signal_emitted);
+
+  EXPECT_EQ(show_xid, 123456789);
+  EXPECT_EQ(show_x, 50);
+  EXPECT_EQ(show_y, 100);
+  EXPECT_EQ(show_timestamp, 1328308554);
+}
+
+}

=== modified file 'tests/test_indicator_entry.cpp'
--- tests/test_indicator_entry.cpp	2011-09-08 14:41:57 +0000
+++ tests/test_indicator_entry.cpp	2012-03-20 10:02:20 +0000
@@ -24,7 +24,7 @@
   EXPECT_TRUE(entry.image_visible());
   EXPECT_FALSE(entry.active());
   EXPECT_FALSE(entry.show_now());
-  EXPECT_FALSE(entry.IsUnused());
+  EXPECT_TRUE(entry.visible());
   EXPECT_EQ(entry.image_type(), 1);
   EXPECT_EQ(entry.image_data(), "some icon");
   EXPECT_EQ(entry.priority(), -1);
@@ -77,21 +77,6 @@
   EXPECT_EQ(entry.priority(), 5);
 }
 
-TEST(TestIndicatorEntry, TestUnused)
-{
-
-  indicator::Entry entry("id", "name_hint", "label", true, true,
-                         0, "some icon", false, true, -1);
-
-  Counter counter;
-  entry.updated.connect(sigc::mem_fun(counter, &Counter::increment));
-
-  entry.MarkUnused();
-  EXPECT_TRUE(entry.IsUnused());
-  // Setting unused emits updated.
-  EXPECT_EQ(counter.count, 1);
-}
-
 TEST(TestIndicatorEntry, TestShowNowEvents)
 {
 
@@ -174,16 +159,18 @@
 
 struct ShowMenuRecorder
 {
-  void OnShowMenu(std::string const& a, int b, int c, int d, int e)
+  void OnShowMenu(std::string const& a, unsigned int b, int c, int d, unsigned int e, unsigned int f)
   {
     name = a;
-    x = b;
-    y = c;
-    timestamp = d;
+    xid = b;
+    x = c;
+    y = d;
     button = e;
+    timestamp = f;
   }
   std::string name;
-  int x, y, timestamp, button;
+  unsigned int xid, timestamp, button;
+  int x, y;
 };
 
 TEST(TestIndicatorEntry, TestOnShowMenu)
@@ -195,13 +182,103 @@
   ShowMenuRecorder recorder;
   entry.on_show_menu.connect(sigc::mem_fun(recorder, &ShowMenuRecorder::OnShowMenu));
 
-  entry.ShowMenu(10, 20, 12345, 1);
+  entry.ShowMenu(10, 20, 1, 12345);
   EXPECT_EQ(recorder.name, "id");
+  EXPECT_EQ(recorder.xid, 0);
   EXPECT_EQ(recorder.x, 10);
   EXPECT_EQ(recorder.y, 20);
   EXPECT_EQ(recorder.timestamp, 12345);
   EXPECT_EQ(recorder.button, 1);
 }
 
+TEST(TestIndicatorEntry, TestOnShowMenuXid)
+{
+
+  indicator::Entry entry("xid", "name_hint", "label", true, true,
+                         0, "some icon", false, true, -1);
+
+  ShowMenuRecorder recorder;
+  entry.on_show_menu.connect(sigc::mem_fun(recorder, &ShowMenuRecorder::OnShowMenu));
+
+  entry.ShowMenu(88492615, 15, 25, 2, 123456);
+  EXPECT_EQ(recorder.name, "xid");
+  EXPECT_EQ(recorder.xid, 88492615);
+  EXPECT_EQ(recorder.x, 15);
+  EXPECT_EQ(recorder.y, 25);
+  EXPECT_EQ(recorder.timestamp, 123456);
+  EXPECT_EQ(recorder.button, 2);
+}
+
+TEST(TestIndicatorEntry, TestVisibility)
+{
+
+  indicator::Entry entry("id", "name_hint", "label", true, true,
+                         0, "some icon", false, false, -1);
+
+  EXPECT_TRUE(entry.visible());
+
+  entry.setLabel("", true, true);
+  EXPECT_FALSE(entry.visible());
+
+  entry.setLabel("valid-label", true, true);
+  EXPECT_TRUE(entry.visible());
+
+  entry.setLabel("invalid-label", true, false);
+  EXPECT_FALSE(entry.visible());
+
+  entry.setImage(1, "valid-image", true, true);
+  EXPECT_TRUE(entry.visible());
+
+  entry.setImage(1, "", true, true);
+  EXPECT_FALSE(entry.visible());
+
+  entry.setImage(1, "valid-image", true, true);
+  EXPECT_TRUE(entry.visible());
+
+  entry.setImage(0, "invalid-image-type", true, true);
+  EXPECT_FALSE(entry.visible());
+
+  entry.setLabel("valid-label", true, true);
+  EXPECT_TRUE(entry.visible());
+
+  entry.setLabel("invalid-label", true, false);
+  EXPECT_FALSE(entry.visible());
+
+  entry.setImage(1, "invalid-image", true, false);
+  EXPECT_FALSE(entry.visible());
+
+  entry.setLabel("valid-label", true, true);
+  entry.setImage(1, "valid-image", true, true);
+  EXPECT_TRUE(entry.visible());
+}
+
+TEST(TestIndicatorEntry, TestGeometry)
+{
+
+  indicator::Entry entry("id", "name_hint", "label", true, true,
+                         0, "some icon", false, true, -1);
+
+  Counter counter;
+  entry.updated.connect(sigc::mem_fun(counter, &Counter::increment));
+  bool geo_changed = false;
+  nux::Rect new_geo;
+
+  entry.geometry_changed.connect([&] (nux::Rect const& geo) {
+    geo_changed = true;
+    new_geo = geo;
+  });
+
+  // Setting to the same value doesn't emit any events.
+  entry.set_geometry(nux::Rect());
+  EXPECT_EQ(entry.geometry(), nux::Rect());
+  EXPECT_EQ(counter.count, 0);
+
+  // Setting to a different value does emit the events.
+  entry.set_geometry(nux::Rect(1, 2, 3, 4));
+  EXPECT_EQ(entry.geometry(), nux::Rect(1, 2, 3, 4));
+  EXPECT_TRUE(geo_changed);
+  EXPECT_EQ(new_geo, nux::Rect(1, 2, 3, 4));
+  EXPECT_EQ(counter.count, 1);
+}
 
 }

=== modified file 'tests/unit/TestMain.cpp'
--- tests/unit/TestMain.cpp	2012-01-09 16:49:50 +0000
+++ tests/unit/TestMain.cpp	2012-03-20 10:02:20 +0000
@@ -25,9 +25,9 @@
 #include "Nux/Nux.h"
 #include "Nux/WindowThread.h"
 
-//void TestPanelServiceCreateSuite (void);
-void TestUBusCreateSuite(void);
-void TestQuicklistMenuitemsCreateSuite(void);
+void TestPanelServiceCreateSuite();
+void TestUBusCreateSuite();
+void TestQuicklistMenuitemsCreateSuite();
 void TestStaticCairoTextCreateSuite();
 
 nux::WindowThread*
@@ -70,7 +70,7 @@
   g_test_init(&argc, &argv, NULL);
 
   //Keep alphabetical please
-  //TestPanelServiceCreateSuite ();
+  TestPanelServiceCreateSuite();
   TestQuicklistMenuitemsCreateSuite();
   TestStaticCairoTextCreateSuite();
   TestUBusCreateSuite();

=== modified file 'tests/unit/TestPanelService.cpp'
--- tests/unit/TestPanelService.cpp	2011-08-09 12:21:18 +0000
+++ tests/unit/TestPanelService.cpp	2012-03-20 10:02:20 +0000
@@ -177,6 +177,7 @@
   GVariantIter* iter;
   gchar*        indicator_id;
   gchar*        entry_id;
+  gchar*        entry_name_hint;
   gchar*        label;
   gboolean      label_sensitive;
   gboolean      label_visible;
@@ -185,10 +186,11 @@
   gboolean      image_sensitive;
   gboolean      image_visible;
 
-  g_variant_get(result, "(a(sssbbusbb))", &iter);
-  while (g_variant_iter_loop(iter, "(sssbbusbb)",
+  g_variant_get(result, "(a(ssssbbusbbi))", &iter);
+  while (g_variant_iter_loop(iter, "(ssssbbusbbi)",
                              &indicator_id,
                              &entry_id,
+                             &entry_name_hint,
                              &label,
                              &label_sensitive,
                              &label_visible,
@@ -215,8 +217,8 @@
   guint  ret = 0;
   GVariantIter* iter;
   gchar*        indicator_id;
-  gchar*        name_hint;
   gchar*        entry_id;
+  gchar*        entry_name_hint;
   gchar*        label;
   gboolean      label_sensitive;
   gboolean      label_visible;
@@ -229,8 +231,8 @@
   g_variant_get(result, "(a(ssssbbusbbi))", &iter);
   while (g_variant_iter_loop(iter, "(ssssbbusbbi)",
                              &indicator_id,
-                             &name_hint,
                              &entry_id,
+                             &entry_name_hint,
                              &label,
                              &label_sensitive,
                              &label_visible,