Merge lp:~3v1n0/indicator-sound/bamf-player-activation into lp:indicator-sound/13.04

[Marco Trevisan (Treviño)]

PlayerActivator: Use BAMF to find the windows to activate with timestamp

Improved the"old" GtkApplicationPlayer, using BAMF as a fallback method to
activate an application's windows. Basically we try to get the windows of the
selected application and when found we focus them using the activation
timestamp.

Added a new fallback method to focus the windows of the player that we need to raise using the event timestamp, if we're not using a GtkApplication based player, we try to get the player windows from Bamf.
At that point we can focus them with the activation event timestamp.

This will work well for banshee, Spotify, Youtube and any other local or web-player.

I've added a new dependency to libbamf3. If this is a problem for packaing stuff, I can provide a version that uses bamf directly via DBus, but I'd really prefer to use the library.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:351
http://jenkins.qa.ubuntu.com/job/indicator-sound-ci/12/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-sound-raring-amd64-ci/1
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-sound-raring-armhf-ci/1

Click here to trigger a rebuild:
http://s-jenkins:8080/job/indicator-sound-ci/12/rebuild

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:351
http://jenkins.qa.ubuntu.com/job/indicator-sound-13.04-ci/1/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-sound-13.04-raring-amd64-ci/1
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-sound-13.04-raring-armhf-ci/1

Click here to trigger a rebuild:
http://s-jenkins:8080/job/indicator-sound-13.04-ci/1/rebuild

review: Approve (continuous-integration)

[Mathieu Trudel-Lapierre (cyphermox)]

This is quite complex and I would have rather it not been including a rename of the file, since it's not necessary to rename to fix the bug..

Abstain. For the debian/control file it seems fine, too, and Jenkins is happy with it.

review: Abstain

[Mathieu Trudel-Lapierre (cyphermox)]

In other words, let's get someone else to review the code just to be safe.

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

Mathieu, yes I've asked a review already to Larsu.

However, about the file renaming, it's a file that it's not in trunk yet... I've just a file I've added in the prerequisite branch that I've renamed to be more general.

So, see these two branches as one split in two for reviewing.

[Lars Karlitski (larsu)]

Looks good to me, thanks Marco.

review: Approve

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

=== modified file 'configure.ac'
--- configure.ac	2012-11-13 20:24:30 +0000
+++ configure.ac	2013-04-03 17:22:31 +0000
@@ -52,7 +52,9 @@
 PKG_CHECK_MODULES(SOUNDSERVICE, dbusmenu-glib-0.4 >= $DBUSMENUGLIB_REQUIRED_VERSION
                                 indicator3-0.4
                                 gee-1.0
+                                gdk-x11-3.0
                                 gio-unix-2.0
+                                libbamf3
                                 libxml-2.0)
 
 AC_SUBST(APPLET_CFLAGS)

=== modified file 'debian/control'
--- debian/control	2012-11-14 21:43:14 +0000
+++ debian/control	2013-04-03 17:22:31 +0000
@@ -10,6 +10,7 @@
                gnome-common,
                autotools-dev,
                valac-0.18,
+               libbamf3-dev,
                libglib2.0-dev (>= 2.22.3),
                libgtk-3-dev,
                libdbusmenu-glib-dev (>= 0.5.90),

=== modified file 'src/Makefile.am'
--- src/Makefile.am	2013-04-03 17:22:31 +0000
+++ src/Makefile.am	2013-04-03 17:22:31 +0000
@@ -68,7 +68,7 @@
 	playlists-menu-item.vala \
 	freedesktop-interfaces.vala \
 	fetch-file.vala \
-	gtk-application-player.vala
+	player-activator.vala
 
 music_bridge_VALAFLAGS = \
   --ccode \
@@ -83,7 +83,9 @@
   --pkg gio-2.0 \
   --pkg gio-unix-2.0 \
   --pkg gdk-3.0 \
+  --pkg gdk-x11-3.0 \
   --pkg gdk-pixbuf-2.0 \
+  --pkg libbamf3 \
   --pkg libxml-2.0
 
  $(MAINTAINER_VALAFLAGS)

=== modified file 'src/metadata-menu-item.vala'
--- src/metadata-menu-item.vala	2013-04-03 17:22:31 +0000
+++ src/metadata-menu-item.vala	2013-04-03 17:22:31 +0000
@@ -177,7 +177,7 @@
       this.owner.instantiate(timestamp);
     }
     else if (this.owner.current_state == PlayerController.state.CONNECTED) {
-      this.owner.gtk_app_player.activate(timestamp);
+      this.owner.player_activator.activate(timestamp);
       this.owner.mpris_bridge.expose(timestamp);
     }
   }

=== renamed file 'src/gtk-application-player.vala' => 'src/player-activator.vala'
--- src/gtk-application-player.vala	2013-04-03 17:22:31 +0000
+++ src/player-activator.vala	2013-04-03 17:22:31 +0000
@@ -4,16 +4,16 @@
 Authors:
     Marco Trevisan <[e-mail]>
 
-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 
+This program is free software: you can redistribute it and/or modify it
+under the terms of the GNU General Public License version 3, as published
 by the Free Software Foundation.
 
