Merge lp:~voldyman/switchboard-plug-default-applications/rework into lp:switchboard-plug-default-applications

Proposed by Akshay Shekher
Status: Needs review
Proposed branch: lp:~voldyman/switchboard-plug-default-applications/rework
Merge into: lp:switchboard-plug-default-applications
Diff against target: 313 lines (+156/-77)
5 files modified
LICENSE (+1/-1)
src/CMakeLists.txt (+3/-2)
src/Utils.vala (+1/-1)
src/Worker.vala (+84/-0)
src/default-plug.vala (+67/-73)
To merge this branch: bzr merge lp:~voldyman/switchboard-plug-default-applications/rework
Reviewer Review Type Date Requested Status
Julián Unrrein (community) code style Needs Fixing
Review via email: mp+193734@code.launchpad.net

Description of the change

the new branch updated the license year to 2013 and

switches form creating new threads to using a thread pool, which forces reuse of threads (making new threads is expensive)

To post a comment you must log in.
Revision history for this message
Julián Unrrein (junrrein) wrote :

Diff line 125, 128: Insert a space before the opening parenthesis.

Diff line 114: Insert a colon inside the warning, after "ocurred".

Diff line 128: Would it be better to use "message" or "warning"?

review: Needs Fixing (code style)

Unmerged revisions

65. By Akshay Shekher

