Ubuntu Application Launcher

Merge lp:~ted/ubuntu-app-launch/disable-handshake-wait into lp:ubuntu-app-launch/16.10

  1. disable-handshake-wait
  2. Merge into trunk.16.10
Proposed by Ted Gould
Status: Merged
Approved by: Charles Kerr
Approved revision: 251
Merged at revision: 248
Proposed branch: lp:~ted/ubuntu-app-launch/disable-handshake-wait
Merge into: lp:ubuntu-app-launch/16.10
Diff against target: 257 lines (+53/-17)
11 files modified
helpers.c (+2/-2)
helpers.h (+2/-1)
libubuntu-app-launch/application-impl-click.cpp (+3/-3)
libubuntu-app-launch/click-exec.c (+2/-2)
libubuntu-app-launch/click-exec.h (+1/-1)
libubuntu-app-launch/desktop-exec.c (+2/-2)
libubuntu-app-launch/desktop-exec.h (+1/-1)
libubuntu-app-launch/registry-impl.cpp (+22/-1)
libubuntu-app-launch/registry-impl.h (+6/-0)
libubuntu-app-launch/ubuntu-app-launch.cpp (+10/-2)
tests/helper-handshake-test.cc (+2/-2)
To merge this branch: bzr merge lp:~ted/ubuntu-app-launch/disable-handshake-wait
Reviewer Review Type Date Requested Status
Charles Kerr (community) Approve
unity-api-1-bot continuous-integration Needs Fixing
Review via email: mp+304790@code.launchpad.net

Commit message

Disable handshake wait with clients doing handshaking

Description of the change

Make it so that when an application registers to get application startup signals we stop waiting on responses for cases where its mainloop is blocked and might not be able to send them.

To post a comment you must log in.
Revision history for this message
unity-api-1-bot (unity-api-1-bot) wrote :

FAILED: Continuous integration, rev:251
https://jenkins.canonical.com/unity-api-1/job/lp-ubuntu-app-launch-ci/91/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build/546/console
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/552
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=vivid+overlay/389/console
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/389
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/389/artifact/output/*zip*/output.zip
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=yakkety/389/console
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=vivid+overlay/389/console
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/389/console
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=yakkety/389/console
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=vivid+overlay/389/console
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/389/console
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=yakkety/389/console

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-ubuntu-app-launch-ci/91/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Charles Kerr (charlesk) wrote :