-This program is distributed in the hope that it will be useful, but 
-WITHOUT ANY WARRANTY; without even the implied warranties of 
-MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR 
+This program is distributed in the hope that it will be useful, but
+WITHOUT ANY WARRANTY; without even the implied warranties of
+MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
 PURPOSE.  See the GNU General Public License for more details.
 
-You should have received a copy of the GNU General Public License along 
+You should have received a copy of the GNU General Public License along
 with this program.  If not, see <http://www.gnu.org/licenses/>.
 */
 
@@ -22,24 +22,44 @@
   public abstract void Activate(GLib.HashTable<string, Variant?> platform_data) throws IOError;
 }
 
-public class GtkApplicationPlayer : GLib.Object
+public class PlayerActivator : GLib.Object
 {
   public PlayerController owner {get; construct;}
 
   private bool gtk_application_searched = false;
   private DBusGtkApplication gtk_application;
-
-  public GtkApplicationPlayer(PlayerController ctrl)
+  private Bamf.Application bamf_application;
+
+  private const uint MAX_BAMF_APPLICATION_WAIT_MS = 1000;
+  private int64 last_check_time;
+
+  public PlayerActivator(PlayerController ctrl)
   {
     GLib.Object(owner: ctrl);
   }
 
   public void activate(uint timestamp)
   {
+    if (!activate_gtk_appplication(timestamp)) {
+      if (!activate_bamf_appplication(timestamp)) {
+        // Let's wait BAMF to update its windows list
+        this.last_check_time = get_monotonic_time();
+
+        Idle.add(() => {
+          bool activated = activate_bamf_appplication(timestamp);
+          int64 waited = (get_monotonic_time() - this.last_check_time) / 1000;
+          return !activated && waited < MAX_BAMF_APPLICATION_WAIT_MS;
+        });
+      }
+    }
+  }
+
+  private bool activate_gtk_appplication(uint timestamp)
+  {
     this.setup_gtk_application();
 
     if (this.gtk_application == null) {
-      return;
+      return false;
     }
 
     var context = Gdk.Display.get_default().get_app_launch_context();
@@ -49,9 +69,13 @@
     data["desktop-startup-id"] = context.get_startup_notify_id(this.owner.app_info, new GLib.List<GLib.File>());
 
     try {
-    	this.gtk_application.Activate(data);
-    }
-    catch (IOError e) {}
+      this.gtk_application.Activate(data);
+    }
+    catch (IOError e) {
+      return false;
+    }
+
+    return true;
   }
 
   private void setup_gtk_application()
@@ -79,7 +103,7 @@
 
   private void find_iface_path(DBusConnection connection, string name, string path, string target_iface, out string found_path)
   {
-  	found_path = null;
+    found_path = null;
     DBusNodeInfo node = null;
 
     try {
@@ -120,4 +144,56 @@
       }
     }
   }
-}
\ No newline at end of file
+
+  private void setup_bamf_application()
+  {
+    this.bamf_application = null;
+    var desktop_app = this.owner.app_info as DesktopAppInfo;
+
+    if (desktop_app == null)
+      return;
+
+    foreach (var app in Bamf.Matcher.get_default().get_applications()) {
+      if (app.get_desktop_file() == desktop_app.get_filename()) {
+        this.bamf_application = app;
+        break;
+      }
+    }
+  }
+
+  private bool activate_bamf_appplication(uint timestamp)
+  {
+    this.setup_bamf_application();
+
+    if (this.bamf_application == null)
+      return false;
+
+    bool focused = false;
+    var dpy = Gdk.Display.get_default();
+
+    foreach (var win in this.bamf_application.get_windows()) {
+      X.Window xid = 0;
+
+      if (win is Bamf.Window) {
+        if (win.get_window_type() != Bamf.WindowType.NORMAL)
+          continue;
+
+        xid = win.get_xid();
+      }
+      else if (win is Bamf.Tab) {
+        xid = (X.Window) (win as Bamf.Tab).get_xid();
+      }
+
+      if (xid > 0) {
+        var xwin = Gdk.X11Window.foreign_new_for_display(dpy, xid);
+
+        if (xwin != null) {
+          xwin.focus(timestamp);
+          focused = true;
+        }
+      }
+    }
+
+    return focused;
+  }
+}

=== modified file 'src/player-controller.vala'
--- src/player-controller.vala	2013-04-03 17:22:31 +0000
+++ src/player-controller.vala	2013-04-03 17:22:31 +0000
@@ -45,7 +45,7 @@
   public string dbus_name { get; set;}
   public ArrayList<PlayerItem> custom_items;
   public Mpris2Controller mpris_bridge;
-  public GtkApplicationPlayer gtk_app_player;
+  public PlayerActivator player_activator;
   public AppInfo? app_info { get; set;}
   public int menu_offset { get; set;}
   public string icon_name { get; set; }
@@ -150,7 +150,7 @@
     debug ( " establish mpris connection - use playlists value = %s ",
             this.use_playlists.to_string() );
     this.mpris_bridge = new Mpris2Controller (this);
-    this.gtk_app_player = new GtkApplicationPlayer (this);
+    this.player_activator = new PlayerActivator (this);
     this.determine_state ();
   }