Merge lp:~ahayzen/content-hub/printing-trusted-export into lp:content-hub
- printing-trusted-export
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | Ken VanDine |
Approved revision: | 391 |
Merged at revision: | 328 |
Proposed branch: | lp:~ahayzen/content-hub/printing-trusted-export |
Merge into: | lp:content-hub |
Prerequisite: | lp:~artmello/content-hub/content-hub-clipboard |
Diff against target: |
195 lines (+73/-37) 3 files modified
src/com/ubuntu/content/common.h (+1/-0) src/com/ubuntu/content/detail/service.cpp (+62/-33) src/com/ubuntu/content/utils.cpp (+10/-4) |
To merge this branch: | bzr merge lp:~ahayzen/content-hub/printing-trusted-export |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Ken VanDine | Approve | ||
system-apps-ci-bot | continuous-integration | Needs Fixing | |
Andrew Hayzen (community) | Needs Fixing | ||
Review via email: mp+314141@code.launchpad.net |
Commit message
* Add special case for ubuntu-printing-app where it is a trusted export
Description of the change
* Add special case for ubuntu-printing-app where it is a trusted export
- 379. By Andrew Hayzen
-
* Remove whitespace
system-apps-ci-bot (system-apps-ci-bot) wrote : | # |
system-apps-ci-bot (system-apps-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:379
https:/
Executed test runs:
FAILURE: https:/
FAILURE: https:/
Click here to trigger a rebuild:
https:/
- 380. By Andrew Hayzen
-
* Add extra debugging
Andrew Hayzen (ahayzen) wrote : | # |
Added some debugging to figure out an issue, don't top approve until they can been removed.
system-apps-ci-bot (system-apps-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:380
https:/
Executed test runs:
FAILURE: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
FAILURE: https:/
FAILURE: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
FAILURE: https:/
FAILURE: https:/
Click here to trigger a rebuild:
https:/
- 381. By Andrew Hayzen
-
* Change to store source in activeSessions not destination
system-apps-ci-bot (system-apps-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:381
https:/
Executed test runs:
FAILURE: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
FAILURE: https:/
FAILURE: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
FAILURE: https:/
Click here to trigger a rebuild:
https:/
- 382. By Andrew Hayzen
-
* Add more tracing and skip storing handler so multiple instances works
system-apps-ci-bot (system-apps-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:382
https:/
Executed test runs:
FAILURE: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
FAILURE: https:/
FAILURE: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
FAILURE: https:/
FAILURE: https:/
Click here to trigger a rebuild:
https:/
- 383. By Andrew Hayzen
-
* Deconstruct r when using printing app
system-apps-ci-bot (system-apps-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:383
https:/
Executed test runs:
FAILURE: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
FAILURE: https:/
FAILURE: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
FAILURE: https:/
FAILURE: https:/
Click here to trigger a rebuild:
https:/
- 384. By Andrew Hayzen
-
* Pull of upstream
system-apps-ci-bot (system-apps-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:384
https:/
Executed test runs:
FAILURE: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
FAILURE: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
FAILURE: https:/
SUCCESS: https:/
deb: https:/
FAILURE: https:/
FAILURE: https:/
Click here to trigger a rebuild:
https:/
- 385. By Andrew Hayzen
-
* Merge with upstream
system-apps-ci-bot (system-apps-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:385
https:/
Executed test runs:
FAILURE: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
FAILURE: https:/
FAILURE: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
FAILURE: https:/
Click here to trigger a rebuild:
https:/
- 386. By Andrew Hayzen
-
* Remove handler skip
system-apps-ci-bot (system-apps-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:386
https:/
Executed test runs:
FAILURE: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
FAILURE: https:/
FAILURE: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
FAILURE: https:/
FAILURE: https:/
Click here to trigger a rebuild:
https:/
- 387. By Andrew Hayzen
-
* Remove unneeded tracing
system-apps-ci-bot (system-apps-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:387
https:/
Executed test runs:
FAILURE: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
FAILURE: https:/
FAILURE: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
FAILURE: https:/
FAILURE: https:/
Click here to trigger a rebuild:
https:/
- 388. By Andrew Hayzen
-
* Remove code which prevents multiple instances of trust sessions as that is now allowed in Mir 0.26
system-apps-ci-bot (system-apps-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:388
https:/
Executed test runs:
FAILURE: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
FAILURE: https:/
FAILURE: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
FAILURE: https:/
FAILURE: https:/
Click here to trigger a rebuild:
https:/
- 389. By Andrew Hayzen
- 390. By Andrew Hayzen
- 391. By Andrew Hayzen
-
* Merge of upstream and resolve conflicts with new UAL parts
Andrew Hayzen (ahayzen) wrote : | # |
This works with trusted sessions, however I've seen it cancel a second instance when closing the first - this needs investigating.
Andrew Hayzen (ahayzen) wrote : | # |
After further testing I'm able to open two trust sessions and close one, the previous issue seems to be a glitch that was caused by bug 1659310 :-)
Ken VanDine (ken-vandine) wrote : | # |
Looks good and works well.
- 392. By Andrew Hayzen
-
* Add try catch around ual::Applicatio
n::create( app_id, reg);
Ken VanDine (ken-vandine) wrote : | # |
See the one inline comment
- 393. By Andrew Hayzen
-
* Put ual::AppID::find inside try..catch as well
Andrew Hayzen (ahayzen) wrote : | # |
Fixed, please re-review :-)
Ken VanDine (ken-vandine) wrote : | # |
Looks great, thanks!
Preview Diff
1 | === modified file 'src/com/ubuntu/content/common.h' |
2 | --- src/com/ubuntu/content/common.h 2017-02-28 14:50:28 +0000 |
3 | +++ src/com/ubuntu/content/common.h 2017-02-28 14:50:29 +0000 |
4 | @@ -28,5 +28,6 @@ |
5 | const QLatin1String HANDLER_BASE_PATH = QLatin1String("/com/ubuntu/content/handler"); |
6 | const QString CLIPBOARD_APP_ID = QLatin1String("content-hub-clipboard"); |
7 | const QString PEER_PICKER_APP_ID = QLatin1String("content-hub-peer-picker"); |
8 | +const QString PRINTING_APP_ID = QLatin1String("ubuntu-printing-app"); |
9 | |
10 | #endif // COMMON_H |
11 | |
12 | === modified file 'src/com/ubuntu/content/detail/service.cpp' |
13 | --- src/com/ubuntu/content/detail/service.cpp 2017-02-28 14:50:28 +0000 |
14 | +++ src/com/ubuntu/content/detail/service.cpp 2017-02-28 14:50:29 +0000 |
15 | @@ -365,7 +365,7 @@ |
16 | |
17 | QDBusObjectPath cucd::Service::CreateExportToPeer(const QString& peer_id, const QString& app_id, const QString& type_id) |
18 | { |
19 | - TRACE() << Q_FUNC_INFO; |
20 | + TRACE() << Q_FUNC_INFO << "APP_ID:" << app_id << "SERVICE:" << this->message().service(); |
21 | QString src_id = app_id; |
22 | if (src_id.isEmpty()) |
23 | { |
24 | @@ -590,7 +590,12 @@ |
25 | |
26 | auto transfer = new cucd::Transfer(import_counter, src_id, dest_id, dir, type_id, this); |
27 | if (dir == cuc::Transfer::Import && qgetenv("CONTENT_HUB_TESTING").isNull() && clientPid > 0) { |
28 | + TRACE() << Q_FUNC_INFO << "Making setupPromptSession:" << dest_id << clientPid; |
29 | setupPromptSession(dest_id, clientPid); |
30 | + } else if (dir == cuc::Transfer::Export && dest_id == PRINTING_APP_ID && qgetenv("CONTENT_HUB_TESTING").isNull()) { |
31 | + uint clientPid = d->connection.interface()->servicePid(this->message().service()); |
32 | + TRACE() << Q_FUNC_INFO << "Making setupPromptSession:" << src_id << clientPid; |
33 | + setupPromptSession(src_id, clientPid); |
34 | } |
35 | |
36 | new TransferAdaptor(transfer); |
37 | @@ -637,34 +642,10 @@ |
38 | if (!qgetenv("CONTENT_HUB_TESTING").isNull()) |
39 | return; |
40 | |
41 | - if (d->active_sessions.keys().contains(app_id)) |
42 | + if (d->active_sessions.keys().contains(app_id)) { |
43 | + TRACE() << Q_FUNC_INFO << "Skipping as dest_id is already an active session"; |
44 | return; |
45 | - |
46 | - /* FIXME: Until bug #1647409 is fixed, we need to release any existing |
47 | - * prompt sessions before starting a new one |
48 | - */ |
49 | - Q_FOREACH(QString key, d->active_sessions.keys()) { |
50 | - PromptSessionP pSession = d->active_sessions.value(key); |
51 | - if (pSession.data()) { |
52 | - TRACE() << "Removing session for" << key; |
53 | - d->active_sessions.remove(key); |
54 | - pSession->deleteLater(); |
55 | - |
56 | - /* When releasing the prompt session, we need to abort |
57 | - * transfers in process |
58 | - */ |
59 | - Q_FOREACH (cucd::Transfer *t, d->active_transfers) { |
60 | - if (t->destination() == key) { |
61 | - TRACE() << Q_FUNC_INFO << "Found active transfer for session:" << key; |
62 | - if (should_cancel(t->State())) { |
63 | - TRACE() << Q_FUNC_INFO << "Aborting active transfer:" << t->Id(); |
64 | - t->Abort(); |
65 | - } |
66 | - } |
67 | - } |
68 | - } |
69 | } |
70 | - /* End hack to work around bug #1647409 */ |
71 | |
72 | PromptSessionP session = MirHelper::instance()->createPromptSession(clientPid); |
73 | if (!session) return; |
74 | @@ -870,7 +851,8 @@ |
75 | if (state == cuc::Transfer::charged) |
76 | { |
77 | TRACE() << Q_FUNC_INFO << "Charged"; |
78 | - if (d->app_manager->is_application_started(transfer->destination().toStdString())) |
79 | + // For printing we always generate a new app |
80 | + if (d->app_manager->is_application_started(transfer->destination().toStdString()) && transfer->destination() != PRINTING_APP_ID) |
81 | transfer->SetSourceStartedByContentHub(false); |
82 | else |
83 | transfer->SetSourceStartedByContentHub(true); |
84 | @@ -893,10 +875,44 @@ |
85 | } |
86 | |
87 | if (transfer->ShouldBeStartedByContentHub()) { |
88 | - auto instance = d->app_manager->invoke_application(transfer->destination().toStdString(), uris); |
89 | - if (instance) { |
90 | - transfer->SetDestinationInstance(instance); |
91 | + // Special case for printing |
92 | + if (transfer->destination() == PRINTING_APP_ID) { |
93 | + TRACE() << Q_FUNC_INFO << "Using ubuntu-printing-app special case"; |
94 | + |
95 | + if (qgetenv("CONTENT_HUB_TESTING").isNull()) { |
96 | + if (!d->active_sessions.keys().contains(transfer->source()) |
97 | + && transfer->WasSourceStartedByContentHub()) { |
98 | + if (!QDBusConnection::sender().baseService().isEmpty()) { |
99 | + uint clientPid = d->connection.interface()->servicePid(this->message().service()); |
100 | + setupPromptSession(transfer->source(), clientPid); |
101 | + } |
102 | + } |
103 | + } |
104 | + |
105 | + if (d->active_sessions.keys().contains(transfer->source()) |
106 | + && transfer->WasSourceStartedByContentHub()) { |
107 | + TRACE() << Q_FUNC_INFO << "Invoking application with session"; |
108 | + PromptSessionP session = d->active_sessions.value(transfer->source()); |
109 | + gchar ** uris = NULL; |
110 | + auto instance = d->app_manager->invoke_application_with_session(transfer->destination().toStdString(), session, uris); |
111 | + if (instance) { |
112 | + transfer->SetHelperInstance(instance); |
113 | + } |
114 | + } else { |
115 | + TRACE() << Q_FUNC_INFO << "Invoking application"; |
116 | + auto instance = d->app_manager->invoke_application(transfer->destination().toStdString(), uris); |
117 | + if (instance) { |
118 | + transfer->SetDestinationInstance(instance); |
119 | + } |
120 | + } |
121 | + } else { |
122 | + TRACE() << Q_FUNC_INFO << "Invoking application"; |
123 | + auto instance = d->app_manager->invoke_application(transfer->destination().toStdString(), uris); |
124 | + if (instance) { |
125 | + transfer->SetDestinationInstance(instance); |
126 | + } |
127 | } |
128 | + |
129 | } |
130 | |
131 | Q_FOREACH (RegHandler *r, d->handlers) |
132 | @@ -990,6 +1006,8 @@ |
133 | |
134 | if (!exists) |
135 | { |
136 | + TRACE() << Q_FUNC_INFO << "ImportExport Handler does not exist, creating one"; |
137 | + |
138 | r = new RegHandler{peer_id, |
139 | this->message().service(), |
140 | new cuc::dbus::Handler( |
141 | @@ -997,8 +1015,14 @@ |
142 | handler.path(), |
143 | QDBusConnection::sessionBus(), |
144 | 0)}; |
145 | - d->handlers.insert(r); |
146 | - m_watcher->addWatchedService(r->service); |
147 | + |
148 | + // Skip adding to handlers for ubuntu-printing-app so we can have multiple instances |
149 | + if (peer_id != PRINTING_APP_ID) { |
150 | + d->handlers.insert(r); |
151 | + m_watcher->addWatchedService(r->service); |
152 | + } |
153 | + } else { |
154 | + TRACE() << Q_FUNC_INFO << "ImportExport Handler exists, skipping"; |
155 | } |
156 | |
157 | TRACE() << Q_FUNC_INFO << r->id; |
158 | @@ -1041,6 +1065,11 @@ |
159 | } |
160 | } |
161 | } |
162 | + |
163 | + // Deconstruct r as we haven't added to store when printing |
164 | + if (peer_id == PRINTING_APP_ID) { |
165 | + delete r; |
166 | + } |
167 | } |
168 | |
169 | void cucd::Service::HandlerActive(const QString& peer_id) |
170 | |
171 | === modified file 'src/com/ubuntu/content/utils.cpp' |
172 | --- src/com/ubuntu/content/utils.cpp 2017-01-25 19:57:28 +0000 |
173 | +++ src/com/ubuntu/content/utils.cpp 2017-02-28 14:50:29 +0000 |
174 | @@ -228,11 +228,17 @@ |
175 | return nullptr; |
176 | |
177 | std::shared_ptr<ual::Registry> reg = ual::Registry::getDefault(); |
178 | - auto app_id = ual::AppID::find(id.toStdString()); |
179 | - if (app_id.empty()) |
180 | + |
181 | + try { |
182 | + auto app_id = ual::AppID::find(id.toStdString()); |
183 | + if (app_id.empty()) |
184 | + return nullptr; |
185 | + auto app = ual::Application::create(app_id, reg); |
186 | + return app; |
187 | + } catch (std::runtime_error &e) { |
188 | + qWarning() << Q_FUNC_INFO << "Unable to create application:" << id; |
189 | return nullptr; |
190 | - auto app = ual::Application::create(app_id, reg); |
191 | - return app; |
192 | + } |
193 | } |
194 | |
195 | QMap<QString, QString> info_for_app_id(QString id) |
FAILED: Continuous integration, rev:378 /jenkins. canonical. com/system- apps/job/ lp-content- hub-ci/ 160/ /jenkins. canonical. com/system- apps/job/ build/2232/ console /jenkins. canonical. com/system- apps/job/ build-0- fetch/2235 /jenkins. canonical. com/system- apps/job/ build-2- binpkg/ arch=amd64, release= vivid+overlay/ 2062 /jenkins. canonical. com/system- apps/job/ build-2- binpkg/ arch=amd64, release= vivid+overlay/ 2062/artifact/ output/ *zip*/output. zip /jenkins. canonical. com/system- apps/job/ build-2- binpkg/ arch=amd64, release= xenial+ overlay/ 2062/console /jenkins. canonical. com/system- apps/job/ build-2- binpkg/ arch=amd64, release= zesty/2062/ console /jenkins. canonical. com/system- apps/job/ build-2- binpkg/ arch=armhf, release= vivid+overlay/ 2062 /jenkins. canonical. com/system- apps/job/ build-2- binpkg/ arch=armhf, release= vivid+overlay/ 2062/artifact/ output/ *zip*/output. zip /jenkins. canonical. com/system- apps/job/ build-2- binpkg/ arch=armhf, release= xenial+ overlay/ 2062 /jenkins. canonical. com/system- apps/job/ build-2- binpkg/ arch=armhf, release= xenial+ overlay/ 2062/artifact/ output/ *zip*/output. zip /jenkins. canonical. com/system- apps/job/ build-2- binpkg/ arch=armhf, release= zesty/2062 /jenkins. canonical. com/system- apps/job/ build-2- binpkg/ arch=armhf, release= zesty/2062/ artifact/ output/ *zip*/output. zip /jenkins. canonical. com/system- apps/job/ build-2- binpkg/ arch=i386, release= vivid+overlay/ 2062 /jenkins. canonical. com/system- apps/job/ build-2- binpkg/ arch=i386, release= vivid+overlay/ 2062/artifact/ output/ *zip*/output. zip /jenkins. canonical. com/system- apps/job/ build-2- binpkg/ arch=i386, release= xenial+ overlay/ 2062/console /jenkins. canonical. com/system- apps/job/ build-2- binpkg/ arch=i386, release= zesty/2062 /jenkins. canonical. com/system- apps/job/ build-2- binpkg/ arch=i386, release= zesty/2062/ artifact/ output/ *zip*/output. zip
https:/
Executed test runs:
FAILURE: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
FAILURE: https:/
FAILURE: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
FAILURE: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild: /jenkins. canonical. com/system- apps/job/ lp-content- hub-ci/ 160/rebuild
https:/