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
=== modified file 'helpers.c'
--- helpers.c 2016-08-16 18:13:06 +0000
+++ helpers.c 2016-09-02 14:11:47 +0000
@@ -503,7 +503,7 @@
503}503}
504504
505handshake_t *505handshake_t *
506starting_handshake_start (const gchar * app_id)506starting_handshake_start (const gchar * app_id, int timeout_s)
507{507{
508 GError * error = NULL;508 GError * error = NULL;
509 handshake_t * handshake = g_new0(handshake_t, 1);509 handshake_t * handshake = g_new0(handshake_t, 1);
@@ -541,7 +541,7 @@
541 &error);541 &error);
542542
543 /* Really, Unity? */543 /* Really, Unity? */
544 handshake->timeout = g_timeout_source_new_seconds(1);544 handshake->timeout = g_timeout_source_new_seconds(timeout_s);
545 g_source_set_callback(handshake->timeout, unity_too_slow_cb, handshake, NULL);545 g_source_set_callback(handshake->timeout, unity_too_slow_cb, handshake, NULL);
546 g_source_attach(handshake->timeout, context);546 g_source_attach(handshake->timeout, context);
547547
548548
=== modified file 'helpers.h'
--- helpers.h 2015-07-15 02:32:54 +0000
+++ helpers.h 2016-09-02 14:11:47 +0000
@@ -45,7 +45,8 @@
45void env_handle_finish (EnvHandle * handle);45void env_handle_finish (EnvHandle * handle);
4646
47typedef struct _handshake_t handshake_t;47typedef struct _handshake_t handshake_t;
48handshake_t * starting_handshake_start (const gchar * app_id);48handshake_t * starting_handshake_start (const gchar * app_id,
49 int timeout_s);
49void starting_handshake_wait (handshake_t * handshake);50void starting_handshake_wait (handshake_t * handshake);
5051
51GDBusConnection * cgroup_manager_connection (void);52GDBusConnection * cgroup_manager_connection (void);
5253
=== modified file 'libubuntu-app-launch/application-impl-click.cpp'
--- libubuntu-app-launch/application-impl-click.cpp 2016-08-17 15:24:23 +0000
+++ libubuntu-app-launch/application-impl-click.cpp 2016-09-02 14:11:47 +0000
@@ -75,7 +75,7 @@
7575
76std::list<AppID::AppName> manifestApps(const std::shared_ptr<JsonObject>& manifest)76std::list<AppID::AppName> manifestApps(const std::shared_ptr<JsonObject>& manifest)
77{77{
78 JsonObject *hooks = nullptr;78 JsonObject* hooks = nullptr;
79 if (!json_object_has_member(manifest.get(), "hooks") ||79 if (!json_object_has_member(manifest.get(), "hooks") ||
80 (hooks = json_object_get_object_member(manifest.get(), "hooks")) == nullptr)80 (hooks = json_object_get_object_member(manifest.get(), "hooks")) == nullptr)
81 {81 {
@@ -108,14 +108,14 @@
108 const std::string& app,108 const std::string& app,
109 const std::string& clickDir)109 const std::string& clickDir)
110{110{
111 JsonObject *hooks = nullptr;111 JsonObject* hooks = nullptr;
112 if (!json_object_has_member(manifest.get(), "hooks") ||112 if (!json_object_has_member(manifest.get(), "hooks") ||
113 (hooks = json_object_get_object_member(manifest.get(), "hooks")) == nullptr)113 (hooks = json_object_get_object_member(manifest.get(), "hooks")) == nullptr)
114 {114 {
115 throw std::runtime_error("Manifest for application '" + app + "' does not have a 'hooks' field");115 throw std::runtime_error("Manifest for application '" + app + "' does not have a 'hooks' field");
116 }116 }
117117
118 JsonObject *hooklist = nullptr;118 JsonObject* hooklist = nullptr;
119 if (!json_object_has_member(hooks, app.c_str()) ||119 if (!json_object_has_member(hooks, app.c_str()) ||
120 (hooklist = json_object_get_object_member(hooks, app.c_str())) == nullptr)120 (hooklist = json_object_get_object_member(hooks, app.c_str())) == nullptr)
121 {121 {
122122
=== modified file 'libubuntu-app-launch/click-exec.c'
--- libubuntu-app-launch/click-exec.c 2014-09-17 14:11:59 +0000
+++ libubuntu-app-launch/click-exec.c 2016-09-02 14:11:47 +0000
@@ -41,7 +41,7 @@
41*/41*/
4242
43gboolean43gboolean
44click_task_setup (GDBusConnection * bus, const gchar * app_id, EnvHandle * handle)44click_task_setup (GDBusConnection * bus, const gchar * app_id, EnvHandle * handle, int timeout_s)
45{45{
46 g_return_val_if_fail(bus != NULL, FALSE);46 g_return_val_if_fail(bus != NULL, FALSE);
47 g_return_val_if_fail(app_id != NULL, FALSE);47 g_return_val_if_fail(app_id != NULL, FALSE);
@@ -51,7 +51,7 @@
5151
52 GError * error = NULL;52 GError * error = NULL;
5353
54 handshake_t * handshake = starting_handshake_start(app_id);54 handshake_t * handshake = starting_handshake_start(app_id, timeout_s);
55 if (handshake == NULL) {55 if (handshake == NULL) {
56 g_warning("Unable to setup starting handshake");56 g_warning("Unable to setup starting handshake");
57 }57 }
5858
=== modified file 'libubuntu-app-launch/click-exec.h'
--- libubuntu-app-launch/click-exec.h 2014-08-22 20:59:55 +0000
+++ libubuntu-app-launch/click-exec.h 2016-09-02 14:11:47 +0000
@@ -20,6 +20,6 @@
20#include <glib.h>20#include <glib.h>
21#include "helpers.h"21#include "helpers.h"
2222
23gboolean click_task_setup (GDBusConnection * bus, const gchar * app_id, EnvHandle * envhandle);23gboolean click_task_setup (GDBusConnection * bus, const gchar * app_id, EnvHandle * envhandle, int timeout_s);
2424
2525
2626
=== modified file 'libubuntu-app-launch/desktop-exec.c'
--- libubuntu-app-launch/desktop-exec.c 2016-01-26 21:17:25 +0000
+++ libubuntu-app-launch/desktop-exec.c 2016-09-02 14:11:47 +0000
@@ -117,7 +117,7 @@
117}117}
118118
119gboolean119gboolean
120desktop_task_setup (GDBusConnection * bus, const gchar * app_id, EnvHandle * handle, gboolean is_libertine)120desktop_task_setup (GDBusConnection * bus, const gchar * app_id, EnvHandle * handle, gboolean is_libertine, int timeout_s)
121{121{
122 if (app_id == NULL) {122 if (app_id == NULL) {
123 g_error("No APP_ID environment variable defined");123 g_error("No APP_ID environment variable defined");
@@ -126,7 +126,7 @@
126126
127 ual_tracepoint(desktop_start, app_id);127 ual_tracepoint(desktop_start, app_id);
128128
129 handshake_t * handshake = starting_handshake_start(app_id);129 handshake_t * handshake = starting_handshake_start(app_id, timeout_s);
130 if (handshake == NULL) {130 if (handshake == NULL) {
131 g_warning("Unable to setup starting handshake");131 g_warning("Unable to setup starting handshake");
132 }132 }
133133
=== modified file 'libubuntu-app-launch/desktop-exec.h'
--- libubuntu-app-launch/desktop-exec.h 2015-07-15 02:12:04 +0000
+++ libubuntu-app-launch/desktop-exec.h 2016-09-02 14:11:47 +0000
@@ -22,5 +22,5 @@
22#include <glib.h>22#include <glib.h>
23#include "helpers.h"23#include "helpers.h"
2424
25gboolean desktop_task_setup (GDBusConnection * bus, const gchar * appid, EnvHandle * envhandle, gboolean is_libertine);25gboolean desktop_task_setup (GDBusConnection * bus, const gchar * appid, EnvHandle * envhandle, gboolean is_libertine, int timeout_s);
2626
2727
=== modified file 'libubuntu-app-launch/registry-impl.cpp'
--- libubuntu-app-launch/registry-impl.cpp 2016-08-17 15:24:23 +0000
+++ libubuntu-app-launch/registry-impl.cpp 2016-09-02 14:11:47 +0000
@@ -116,7 +116,7 @@
116116
117 auto retval = std::shared_ptr<JsonObject>(json_node_dup_object(node), json_object_unref);117 auto retval = std::shared_ptr<JsonObject>(json_node_dup_object(node), json_object_unref);
118118
119#if JSON_CHECK_VERSION(1,1,2)119#if JSON_CHECK_VERSION(1, 1, 2)
120 // Not available in json-glib 1.0, so must leak there.120 // Not available in json-glib 1.0, so must leak there.
121 json_node_unref(node);121 json_node_unref(node);
122#endif122#endif
@@ -520,5 +520,26 @@
520}520}
521#endif521#endif
522522
523/** App start watching, if we're registered for the signal we
524 can't wait on it. We are making this static right now because
525 we need it to go across the C and C++ APIs smoothly, and those
526 can end up with different registry objects. Long term, this
527 should become a member variable. */
528static bool watchingAppStarting_ = false;
529
530/** Variable to track if this program is watching app startup
531 so that we can know to not wait on the response to that. */
532void Registry::Impl::watchingAppStarting(bool rWatching)
533{
534 watchingAppStarting_ = rWatching;
535}
536
537/** Accessor for the internal variable to know whether an app
538 is watching for app startup */
539bool Registry::Impl::isWatchingAppStarting()
540{
541 return watchingAppStarting_;
542}
543
523} // namespace app_launch544} // namespace app_launch
524} // namespace ubuntu545} // namespace ubuntu
525546
=== modified file 'libubuntu-app-launch/registry-impl.h'
--- libubuntu-app-launch/registry-impl.h 2016-08-17 15:24:23 +0000
+++ libubuntu-app-launch/registry-impl.h 2016-09-02 14:11:47 +0000
@@ -70,6 +70,12 @@
70 std::list<std::string> upstartInstancesForJob(const std::string& job);70 std::list<std::string> upstartInstancesForJob(const std::string& job);
71 std::string upstartJobPath(const std::string& job);71 std::string upstartJobPath(const std::string& job);
7272
73 /* Signal Hints */
74 /* NOTE: Static because we don't have registry instances in the C
75 code right now. We want these to not be static in the future */
76 static void watchingAppStarting(bool rWatching);
77 static bool isWatchingAppStarting();
78
73private:79private:
74 Registry* _registry;80 Registry* _registry;
75#if 081#if 0
7682
=== modified file 'libubuntu-app-launch/ubuntu-app-launch.cpp'
--- libubuntu-app-launch/ubuntu-app-launch.cpp 2016-08-25 14:20:38 +0000
+++ libubuntu-app-launch/ubuntu-app-launch.cpp 2016-09-02 14:11:47 +0000
@@ -42,6 +42,7 @@
42#include "application.h"42#include "application.h"
43#include "appid.h"43#include "appid.h"
44#include "registry.h"44#include "registry.h"
45#include "registry-impl.h"
4546
46static void apps_for_job (GDBusConnection * con, const gchar * name, GArray * apps, gboolean truncate_legacy);47static void apps_for_job (GDBusConnection * con, const gchar * name, GArray * apps, gboolean truncate_legacy);
47static void free_helper (gpointer value);48static void free_helper (gpointer value);
@@ -299,11 +300,16 @@
299 g_variant_builder_add_value(&builder, g_variant_new_string("QT_LOAD_TESTABILITY=1"));300 g_variant_builder_add_value(&builder, g_variant_new_string("QT_LOAD_TESTABILITY=1"));
300 }301 }
301302
303 int timeout = 1;
304 if (ubuntu::app_launch::Registry::Impl::isWatchingAppStarting()) {
305 timeout = 0;
306 }
307
302 gboolean setup_complete = FALSE;308 gboolean setup_complete = FALSE;
303 if (click) {309 if (click) {
304 setup_complete = click_task_setup(con, appid, (EnvHandle*)&builder);310 setup_complete = click_task_setup(con, appid, (EnvHandle*)&builder, timeout);
305 } else {311 } else {
306 setup_complete = desktop_task_setup(con, appid, (EnvHandle*)&builder, libertine);312 setup_complete = desktop_task_setup(con, appid, (EnvHandle*)&builder, libertine, timeout);
307 }313 }
308314
309 if (setup_complete) {315 if (setup_complete) {
@@ -820,6 +826,7 @@
820gboolean826gboolean
821ubuntu_app_launch_observer_add_app_starting (UbuntuAppLaunchAppObserver observer, gpointer user_data)827ubuntu_app_launch_observer_add_app_starting (UbuntuAppLaunchAppObserver observer, gpointer user_data)
822{828{
829 ubuntu::app_launch::Registry::Impl::watchingAppStarting(true);
823 return add_session_generic(observer, user_data, "UnityStartingBroadcast", &starting_array, starting_signal_cb);830 return add_session_generic(observer, user_data, "UnityStartingBroadcast", &starting_array, starting_signal_cb);
824}831}
825832
@@ -1011,6 +1018,7 @@
1011gboolean1018gboolean
1012ubuntu_app_launch_observer_delete_app_starting (UbuntuAppLaunchAppObserver observer, gpointer user_data)1019ubuntu_app_launch_observer_delete_app_starting (UbuntuAppLaunchAppObserver observer, gpointer user_data)
1013{1020{
1021 ubuntu::app_launch::Registry::Impl::watchingAppStarting(false);
1014 return delete_app_generic(observer, user_data, &starting_array);1022 return delete_app_generic(observer, user_data, &starting_array);
1015}1023}
10161024
10171025
=== modified file 'tests/helper-handshake-test.cc'
--- tests/helper-handshake-test.cc 2015-12-18 17:21:20 +0000
+++ tests/helper-handshake-test.cc 2016-09-02 14:11:47 +0000
@@ -77,7 +77,7 @@
77 GDBusConnection * con = g_bus_get_sync(G_BUS_TYPE_SESSION, NULL, NULL);77 GDBusConnection * con = g_bus_get_sync(G_BUS_TYPE_SESSION, NULL, NULL);
78 guint filter = g_dbus_connection_add_filter(con, filter_func, this, NULL);78 guint filter = g_dbus_connection_add_filter(con, filter_func, this, NULL);
7979
80 handshake_t * handshake = starting_handshake_start("fooapp");80 handshake_t * handshake = starting_handshake_start("fooapp", 1);
8181
82 g_main_loop_run(mainloop);82 g_main_loop_run(mainloop);
8383
@@ -109,7 +109,7 @@
109TEST_F(HelperHandshakeTest, HandshakeTimeout)109TEST_F(HelperHandshakeTest, HandshakeTimeout)
110{110{
111 bool timeout_reached = false;111 bool timeout_reached = false;
112 handshake_t * handshake = starting_handshake_start("fooapp");112 handshake_t * handshake = starting_handshake_start("fooapp", 1);
113113
114 guint outertimeout = g_timeout_add_seconds(2, two_second_reached, &timeout_reached);114 guint outertimeout = g_timeout_add_seconds(2, two_second_reached, &timeout_reached);
115115

Subscribers

People subscribed via source and target branches