Updated License year & Switched to thread pool instead of making new threads for each change.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'LICENSE'
2--- LICENSE 2013-02-12 20:07:40 +0000
3+++ LICENSE 2013-11-04 07:13:22 +0000
4@@ -1,4 +1,4 @@
5-Copyright (C) 2011-2012 elementary Developers
6+Copyright (C) 2011-2013 elementary Developers
7 This program is free software: you can redistribute it and/or modify it
8 under the terms of the GNU Lesser General Public License version 3, as published
9 by the Free Software Foundation.
10
11=== modified file 'src/CMakeLists.txt'
12--- src/CMakeLists.txt 2013-02-16 05:53:32 +0000
13+++ src/CMakeLists.txt 2013-11-04 07:13:22 +0000
14@@ -9,13 +9,14 @@
15
16 find_package (Vala REQUIRED)
17 include (ValaVersion)
18-ensure_vala_version ("0.16.0" MINIMUM)
19+ensure_vala_version ("0.18.0" MINIMUM)
20
21 include (ValaPrecompile)
22 # Add all your vala files and requires packages to the List below to include them in the build
23 vala_precompile (VALA_C
24 default-plug.vala
25- utils.vala
26+ Utils.vala
27+ Worker.vala
28 ${CMAKE_CURRENT_BINARY_DIR}/config.vala
29 PACKAGES
30 pantheon
31
32=== renamed file 'src/utils.vala' => 'src/Utils.vala'
33--- src/utils.vala 2013-02-16 05:35:14 +0000
34+++ src/Utils.vala 2013-11-04 07:13:22 +0000
35@@ -1,7 +1,7 @@
36 /***
37 BEGIN LICENSE
38
39- Copyright (C) 2011-2012 elementary Developers
40+ Copyright (C) 2011-2013 elementary Developers
41 This program is free software: you can redistribute it and/or modify it
42 under the terms of the GNU Lesser General Public License version 3, as published
43 by the Free Software Foundation.
44
45=== added file 'src/Worker.vala'
46--- src/Worker.vala 1970-01-01 00:00:00 +0000
47+++ src/Worker.vala 2013-11-04 07:13:22 +0000
48@@ -0,0 +1,84 @@
49+/***
50+ BEGIN LICENSE
51+
52+ Copyright (C) 2011-2013 elementary Developers
53+ This program is free software: you can redistribute it and/or modify it
54+ under the terms of the GNU Lesser General Public License version 3, as published
55+ by the Free Software Foundation.
56+
57+ This program is distributed in the hope that it will be useful, but
58+ WITHOUT ANY WARRANTY; without even the implied warranties of
59+ MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
60+ PURPOSE. See the GNU General Public License for more details.
61+
62+ You should have received a copy of the GNU General Public License along
63+ with this program. If not, see <http://www.gnu.org/licenses/>
64+
65+ END LICENSE
66+ Written By: Akshay Shekher <voldyman666@gmail.com>
67+
68+***/
69+
70+public class Worker {
71+
72+ GLib.AppInfo app_from;
73+ GLib.AppInfo app_to;
74+ string app_type;
75+
76+ public Worker (GLib.AppInfo change_from, GLib.AppInfo change_to,
77+ string item_type) {
78+
79+ this.app_from = change_from;
80+ this.app_to = change_to;
81+ app_type = item_type;
82+ }
83+
84+ public void run () {
85+ change_default ();
86+ }
87+
88+ private void change_default () {
89+
90+ map_types_to_app (get_types_for_app (app_type), app_to);
91+
92+ /* the code below implements ->
93+ string[] old_types = app_from.get_supported_types ();
94+ map_types_to_app (old_types, app_to);
95+
96+ The function AppInfo.get_supported_types () is not present in the current glib
97+ and will be used when we switch to the newer version.
98+ */
99+ var old_app_keyfile = new KeyFile ();
100+
101+ try {
102+ old_app_keyfile.load_from_file (((DesktopAppInfo) app_from).filename,
103+ KeyFileFlags.NONE);
104+ } catch (Error e) {
105+ warning ("An error occured %s".printf (e.message));
106+ }
107+
108+ string oldapp_types;
109+
110+ try {
111+ oldapp_types = old_app_keyfile.get_string (KeyFileDesktop.GROUP,
112+ KeyFileDesktop.KEY_MIME_TYPE);
113+ } catch (Error e) {
114+ warning ("An error occured %s".printf (e.message));
115+ oldapp_types = "";
116+ }
117+ // end block
118+
119+ map_types_to_app (oldapp_types.split (","), app_to);
120+ }
121+
122+ private static void map_types_to_app (string[] types, GLib.AppInfo app) {
123+ try {
124+ for (int i=0; i < types.length; i++) {
125+ app.set_as_default_for_type(types[i]);
126+ }
127+ } catch (GLib.Error e) {
128+ stdout.printf("Error: %s\n", e.message);
129+ }
130+
131+ }
132+}
133\ No newline at end of file
134
135=== modified file 'src/default-plug.vala'
136--- src/default-plug.vala 2013-04-28 09:16:45 +0000
137+++ src/default-plug.vala 2013-11-04 07:13:22 +0000
138@@ -1,7 +1,7 @@
139 /***
140 BEGIN LICENSE
141
142- Copyright (C) 2011-2012 elementary Developers
143+ Copyright (C) 2011-2013 elementary Developers
144 This program is free software: you can redistribute it and/or modify it
145 under the terms of the GNU Lesser General Public License version 3, as published
146 by the Free Software Foundation.
147@@ -65,14 +65,29 @@
148 GLib.AppInfo te_old;
149 GLib.AppInfo fb_old;
150
151+ /* make a thread pool to avoid blocking the UI when processing */
152+ GLib.ThreadPool<Worker> pool;
153+
154 public DefaultPlug () {
155
156+ try {
157+ pool = new ThreadPool<Worker>.with_owned_data (worker_func, 3, false);
158+ } catch (ThreadError e) {
159+ critical ("Unable to create a thread pool: %s", e.message);
160+ }
161+
162+ setup_ui ();
163+ cache_apps ();
164+ connect_signals ();
165+ }
166+
167+ private void setup_ui () {
168 var grid = new Gtk.Grid ();
169 grid.halign = Gtk.Align.CENTER;
170 grid.set_column_homogeneous (false);
171 grid.set_row_spacing (6);
172 grid.set_column_spacing (10);
173-
174+
175 //space between the two columns
176 int margin_columns = 40;
177 grid.margin_left = margin_columns;
178@@ -132,77 +147,56 @@
179 grid.attach (fb_label, 2, 3, 1, 1);
180 grid.attach (fb_chooser, 3, 3, 1, 1);
181
182- cache_apps ();
183-
184- wb_chooser.changed.connect ( () => run_in_thread ( () => {
185- change_default(wb_old, wb_chooser.get_app_info (), "web_browser");
186- return null;
187- }));
188-
189- ec_chooser.changed.connect ( () => run_in_thread ( () => {
190- change_default(ec_old, ec_chooser.get_app_info (), "email_client");
191- return null;
192- }));
193-
194- c_chooser.changed.connect ( () => run_in_thread ( () => {
195- change_default(c_old, c_chooser.get_app_info (), "calendar");
196- return null;
197- }));
198-
199- vp_chooser.changed.connect ( () => run_in_thread ( () => {
200- change_default(vp_old, vp_chooser.get_app_info (), "video_player");
201- return null;
202- }));
203-
204- mp_chooser.changed.connect ( () => run_in_thread ( () => {
205- change_default(mp_old, mp_chooser.get_app_info (), "music_player");
206- return null;
207- }));
208-
209- iv_chooser.changed.connect ( () => run_in_thread ( () => {
210- change_default (iv_old, iv_chooser.get_app_info (), "image_viewer");
211- return null;
212- }));
213- te_chooser.changed.connect ( () => run_in_thread ( () => {
214- change_default(te_old, te_chooser.get_app_info (), "text_editor");
215- return null;
216- }));
217- fb_chooser.changed.connect ( () => run_in_thread ( () => {
218- change_default(fb_old, fb_chooser.get_app_info (), "file_browser");
219- return null;
220- }));
221-
222 add (grid);
223-
224- }
225- private void run_in_thread (ThreadFunc<void*> func) {
226- new Thread<void*> (null, func);
227- }
228- public void change_default (GLib.AppInfo old_app, GLib.AppInfo new_app, string item_type) {
229- map_types_to_app (get_types_for_app (item_type), new_app);
230-
231- /* the code below implements ->
232- string[] old_types = old_app.get_supported_types ();
233- map_types_to_app (old_types, new_app);
234- The function AppInfo.get_supported_types () is not present in the current glib
235- and will be used when we switch to the newer version.
236- */
237- var old_app_keyfile = new KeyFile ();
238- try {
239- old_app_keyfile.load_from_file (((DesktopAppInfo) old_app).filename, KeyFileFlags.NONE);
240- } catch (Error e) {
241- warning ("An error occured %s".printf (e.message));
242- }
243- string oldapp_types;
244- try {
245- oldapp_types = old_app_keyfile.get_string (KeyFileDesktop.GROUP, KeyFileDesktop.KEY_MIME_TYPE);
246- } catch (Error e) {
247- warning ("An error occured %s".printf (e.message));
248- oldapp_types = "";
249- }
250- //end block
251- map_types_to_app (oldapp_types.split (","), new_app);
252-
253+ }
254+
255+
256+ private void connect_signals () {
257+
258+ wb_chooser.changed.connect ( () => {
259+ add_task (wb_old, wb_chooser.get_app_info (), "web_browser");
260+ });
261+
262+ ec_chooser.changed.connect ( () => {
263+ add_task (ec_old, ec_chooser.get_app_info (), "email_client");
264+ });
265+
266+ c_chooser.changed.connect ( () => {
267+ add_task (c_old, c_chooser.get_app_info (), "calendar");
268+ });
269+
270+ vp_chooser.changed.connect ( () => {
271+ add_task (vp_old, vp_chooser.get_app_info (), "video_player");
272+ });
273+
274+ mp_chooser.changed.connect ( () => {
275+ add_task (mp_old, mp_chooser.get_app_info (), "music_player");
276+ });
277+
278+ iv_chooser.changed.connect ( () => {
279+ add_task (iv_old, iv_chooser.get_app_info (), "image_viewer");
280+ });
281+
282+ te_chooser.changed.connect ( () => {
283+ add_task (te_old, te_chooser.get_app_info (), "text_editor");
284+ });
285+
286+ fb_chooser.changed.connect ( () => {
287+ add_task (fb_old, fb_chooser.get_app_info (), "file_browser");
288+ });
289+ }
290+
291+ private void add_task (GLib.AppInfo app_from, GLib.AppInfo app_to,
292+ string item_type) {
293+ try {
294+ pool.add (new Worker (app_from, app_to, item_type));
295+ } catch (ThreadError e) {
296+ warning ("Could not add the worker to pool: %s", e.message);
297+ }
298+ }
299+
300+ private void worker_func (owned Worker task) {
301+ task.run ();
302 cache_apps ();
303 }
304
305@@ -210,7 +204,7 @@
306 /* Cache the AppInfo of the old default apps */
307 wb_old = wb_chooser.get_app_info ();
308 ec_old = ec_chooser.get_app_info ();
309- c_old = c_chooser.get_app_info ();
310+ c_old = c_chooser.get_app_info ();
311 vp_old = vp_chooser.get_app_info ();
312 mp_old = mp_chooser.get_app_info ();
313 iv_old = iv_chooser.get_app_info ();

Subscribers

People subscribed via source and target branches

to all changes: