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

Proposed by Marco Trevisan (Treviño)
Status: Merged
Approved by: Lars Karlitski
Approved revision: 351
Merged at revision: 347
Proposed branch: lp:~3v1n0/indicator-sound/bamf-player-activation
Merge into: lp:indicator-sound/13.04
Prerequisite: lp:~3v1n0/indicator-sound/gtk-application-player-activate
Diff against target: 243 lines (+100/-19)
6 files modified
configure.ac (+2/-0)
debian/control (+1/-0)
src/Makefile.am (+3/-1)
src/metadata-menu-item.vala (+1/-1)
src/player-activator.vala (+91/-15)
src/player-controller.vala (+2/-2)
To merge this branch: bzr merge lp:~3v1n0/indicator-sound/bamf-player-activation
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Lars Karlitski (community) Approve
Mathieu Trudel-Lapierre Abstain
Review via email: mp+156920@code.launchpad.net

This proposal supersedes a proposal from 2013-04-02.

Commit message

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.

Description of the change

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.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Mathieu Trudel-Lapierre (cyphermox) wrote :

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
Revision history for this message
Mathieu Trudel-Lapierre (cyphermox) wrote :

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

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

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.

Revision history for this message
Lars Karlitski (larsu) wrote :

Looks good to me, thanks Marco.

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
52PKG_CHECK_MODULES(SOUNDSERVICE, dbusmenu-glib-0.4 >= $DBUSMENUGLIB_REQUIRED_VERSION52PKG_CHECK_MODULES(SOUNDSERVICE, dbusmenu-glib-0.4 >= $DBUSMENUGLIB_REQUIRED_VERSION
53 indicator3-0.453 indicator3-0.4
54 gee-1.054 gee-1.0
55 gdk-x11-3.0
55 gio-unix-2.056 gio-unix-2.0
57 libbamf3
56 libxml-2.0)58 libxml-2.0)
5759
58AC_SUBST(APPLET_CFLAGS)60AC_SUBST(APPLET_CFLAGS)
5961
=== 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 @@
10 gnome-common,10 gnome-common,
11 autotools-dev,11 autotools-dev,
12 valac-0.18,12 valac-0.18,
13 libbamf3-dev,
13 libglib2.0-dev (>= 2.22.3),14 libglib2.0-dev (>= 2.22.3),
14 libgtk-3-dev,15 libgtk-3-dev,
15 libdbusmenu-glib-dev (>= 0.5.90),16 libdbusmenu-glib-dev (>= 0.5.90),
1617
=== 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 @@
68 playlists-menu-item.vala \68 playlists-menu-item.vala \
69 freedesktop-interfaces.vala \69 freedesktop-interfaces.vala \
70 fetch-file.vala \70 fetch-file.vala \
71 gtk-application-player.vala71 player-activator.vala
7272
73music_bridge_VALAFLAGS = \73music_bridge_VALAFLAGS = \
74 --ccode \74 --ccode \
@@ -83,7 +83,9 @@
83 --pkg gio-2.0 \83 --pkg gio-2.0 \
84 --pkg gio-unix-2.0 \84 --pkg gio-unix-2.0 \
85 --pkg gdk-3.0 \85 --pkg gdk-3.0 \
86 --pkg gdk-x11-3.0 \
86 --pkg gdk-pixbuf-2.0 \87 --pkg gdk-pixbuf-2.0 \
88 --pkg libbamf3 \
87 --pkg libxml-2.089 --pkg libxml-2.0
8890
89 $(MAINTAINER_VALAFLAGS)91 $(MAINTAINER_VALAFLAGS)
9092
=== 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 @@
177 this.owner.instantiate(timestamp);177 this.owner.instantiate(timestamp);
178 }178 }
179 else if (this.owner.current_state == PlayerController.state.CONNECTED) {179 else if (this.owner.current_state == PlayerController.state.CONNECTED) {
180 this.owner.gtk_app_player.activate(timestamp);180 this.owner.player_activator.activate(timestamp);
181 this.owner.mpris_bridge.expose(timestamp);181 this.owner.mpris_bridge.expose(timestamp);
182 }182 }
183 }183 }
184184
=== 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 @@
4Authors:4Authors:
5 Marco Trevisan <marco.trevisan@canonical.com>5 Marco Trevisan <marco.trevisan@canonical.com>
66
7This program is free software: you can redistribute it and/or modify it 7This program is free software: you can redistribute it and/or modify it
8under the terms of the GNU General Public License version 3, as published 8under the terms of the GNU General Public License version 3, as published
9by the Free Software Foundation.9by the Free Software Foundation.
1010
11This program is distributed in the hope that it will be useful, but 11This program is distributed in the hope that it will be useful, but
12WITHOUT ANY WARRANTY; without even the implied warranties of 12WITHOUT ANY WARRANTY; without even the implied warranties of
13MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR 13MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
14PURPOSE. See the GNU General Public License for more details.14PURPOSE. See the GNU General Public License for more details.
1515
16You should have received a copy of the GNU General Public License along 16You should have received a copy of the GNU General Public License along
17with this program. If not, see <http://www.gnu.org/licenses/>.17with this program. If not, see <http://www.gnu.org/licenses/>.
18*/18*/
1919
@@ -22,24 +22,44 @@
22 public abstract void Activate(GLib.HashTable<string, Variant?> platform_data) throws IOError;22 public abstract void Activate(GLib.HashTable<string, Variant?> platform_data) throws IOError;
23}23}
2424
25public class GtkApplicationPlayer : GLib.Object25public class PlayerActivator : GLib.Object
26{26{
27 public PlayerController owner {get; construct;}27 public PlayerController owner {get; construct;}
2828
29 private bool gtk_application_searched = false;29 private bool gtk_application_searched = false;
30 private DBusGtkApplication gtk_application;30 private DBusGtkApplication gtk_application;
3131 private Bamf.Application bamf_application;
32 public GtkApplicationPlayer(PlayerController ctrl)32
33 private const uint MAX_BAMF_APPLICATION_WAIT_MS = 1000;
34 private int64 last_check_time;
35
36 public PlayerActivator(PlayerController ctrl)
33 {37 {
34 GLib.Object(owner: ctrl);38 GLib.Object(owner: ctrl);
35 }39 }
3640
37 public void activate(uint timestamp)41 public void activate(uint timestamp)
38 {42 {
43 if (!activate_gtk_appplication(timestamp)) {
44 if (!activate_bamf_appplication(timestamp)) {
45 // Let's wait BAMF to update its windows list
46 this.last_check_time = get_monotonic_time();
47
48 Idle.add(() => {
49 bool activated = activate_bamf_appplication(timestamp);
50 int64 waited = (get_monotonic_time() - this.last_check_time) / 1000;
51 return !activated && waited < MAX_BAMF_APPLICATION_WAIT_MS;
52 });
53 }
54 }
55 }
56
57 private bool activate_gtk_appplication(uint timestamp)
58 {
39 this.setup_gtk_application();59 this.setup_gtk_application();
4060
41 if (this.gtk_application == null) {61 if (this.gtk_application == null) {
42 return;62 return false;
43 }63 }
4464
45 var context = Gdk.Display.get_default().get_app_launch_context();65 var context = Gdk.Display.get_default().get_app_launch_context();
@@ -49,9 +69,13 @@
49 data["desktop-startup-id"] = context.get_startup_notify_id(this.owner.app_info, new GLib.List<GLib.File>());69 data["desktop-startup-id"] = context.get_startup_notify_id(this.owner.app_info, new GLib.List<GLib.File>());
5070
51 try {71 try {
52 this.gtk_application.Activate(data);72 this.gtk_application.Activate(data);
53 }73 }
54 catch (IOError e) {}74 catch (IOError e) {
75 return false;
76 }
77
78 return true;
55 }79 }
5680
57 private void setup_gtk_application()81 private void setup_gtk_application()
@@ -79,7 +103,7 @@
79103
80 private void find_iface_path(DBusConnection connection, string name, string path, string target_iface, out string found_path)104 private void find_iface_path(DBusConnection connection, string name, string path, string target_iface, out string found_path)
81 {105 {
82 found_path = null;106 found_path = null;
83 DBusNodeInfo node = null;107 DBusNodeInfo node = null;
84108
85 try {109 try {
@@ -120,4 +144,56 @@
120 }144 }
121 }145 }
122 }146 }
123}
124\ No newline at end of file147\ No newline at end of file
148
149 private void setup_bamf_application()
150 {
151 this.bamf_application = null;
152 var desktop_app = this.owner.app_info as DesktopAppInfo;
153
154 if (desktop_app == null)
155 return;
156
157 foreach (var app in Bamf.Matcher.get_default().get_applications()) {
158 if (app.get_desktop_file() == desktop_app.get_filename()) {
159 this.bamf_application = app;
160 break;
161 }
162 }
163 }
164
165 private bool activate_bamf_appplication(uint timestamp)
166 {
167 this.setup_bamf_application();
168
169 if (this.bamf_application == null)
170 return false;
171
172 bool focused = false;
173 var dpy = Gdk.Display.get_default();
174
175 foreach (var win in this.bamf_application.get_windows()) {
176 X.Window xid = 0;
177
178 if (win is Bamf.Window) {
179 if (win.get_window_type() != Bamf.WindowType.NORMAL)
180 continue;
181
182 xid = win.get_xid();
183 }
184 else if (win is Bamf.Tab) {
185 xid = (X.Window) (win as Bamf.Tab).get_xid();
186 }
187
188 if (xid > 0) {
189 var xwin = Gdk.X11Window.foreign_new_for_display(dpy, xid);
190
191 if (xwin != null) {
192 xwin.focus(timestamp);
193 focused = true;
194 }
195 }
196 }
197
198 return focused;
199 }
200}
125201
=== 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 @@
45 public string dbus_name { get; set;}45 public string dbus_name { get; set;}
46 public ArrayList<PlayerItem> custom_items;46 public ArrayList<PlayerItem> custom_items;
47 public Mpris2Controller mpris_bridge;47 public Mpris2Controller mpris_bridge;
48 public GtkApplicationPlayer gtk_app_player;48 public PlayerActivator player_activator;
49 public AppInfo? app_info { get; set;}49 public AppInfo? app_info { get; set;}
50 public int menu_offset { get; set;}50 public int menu_offset { get; set;}
51 public string icon_name { get; set; }51 public string icon_name { get; set; }
@@ -150,7 +150,7 @@
150 debug ( " establish mpris connection - use playlists value = %s ",150 debug ( " establish mpris connection - use playlists value = %s ",
151 this.use_playlists.to_string() );151 this.use_playlists.to_string() );
152 this.mpris_bridge = new Mpris2Controller (this);152 this.mpris_bridge = new Mpris2Controller (this);
153 this.gtk_app_player = new GtkApplicationPlayer (this);153 this.player_activator = new PlayerActivator (this);
154 this.determine_state ();154 this.determine_state ();
155 }155 }
156 156

Subscribers

People subscribed via source and target branches