Looks like a reasonable change for the workaround, agree wrt this being better as a nonstatic in the future.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'helpers.c'
2--- helpers.c 2016-08-16 18:13:06 +0000
3+++ helpers.c 2016-09-02 14:11:47 +0000
4@@ -503,7 +503,7 @@
5 }
6
7 handshake_t *
8-starting_handshake_start (const gchar * app_id)
9+starting_handshake_start (const gchar * app_id, int timeout_s)
10 {
11 GError * error = NULL;
12 handshake_t * handshake = g_new0(handshake_t, 1);
13@@ -541,7 +541,7 @@
14 &error);
15
16 /* Really, Unity? */
17- handshake->timeout = g_timeout_source_new_seconds(1);
18+ handshake->timeout = g_timeout_source_new_seconds(timeout_s);
19 g_source_set_callback(handshake->timeout, unity_too_slow_cb, handshake, NULL);
20 g_source_attach(handshake->timeout, context);
21
22
23=== modified file 'helpers.h'
24--- helpers.h 2015-07-15 02:32:54 +0000
25+++ helpers.h 2016-09-02 14:11:47 +0000
26@@ -45,7 +45,8 @@
27 void env_handle_finish (EnvHandle * handle);
28
29 typedef struct _handshake_t handshake_t;
30-handshake_t * starting_handshake_start (const gchar * app_id);
31+handshake_t * starting_handshake_start (const gchar * app_id,
32+ int timeout_s);
33 void starting_handshake_wait (handshake_t * handshake);
34
35 GDBusConnection * cgroup_manager_connection (void);
36
37=== modified file 'libubuntu-app-launch/application-impl-click.cpp'
38--- libubuntu-app-launch/application-impl-click.cpp 2016-08-17 15:24:23 +0000
39+++ libubuntu-app-launch/application-impl-click.cpp 2016-09-02 14:11:47 +0000
40@@ -75,7 +75,7 @@
41
42 std::list<AppID::AppName> manifestApps(const std::shared_ptr<JsonObject>& manifest)
43 {
44- JsonObject *hooks = nullptr;
45+ JsonObject* hooks = nullptr;
46 if (!json_object_has_member(manifest.get(), "hooks") ||
47 (hooks = json_object_get_object_member(manifest.get(), "hooks")) == nullptr)
48 {
49@@ -108,14 +108,14 @@
50 const std::string& app,
51 const std::string& clickDir)
52 {
53- JsonObject *hooks = nullptr;
54+ JsonObject* hooks = nullptr;
55 if (!json_object_has_member(manifest.get(), "hooks") ||
56 (hooks = json_object_get_object_member(manifest.get(), "hooks")) == nullptr)
57 {
58 throw std::runtime_error("Manifest for application '" + app + "' does not have a 'hooks' field");
59 }
60
61- JsonObject *hooklist = nullptr;
62+ JsonObject* hooklist = nullptr;
63 if (!json_object_has_member(hooks, app.c_str()) ||
64 (hooklist = json_object_get_object_member(hooks, app.c_str())) == nullptr)
65 {
66
67=== modified file 'libubuntu-app-launch/click-exec.c'
68--- libubuntu-app-launch/click-exec.c 2014-09-17 14:11:59 +0000
69+++ libubuntu-app-launch/click-exec.c 2016-09-02 14:11:47 +0000
70@@ -41,7 +41,7 @@
71 */
72
73 gboolean
74-click_task_setup (GDBusConnection * bus, const gchar * app_id, EnvHandle * handle)
75+click_task_setup (GDBusConnection * bus, const gchar * app_id, EnvHandle * handle, int timeout_s)
76 {
77 g_return_val_if_fail(bus != NULL, FALSE);
78 g_return_val_if_fail(app_id != NULL, FALSE);
79@@ -51,7 +51,7 @@
80
81 GError * error = NULL;
82
83- handshake_t * handshake = starting_handshake_start(app_id);
84+ handshake_t * handshake = starting_handshake_start(app_id, timeout_s);
85 if (handshake == NULL) {
86 g_warning("Unable to setup starting handshake");
87 }
88
89=== modified file 'libubuntu-app-launch/click-exec.h'
90--- libubuntu-app-launch/click-exec.h 2014-08-22 20:59:55 +0000
91+++ libubuntu-app-launch/click-exec.h 2016-09-02 14:11:47 +0000
92@@ -20,6 +20,6 @@
93 #include <glib.h>
94 #include "helpers.h"
95
96-gboolean click_task_setup (GDBusConnection * bus, const gchar * app_id, EnvHandle * envhandle);
97+gboolean click_task_setup (GDBusConnection * bus, const gchar * app_id, EnvHandle * envhandle, int timeout_s);
98
99
100
101=== modified file 'libubuntu-app-launch/desktop-exec.c'
102--- libubuntu-app-launch/desktop-exec.c 2016-01-26 21:17:25 +0000
103+++ libubuntu-app-launch/desktop-exec.c 2016-09-02 14:11:47 +0000
104@@ -117,7 +117,7 @@
105 }
106
107 gboolean
108-desktop_task_setup (GDBusConnection * bus, const gchar * app_id, EnvHandle * handle, gboolean is_libertine)
109+desktop_task_setup (GDBusConnection * bus, const gchar * app_id, EnvHandle * handle, gboolean is_libertine, int timeout_s)
110 {
111 if (app_id == NULL) {
112 g_error("No APP_ID environment variable defined");
113@@ -126,7 +126,7 @@
114
115 ual_tracepoint(desktop_start, app_id);
116
117- handshake_t * handshake = starting_handshake_start(app_id);
118+ handshake_t * handshake = starting_handshake_start(app_id, timeout_s);
119 if (handshake == NULL) {
120 g_warning("Unable to setup starting handshake");
121 }
122
123=== modified file 'libubuntu-app-launch/desktop-exec.h'
124--- libubuntu-app-launch/desktop-exec.h 2015-07-15 02:12:04 +0000
125+++ libubuntu-app-launch/desktop-exec.h 2016-09-02 14:11:47 +0000
126@@ -22,5 +22,5 @@
127 #include <glib.h>
128 #include "helpers.h"
129
130-gboolean desktop_task_setup (GDBusConnection * bus, const gchar * appid, EnvHandle * envhandle, gboolean is_libertine);
131+gboolean desktop_task_setup (GDBusConnection * bus, const gchar * appid, EnvHandle * envhandle, gboolean is_libertine, int timeout_s);
132
133
134=== modified file 'libubuntu-app-launch/registry-impl.cpp'
135--- libubuntu-app-launch/registry-impl.cpp 2016-08-17 15:24:23 +0000
136+++ libubuntu-app-launch/registry-impl.cpp 2016-09-02 14:11:47 +0000
137@@ -116,7 +116,7 @@
138
139 auto retval = std::shared_ptr<JsonObject>(json_node_dup_object(node), json_object_unref);
140
141-#if JSON_CHECK_VERSION(1,1,2)
142+#if JSON_CHECK_VERSION(1, 1, 2)
143 // Not available in json-glib 1.0, so must leak there.
144 json_node_unref(node);
145 #endif
146@@ -520,5 +520,26 @@
147 }
148 #endif
149
150+/** App start watching, if we're registered for the signal we
151+ can't wait on it. We are making this static right now because
152+ we need it to go across the C and C++ APIs smoothly, and those
153+ can end up with different registry objects. Long term, this
154+ should become a member variable. */
155+static bool watchingAppStarting_ = false;
156+
157+/** Variable to track if this program is watching app startup
158+ so that we can know to not wait on the response to that. */
159+void Registry::Impl::watchingAppStarting(bool rWatching)
160+{
161+ watchingAppStarting_ = rWatching;
162+}
163+
164+/** Accessor for the internal variable to know whether an app
165+ is watching for app startup */
166+bool Registry::Impl::isWatchingAppStarting()
167+{
168+ return watchingAppStarting_;
169+}
170+
171 } // namespace app_launch
172 } // namespace ubuntu
173
174=== modified file 'libubuntu-app-launch/registry-impl.h'
175--- libubuntu-app-launch/registry-impl.h 2016-08-17 15:24:23 +0000
176+++ libubuntu-app-launch/registry-impl.h 2016-09-02 14:11:47 +0000
177@@ -70,6 +70,12 @@
178 std::list<std::string> upstartInstancesForJob(const std::string& job);
179 std::string upstartJobPath(const std::string& job);
180
181+ /* Signal Hints */
182+ /* NOTE: Static because we don't have registry instances in the C
183+ code right now. We want these to not be static in the future */
184+ static void watchingAppStarting(bool rWatching);
185+ static bool isWatchingAppStarting();
186+
187 private:
188 Registry* _registry;
189 #if 0
190
191=== modified file 'libubuntu-app-launch/ubuntu-app-launch.cpp'
192--- libubuntu-app-launch/ubuntu-app-launch.cpp 2016-08-25 14:20:38 +0000
193+++ libubuntu-app-launch/ubuntu-app-launch.cpp 2016-09-02 14:11:47 +0000
194@@ -42,6 +42,7 @@
195 #include "application.h"
196 #include "appid.h"
197 #include "registry.h"
198+#include "registry-impl.h"
199
200 static void apps_for_job (GDBusConnection * con, const gchar * name, GArray * apps, gboolean truncate_legacy);
201 static void free_helper (gpointer value);
202@@ -299,11 +300,16 @@
203 g_variant_builder_add_value(&builder, g_variant_new_string("QT_LOAD_TESTABILITY=1"));
204 }
205
206+ int timeout = 1;
207+ if (ubuntu::app_launch::Registry::Impl::isWatchingAppStarting()) {
208+ timeout = 0;
209+ }
210+
211 gboolean setup_complete = FALSE;
212 if (click) {
213- setup_complete = click_task_setup(con, appid, (EnvHandle*)&builder);
214+ setup_complete = click_task_setup(con, appid, (EnvHandle*)&builder, timeout);
215 } else {
216- setup_complete = desktop_task_setup(con, appid, (EnvHandle*)&builder, libertine);
217+ setup_complete = desktop_task_setup(con, appid, (EnvHandle*)&builder, libertine, timeout);
218 }
219
220 if (setup_complete) {
221@@ -820,6 +826,7 @@
222 gboolean
223 ubuntu_app_launch_observer_add_app_starting (UbuntuAppLaunchAppObserver observer, gpointer user_data)
224 {
225+ ubuntu::app_launch::Registry::Impl::watchingAppStarting(true);
226 return add_session_generic(observer, user_data, "UnityStartingBroadcast", &starting_array, starting_signal_cb);
227 }
228
229@@ -1011,6 +1018,7 @@
230 gboolean
231 ubuntu_app_launch_observer_delete_app_starting (UbuntuAppLaunchAppObserver observer, gpointer user_data)
232 {
233+ ubuntu::app_launch::Registry::Impl::watchingAppStarting(false);
234 return delete_app_generic(observer, user_data, &starting_array);
235 }
236
237
238=== modified file 'tests/helper-handshake-test.cc'
239--- tests/helper-handshake-test.cc 2015-12-18 17:21:20 +0000
240+++ tests/helper-handshake-test.cc 2016-09-02 14:11:47 +0000
241@@ -77,7 +77,7 @@
242 GDBusConnection * con = g_bus_get_sync(G_BUS_TYPE_SESSION, NULL, NULL);
243 guint filter = g_dbus_connection_add_filter(con, filter_func, this, NULL);
244
245- handshake_t * handshake = starting_handshake_start("fooapp");
246+ handshake_t * handshake = starting_handshake_start("fooapp", 1);
247
248 g_main_loop_run(mainloop);
249
250@@ -109,7 +109,7 @@
251 TEST_F(HelperHandshakeTest, HandshakeTimeout)
252 {
253 bool timeout_reached = false;
254- handshake_t * handshake = starting_handshake_start("fooapp");
255+ handshake_t * handshake = starting_handshake_start("fooapp", 1);
256
257 guint outertimeout = g_timeout_add_seconds(2, two_second_reached, &timeout_reached);
258

Subscribers

People subscribed via source and target branches