Merge lp://qastaging/~mterry/ubuntu-app-launch/warn-on-xapp into lp://qastaging/ubuntu-app-launch/15.10

Proposed by Michael Terry
Status: Superseded
Proposed branch: lp://qastaging/~mterry/ubuntu-app-launch/warn-on-xapp
Merge into: lp://qastaging/ubuntu-app-launch/15.10
Prerequisite: lp://qastaging/~mterry/ubuntu-app-launch/fix-ftbfs
Diff against target: 1334 lines (+757/-124)
16 files modified
CMakeLists.txt (+4/-0)
data/com.canonical.UbuntuAppLaunch.xml (+4/-0)
debian/changelog (+8/-0)
debian/libubuntu-app-launch2.symbols (+3/-0)
helpers.c (+130/-23)
helpers.h (+7/-2)
libubuntu-app-launch/click-exec.c (+0/-13)
libubuntu-app-launch/desktop-exec.c (+0/-13)
libubuntu-app-launch/ubuntu-app-launch-trace.tp (+18/-24)
libubuntu-app-launch/ubuntu-app-launch.c (+124/-26)
libubuntu-app-launch/ubuntu-app-launch.h (+41/-2)
tests/CMakeLists.txt (+1/-1)
tests/exec-util-test.cc (+2/-1)
tests/helper-handshake-test.cc (+263/-13)
tests/libual-test.cc (+151/-6)
tools/ubuntu-app-watch.c (+1/-0)
To merge this branch: bzr merge lp://qastaging/~mterry/ubuntu-app-launch/warn-on-xapp
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Needs Fixing
Indicator Applet Developers Pending
Review via email: mp+278497@code.qastaging.launchpad.net

This proposal has been superseded by a proposal from 2015-12-02.

Commit message

Allow Unity to prevent an app from starting and give it as long as it wants to figure that out.

Description of the change

Allow Unity to prevent an app from starting and give it as long as it wants to figure that out.

This branch gives Unity the option to prevent an app from launching by blocking on its answer before actually starting the app and then acting on that answer.

Related branches:
 https://code.launchpad.net/~mterry/unity-api/warn-on-xapp/+merge/277922
 https://code.launchpad.net/~mterry/qtmir/warn-on-xapp/+merge/279172
 https://code.launchpad.net/~mterry/unity8/warn-on-xapp/+merge/277915

== The existing design ==

The current code goes through pains to avoid any knowledge of Unity (despite using the name Unity in several signals). It doesn't connect to it on the bus. And it doesn't restrict who can answer its "I am starting an app" signal.

I'm not sure why it does this, but it does. So the way it communicates is via broadcast signals on the bus. Which gets answered by "someone" (presumably Unity). If there are multiple signal observers on the bus, we continue launching the app as soon as we hear the first response.

It also waits 1s for an answer; if no answer is received, it just launches the app anyway. The signal is more of a courtesy to Unity than a requirement. So if Unity isn't running, exits before it can respond, or is taking too long, UAL just continues.

== New design ==

The new paradigm is that Unity is an approver of which apps can launch. The proximate reason for this MP is that we want to be able to stop legacy apps from launching when not in Desktop mode. But I could imagine other scenarios (parental controls on some apps, or admin privileges needed for some apps, etc). [1]

I also wanted to allow Unity to take as long as it wanted to decide. Because maybe it is taking user input (authenticating as parent maybe) or in the case of a legacy app on the phone, waiting for user to either cancel the app launch or dock the phone (in which case we'd like to continue the app launch).

If Unity is not running, I've assumed we don't want the app to launch (which is a behavior change).

== Changes ==

- So we want to change the API for observers of the "starting" signal to allow for providing an "approved or not" answer. And we want to make it easy to answer in an async manner. Unfortunately, the current API has no state (no objects on which the API acts) to match up callbacks to their answers. So rather than redo the API, I've just added a new API call that app-starting-observers can use to answer any app-starting message: ubuntu_app_launch_observer_finish_app_starting(). It just broadcasts the answer on the bus. If that new method is not called, the app will never finish launching. The only current user of this API should be unity8, for which I have a related branch to use this new API.

- We want to know if Unity is even running (if not, we should reject the app). So I've kept the immediate signal response UnityStartingSignal, so that UAL knows someone is listening and handling the request. If it doesn't hear that signal within 5s, we reject the app. (I think it would be nicer to call com.canonical.Unity.RequestAppStart or similar, which would mean that only Unity could handle the request, would immediately tell us if Unity is running, and would give us a nice error instead of silence on the bus if something is wrong. But I didn't want to change the design of UAL that much, so I've kept the signal dance.)

- As noted above, I've bumped the timeout from 1s to 5s. This is because now we reject by default if the timeout happens, so it's more important that we give unity8 time. And secondly, we're adding an async API, so increasing the potential blocking time shouldn't be as much of a burden.

- We only support listening to the first responder we hear on the bus. This would be easy to extend to support listening to all responders we hear and waiting for all of them to weigh in. But I didn't need that feature yet, so I didn't write it.

- Since we now wait as long as Unity wants, we have to be sensitive to Unity quitting on us. So once we hear UnityStartingSignal, we also listen for NameOwnerChanges on the bus.

- Eventually, Unity gets back to us with a UnityStartingApproved signal that tells us to continue or not.

- Consolidating the handshake logic out of click-exec.c and desktop-exec.c has the side effect of fixing a bug where task_setup failed but we already emitted a UnityStartingBroadcast. And we had no way to cancel the handshake. So Unity would think an app was starting even though it wasn't. Now all the handshaking happens right in a row, without the ability to fail due to desktop parsing or similar.

- I've added an async version of ubuntu_app_launch_start_application so that callers don't have to block on this whole process (even without this branch, there was blocking occurring, waiting up to 5s on signals -- this method should definitively have existed before anyway).

== Footnotes ==

[1] ubuntu_app_launch_start_application is only for unconfined apps, right? Specifically, Touch apps or scopes can't start something via upstart directly themselves, bypassing the above scheme? I *believe* they always have to go the URLDispatcher route. Otherwise, this paradigm wouldn't really work for parental controls.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
216. By Michael Terry

Fix tests to still use old smaller timeout for convenience

Unmerged revisions

216. By Michael Terry

Fix tests to still use old smaller timeout for convenience

215. By Michael Terry

Bump timeout to 5s

214. By Michael Terry

Add async versions of start_application calls

213. By Michael Terry

fix tabs and revert some unnecessary wording changes

212. By Michael Terry

Wait for unity to get back to us

211. By Michael Terry

Allow server-side of handshake to be async by adding ubuntu_app_launch_observer_finish_app_starting

210. By Michael Terry

Add 'approved' bool parameter to UnityStartingSignal return signal, so that unity could prevent an app from opening

209. By Michael Terry

Fix ftbfs

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches