unity-scopes-api

Merge lp:~marcustomlinson/unity-scopes-api/fix_ui_policy into lp:unity-scopes-api/devel

  1. fix_ui_policy
  2. Merge into devel
Proposed by Marcus Tomlinson
Status: Merged
Approved by: Paweł Stołowski
Approved revision: 264
Merged at revision: 501
Proposed branch: lp:~marcustomlinson/unity-scopes-api/fix_ui_policy
Merge into: lp:unity-scopes-api/devel
Diff against target: 803 lines (+393/-135)
7 files modified
CMakeLists.txt (+1/-1)
RELEASE_NOTES.md (+5/-0)
debian/changelog (+7/-0)
include/unity/scopes/OnlineAccountClient.h (+8/-9)
include/unity/scopes/internal/OnlineAccountClientImpl.h (+23/-13)
src/scopes/internal/OnlineAccountClientImpl.cpp (+347/-112)
test/gtest/scopes/OnlineAccountClient/OnlineAccountClient_test.cpp (+2/-0)
To merge this branch: bzr merge lp:~marcustomlinson/unity-scopes-api/fix_ui_policy
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Paweł Stołowski (community) Approve
Review via email: mp+237572@code.launchpad.net

This proposal supersedes a proposal from 2014-10-06.

Commit message

1. Depending on whether OnlineAccountClient is used by the shell or by a scope, set the SIGNON_SESSION_DATA_UI_POLICY accordingly.

2. Inform scopes of successful user authentications via a pub/sub network.

3. Removed inc_logins() and dec_logins() as only one session can be active per service (now using info->session != nullptr).

4. Added better thread and exception safety.

5. Run the internal event loop within its own context to avoid clashing with external main loops.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal

PASSED: Continuous integration, rev:263
http://jenkins.qa.ubuntu.com/job/unity-scopes-api-ci/473/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-utopic-amd64-ci/100
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-utopic-armhf-ci/100
        deb: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-utopic-armhf-ci/100/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-utopic-i386-ci/100

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-scopes-api-ci/473/rebuild

review: Approve (continuous-integration)
Revision history for this message
Paweł Stołowski (stolowski) wrote : Posted in a previous version of this proposal

+1

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:264
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~marcustomlinson/unity-scopes-api/fix_ui_policy/+merge/237572/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-ci/1203/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-utopic-amd64-ci/761
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-utopic-armhf-ci/756
        deb: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-utopic-armhf-ci/756/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-utopic-i386-ci/755

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-scopes-api-devel-ci/1203/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Marcus Tomlinson (marcustomlinson) wrote :

Set a commit message and hit rebuild.

Revision history for this message
Paweł Stołowski (stolowski) wrote :

+1

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:264
http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-ci/1206/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-utopic-amd64-ci/764
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-utopic-armhf-ci/759
        deb: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-utopic-armhf-ci/759/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-utopic-i386-ci/758

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-scopes-api-devel-ci/1206/rebuild

review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'CMakeLists.txt'
2--- CMakeLists.txt 2014-09-11 08:35:01 +0000
3+++ CMakeLists.txt 2014-10-08 11:00:22 +0000
4@@ -195,7 +195,7 @@
5 # API version
6 set(UNITY_SCOPES_MAJOR 0)
7 set(UNITY_SCOPES_MINOR 6)
8-set(UNITY_SCOPES_MICRO 6)
9+set(UNITY_SCOPES_MICRO 7)
10 set(UNITY_SCOPES_SOVERSION 3)
11
12 # Version for testing, with all symbols visible
13
14=== modified file 'RELEASE_NOTES.md'
15--- RELEASE_NOTES.md 2014-09-11 08:42:38 +0000
16+++ RELEASE_NOTES.md 2014-10-08 11:00:22 +0000
17@@ -1,6 +1,11 @@
18 Release notes
19 =============
20
21+Changes in version 0.6.7
22+========================
23+ - Added new RunInExternalUiMainLoop option to OnlineAccountClient::MainLoopSelect.
24+ - OnlineAccountClient signon UI policy determined by main loop used (shell: show / scope: hide).
25+
26 Changes in version 0.6.6
27 ========================
28 - Added support for online accounts (via new OnlineAccountClient class).
29
30=== modified file 'debian/changelog'
31--- debian/changelog 2014-10-06 12:03:20 +0000
32+++ debian/changelog 2014-10-08 11:00:22 +0000
33@@ -1,3 +1,10 @@
34+unity-scopes-api (0.6.7-0ubuntu1) UNRELEASED; urgency=medium
35+
36+ * Added new RunInExternalUiMainLoop option to OnlineAccountClient::MainLoopSelect.
37+ * OnlineAccountClient signon UI policy determined by main loop used (shell: show / scope: hide).
38+
39+ -- Marcus Tomlinson <marcus.tomlinson@canonical.com> Wed, 01 Oct 2014 15:46:30 +0200
40+
41 unity-scopes-api (0.6.6+14.10.20141006-0ubuntu1) utopic; urgency=medium
42
43 [ Pawel Stolowski ]
44
45=== modified file 'include/unity/scopes/OnlineAccountClient.h'
46--- include/unity/scopes/OnlineAccountClient.h 2014-09-15 05:10:17 +0000
47+++ include/unity/scopes/OnlineAccountClient.h 2014-10-08 11:00:22 +0000
48@@ -55,7 +55,7 @@
49 */
50 struct ServiceStatus
51 {
52- uint account_id; ///< A unique ID of the online account parenting this service.
53+ int account_id; ///< A unique ID of the online account parenting this service.
54 bool service_enabled; ///< True if this service is enabled.
55 bool service_authenticated; ///< True if this service is authenticated.
56 std::string client_id; ///< "ConsumerKey" / "ClientId" OAuth (1 / 2) parameter.
57@@ -68,16 +68,15 @@
58 /**
59 \brief Indicates whether an external main loop already exists, or one should be created internally.
60
61- A running main loop is essential in order to receive service update callbacks. When in doubt, set
62- RunInExternalMainLoop and just use refresh_service_statuses() and get_service_statuses() to obtain
63- service statuses.
64+ A running main loop is essential in order to receive service updates from the online accounts
65+ backend. When in doubt, set to CreateInternalMainLoop.
66 */
67 enum MainLoopSelect
68 {
69- RunInExternalMainLoop, ///< An external main loop already exists or the service update
70- /// callback is not required.
71- CreateInternalMainLoop ///< An external main loop does not exist and the service update
72- /// callback is required.
73+ RunInExternalMainLoop, ///< An external main loop already exists and is running.
74+ CreateInternalMainLoop, ///< An external main loop does not exist.
75+ RunInExternalUiMainLoop, ///< An external UI main loop exists and is running (Intended for
76+ /// shell use only. A scope should not be running a UI main loop).
77 };
78
79 /**
80@@ -91,7 +90,7 @@
81 OnlineAccountClient(std::string const& service_name,
82 std::string const& service_type,
83 std::string const& provider_name,
84- MainLoopSelect main_loop_select = RunInExternalMainLoop);
85+ MainLoopSelect main_loop_select = CreateInternalMainLoop);
86
87 /// @cond
88 ~OnlineAccountClient();
89
90=== modified file 'include/unity/scopes/internal/OnlineAccountClientImpl.h'
91--- include/unity/scopes/internal/OnlineAccountClientImpl.h 2014-09-11 11:29:29 +0000
92+++ include/unity/scopes/internal/OnlineAccountClientImpl.h 2014-10-08 11:00:22 +0000
93@@ -19,6 +19,7 @@
94 #ifndef UNITY_SCOPES_INTERNAL_ONLINEACCOUNTCLIENTIMPL_H
95 #define UNITY_SCOPES_INTERNAL_ONLINEACCOUNTCLIENTIMPL_H
96
97+#include <unity/scopes/internal/MiddlewareBase.h>
98 #include <unity/scopes/OnlineAccountClient.h>
99
100 #include <libaccounts-glib/accounts-glib.h>
101@@ -78,42 +79,51 @@
102 OnlineAccountClient::PostLoginAction login_failed_action);
103
104 // Methods used only by impl
105- void flush_pending_sessions();
106+ void construct();
107+ void tear_down();
108+
109+ void flush_pending_session(AgAccountId const& account_id, std::unique_lock<std::mutex>& lock);
110
111 void main_loop_state_notify(bool is_running);
112
113+ std::shared_ptr<AgManager> manager();
114 std::string service_name();
115- void callback(OnlineAccountClient::ServiceStatus const& service_status);
116+ OnlineAccountClient::MainLoopSelect main_loop_select();
117+
118+ void callback(AccountInfo const* info, std::string const& error = "");
119
120 bool has_account(AgAccountId const& account_id);
121 void add_account(AgAccountId const& account_id, std::shared_ptr<AccountInfo> account_info);
122 void remove_account(AgAccountId const& account_id);
123
124- bool inc_logins();
125- void dec_logins();
126-
127 private:
128- std::string service_name_;
129- std::string service_type_;
130- std::string provider_name_;
131+ std::string const service_name_;
132+ std::string const service_type_;
133+ std::string const provider_name_;
134+ OnlineAccountClient::MainLoopSelect const main_loop_select_;
135+
136+ std::mutex callback_mutex_;
137 OnlineAccountClient::ServiceUpdateCallback callback_;
138- bool use_external_main_loop_;
139
140- std::thread main_loop_thread_;
141- std::mutex callback_mutex_;
142 std::mutex mutex_;
143 std::condition_variable cond_;
144+ std::thread main_loop_thread_;
145 bool main_loop_is_running_;
146- bool client_stopping_;
147- int logins_busy_;
148+ std::exception_ptr thread_exception_;
149 gulong account_enabled_signal_id_;
150 gulong account_deleted_signal_id_;
151
152 std::shared_ptr<GMainLoop> main_loop_;
153+ std::shared_ptr<GMainContext> main_loop_context_;
154 std::shared_ptr<AgManager> manager_;
155 std::map<AgAccountId, std::shared_ptr<AccountInfo>> accounts_;
156
157+ MiddlewareBase::SPtr mw_;
158+ MWPublisher::UPtr auth_publisher_;
159+ MWSubscriber::UPtr auth_subscriber_;
160+
161 void main_loop_thread();
162+ void auth_callback(std::string const& details_json);
163 };
164
165 } // namespace internal
166
167=== modified file 'src/scopes/internal/OnlineAccountClientImpl.cpp'
168--- src/scopes/internal/OnlineAccountClientImpl.cpp 2014-09-15 07:06:51 +0000
169+++ src/scopes/internal/OnlineAccountClientImpl.cpp 2014-10-08 11:00:22 +0000
170@@ -17,7 +17,12 @@
171 */
172
173 #include <unity/scopes/internal/OnlineAccountClientImpl.h>
174-#include <unity/util/ResourcePtr.h>
175+
176+#include <unity/scopes/internal/JsonCppNode.h>
177+#include <unity/scopes/internal/MiddlewareFactory.h>
178+#include <unity/scopes/internal/RuntimeConfig.h>
179+#include <unity/scopes/internal/RuntimeImpl.h>
180+#include <unity/UnityExceptions.h>
181
182 #include <iostream>
183
184@@ -88,7 +93,41 @@
185 return service_status;
186 }
187
188-static void clear_session_info(AccountInfo* info)
189+static std::string details_to_json(OnlineAccountClient::ServiceStatus const& details)
190+{
191+ VariantMap vm;
192+ vm["account_id"] = details.account_id;
193+ vm["service_enabled"] = details.service_enabled;
194+ vm["service_authenticated"] = details.service_authenticated;
195+ vm["client_id"] = details.client_id;
196+ vm["client_secret"] = details.client_secret;
197+ vm["access_token"] = details.access_token;
198+ vm["token_secret"] = details.token_secret;
199+ vm["error"] = details.error;
200+
201+ Variant var(vm);
202+ JsonCppNode node(var);
203+ return node.to_json_string();
204+}
205+
206+static OnlineAccountClient::ServiceStatus json_to_details(std::string const& json)
207+{
208+ OnlineAccountClient::ServiceStatus details;
209+ JsonCppNode node(json);
210+
211+ details.account_id = node.get_node("account_id")->as_int();
212+ details.service_enabled = node.get_node("service_enabled")->as_bool();
213+ details.service_authenticated = node.get_node("service_authenticated")->as_bool();
214+ details.client_id = node.get_node("client_id")->as_string();
215+ details.client_secret = node.get_node("client_secret")->as_string();
216+ details.access_token = node.get_node("access_token")->as_string();
217+ details.token_secret = node.get_node("token_secret")->as_string();
218+ details.error = node.get_node("error")->as_string();
219+
220+ return details;
221+}
222+
223+static void clear_session(AccountInfo* info)
224 {
225 if (info->session)
226 {
227@@ -96,8 +135,6 @@
228 signon_auth_session_cancel(info->session.get());
229 }
230 info->session = nullptr;
231- info->auth_params = nullptr;
232- info->session_data = nullptr;
233 }
234
235 static gboolean main_loop_is_running_cb(void* user_data)
236@@ -107,6 +144,13 @@
237 return G_SOURCE_REMOVE;
238 }
239
240+static gboolean main_loop_is_stopping_cb(void* user_data)
241+{
242+ OnlineAccountClientImpl* account_client = reinterpret_cast<OnlineAccountClientImpl*>(user_data);
243+ account_client->main_loop_state_notify(false);
244+ return G_SOURCE_REMOVE;
245+}
246+
247 static gboolean wake_up_event_loop_cb(void* user_data)
248 {
249 GMainLoop* event_loop = reinterpret_cast<GMainLoop*>(user_data);
250@@ -116,58 +160,75 @@
251
252 static void service_login_cb(GObject* source, GAsyncResult* result, void* user_data)
253 {
254- GError* error = nullptr;
255- util::ResourcePtr<GError*, decltype(&g_error_free)> error_cleanup(error, free_error);
256-
257 SignonAuthSession* session = reinterpret_cast<SignonAuthSession*>(source);
258 AccountInfo* info = reinterpret_cast<AccountInfo*>(user_data);
259
260 // Get session data then send a notification with the login result
261+ GError* error = nullptr;
262 info->session_data.reset(signon_auth_session_process_finish(session, result, &error), free_variant);
263- info->account_client->callback(info_to_details(info, error ? error->message : ""));
264- info->account_client->dec_logins();
265+ std::shared_ptr<GError> error_cleanup(error, free_error);
266+
267+ info->account_client->callback(info, error ? error->message : "");
268+
269+ // Clear session info
270+ clear_session(info);
271 }
272
273 static void service_update_cb(AgAccountService* account_service, gboolean enabled, AccountInfo* info)
274 {
275- GError* error = nullptr;
276- util::ResourcePtr<GError*, decltype(&g_error_free)> error_cleanup(error, free_error);
277-
278 // Service state has updated, clear the old session data
279 info->service_enabled = enabled;
280- clear_session_info(info);
281+ clear_session(info);
282
283 if (enabled)
284 {
285 // Get authorization data then create a new authorization session
286 std::shared_ptr<AgAuthData> auth_data(ag_account_service_get_auth_data(account_service), ag_auth_data_unref);
287+
288+ GError* error = nullptr;
289 info->session.reset(signon_auth_session_new(
290 ag_auth_data_get_credentials_id(auth_data.get()), ag_auth_data_get_method(auth_data.get()), &error), g_object_unref);
291+ std::shared_ptr<GError> error_cleanup(error, free_error);
292+
293 if (error)
294 {
295 // Send notification that the authorization session failed
296- info->account_client->callback(info_to_details(info, error->message));
297+ info->account_client->callback(info, error->message);
298 return;
299 }
300
301 // Get authorization parameters then attempt to signon
302+ GVariantBuilder builder;
303+ g_variant_builder_init(&builder, G_VARIANT_TYPE_VARDICT);
304+
305+ if (info->account_client->main_loop_select() == OnlineAccountClient::RunInExternalUiMainLoop)
306+ {
307+ g_variant_builder_add(&builder, "{sv}",
308+ SIGNON_SESSION_DATA_UI_POLICY,
309+ g_variant_new_int32(SIGNON_POLICY_DEFAULT));
310+ }
311+ else
312+ {
313+ g_variant_builder_add(&builder, "{sv}",
314+ SIGNON_SESSION_DATA_UI_POLICY,
315+ g_variant_new_int32(SIGNON_POLICY_NO_USER_INTERACTION));
316+ }
317+
318 info->auth_params.reset(
319- g_variant_ref_sink(ag_auth_data_get_login_parameters(auth_data.get(), nullptr)), free_variant);
320+ g_variant_ref_sink(ag_auth_data_get_login_parameters(auth_data.get(), g_variant_builder_end(&builder))), free_variant);
321
322- if (info->account_client->inc_logins())
323- {
324- signon_auth_session_process_async(info->session.get(),
325- info->auth_params.get(),
326- ag_auth_data_get_mechanism(auth_data.get()),
327- nullptr,
328- service_login_cb,
329- info);
330- }
331+ // Start signon process
332+ signon_auth_session_process_async(info->session.get(),
333+ info->auth_params.get(),
334+ ag_auth_data_get_mechanism(auth_data.get()),
335+ nullptr,
336+ service_login_cb,
337+ info);
338 }
339 else
340 {
341 // Send notification that account has been disabled
342- info->account_client->callback(info_to_details(info));
343+ info->account_client->callback(info);
344 }
345 }
346
347@@ -193,7 +254,7 @@
348 if (account_client->service_name() == ag_service_get_name(service))
349 {
350 std::shared_ptr<AgAccountService> account_service(ag_account_service_new(account.get(), service), g_object_unref);
351- std::shared_ptr<AccountInfo> info(new AccountInfo, [](AccountInfo* info){ info->account_client = nullptr; delete info; });
352+ std::shared_ptr<AccountInfo> info(new AccountInfo);
353 info->service_enabled = false;
354 info->account_client = account_client;
355 info->account_service.reset(reinterpret_cast<AgAccountService*>(g_object_ref(account_service.get())), g_object_unref);
356@@ -226,51 +287,95 @@
357 : service_name_(service_name)
358 , service_type_(service_type)
359 , provider_name_(provider_name)
360- , use_external_main_loop_(main_loop_select == OnlineAccountClient::RunInExternalMainLoop)
361- , main_loop_is_running_(false)
362- , client_stopping_(false)
363- , logins_busy_(0)
364+ , main_loop_select_(main_loop_select)
365+ , main_loop_is_running_(main_loop_select != OnlineAccountClient::CreateInternalMainLoop)
366 {
367+ // Set up authentication pub/sub
368+ std::string oa_id = provider_name + ":" + service_type + ":" + service_name;
369+ RuntimeImpl::UPtr rt = RuntimeImpl::create(oa_id);
370+ MiddlewareFactory mw_factory(rt.get());
371+ RuntimeConfig rt_config("");
372+
373+ mw_ = mw_factory.create(oa_id, rt_config.default_middleware(), rt_config.default_middleware_configfile());
374+ if (main_loop_select == OnlineAccountClient::RunInExternalUiMainLoop)
375+ {
376+ // If this client was created by the shell, set up as the publisher
377+ auth_publisher_ = mw_->create_publisher(oa_id);
378+ }
379+ else
380+ {
381+ // If this client was created by a scope, set up as a subscriber
382+ auth_subscriber_ = mw_->create_subscriber(oa_id);
383+ auth_subscriber_->message_received().connect(std::bind(&OnlineAccountClientImpl::auth_callback, this, std::placeholders::_1));
384+ }
385+
386 // If we are responsible for the main loop
387- if (!use_external_main_loop_)
388+ if (main_loop_select_ == OnlineAccountClient::CreateInternalMainLoop)
389 {
390 // Wait here until either the main loop begins running, or the thread exits
391- std::unique_lock<std::mutex> lock(mutex_);
392- main_loop_thread_ = std::thread(&OnlineAccountClientImpl::main_loop_thread, this);
393- cond_.wait(lock, [this] { return main_loop_is_running_; });
394- }
395-
396- manager_.reset(ag_manager_new_for_service_type(service_type_.c_str()), g_object_unref);
397-
398- // Watch for changes to accounts
399- account_enabled_signal_id_ = g_signal_connect(manager_.get(), "enabled-event", G_CALLBACK(account_enabled_cb), this);
400- account_deleted_signal_id_ = g_signal_connect(manager_.get(), "account-deleted", G_CALLBACK(account_deleted_cb), this);
401-
402- // Now check initial state
403- refresh_service_statuses();
404+ {
405+ std::unique_lock<std::mutex> lock(mutex_);
406+ main_loop_thread_ = std::thread(&OnlineAccountClientImpl::main_loop_thread, this);
407+ cond_.wait_for(lock, std::chrono::seconds(5), [this] { return main_loop_is_running_; });
408+ }
409+ if (!main_loop_is_running_)
410+ {
411+ if (main_loop_)
412+ {
413+ // Quit the main loop, causing the thread to exit
414+ g_main_loop_quit(main_loop_.get());
415+ if (main_loop_thread_.joinable())
416+ {
417+ main_loop_thread_.join();
418+ }
419+ }
420+ if (thread_exception_)
421+ {
422+ std::rethrow_exception(thread_exception_);
423+ }
424+ else
425+ {
426+ throw unity::ResourceException("OnlineAccountClientImpl(): main_loop_thread failed to start.");
427+ }
428+ }
429+ }
430+ else
431+ {
432+ construct();
433+ }
434 }
435
436 OnlineAccountClientImpl::~OnlineAccountClientImpl()
437 {
438- // Disconnect all signal handlers
439- g_signal_handler_disconnect(manager_.get(), account_enabled_signal_id_);
440- g_signal_handler_disconnect(manager_.get(), account_deleted_signal_id_);
441-
442- for (auto info : accounts_)
443- {
444- g_signal_handler_disconnect(info.second->account_service.get(), info.second->service_update_signal_id_);
445- }
446-
447- // Wait until all currently running login sessions are done
448- {
449- std::unique_lock<std::mutex> lock(mutex_);
450- client_stopping_ = true;
451- }
452- flush_pending_sessions();
453-
454- // If we are responsible for the main loop, quit it on destruction
455- if (main_loop_)
456- {
457+ {
458+ std::lock_guard<std::mutex> lock(mutex_);
459+ if (thread_exception_)
460+ {
461+ try
462+ {
463+ std::rethrow_exception(thread_exception_);
464+ }
465+ catch (std::exception const& e)
466+ {
467+ std::cerr << "~OnlineAccountClientImpl(): main_loop_thread threw an exception: " << e.what() << std::endl;
468+ }
469+ catch (...)
470+ {
471+ std::cerr << "~OnlineAccountClientImpl(): main_loop_thread threw an unknown exception" << std::endl;
472+ }
473+ }
474+ }
475+
476+ // If we are responsible for the main loop
477+ if (main_loop_select_ == OnlineAccountClient::CreateInternalMainLoop)
478+ {
479+ // Invoke tear_down() from within the main loop
480+ {
481+ std::unique_lock<std::mutex> lock(mutex_);
482+ g_main_context_invoke(main_loop_context_.get(), main_loop_is_stopping_cb, this);
483+ cond_.wait(lock, [this] { return !main_loop_is_running_; });
484+ }
485+
486 // Quit the main loop, causing the thread to exit
487 g_main_loop_quit(main_loop_.get());
488 if (main_loop_thread_.joinable())
489@@ -278,16 +383,32 @@
490 main_loop_thread_.join();
491 }
492 }
493+ else
494+ {
495+ tear_down();
496+ }
497 }
498
499 void OnlineAccountClientImpl::set_service_update_callback(OnlineAccountClient::ServiceUpdateCallback callback)
500 {
501- std::lock_guard<std::mutex> lock(callback_mutex_);
502+ std::lock_guard<std::mutex> lock(mutex_);
503+ if (thread_exception_)
504+ {
505+ std::rethrow_exception(thread_exception_);
506+ }
507+
508+ std::lock_guard<std::mutex> callback_lock(callback_mutex_);
509 callback_ = callback;
510 }
511
512 void OnlineAccountClientImpl::refresh_service_statuses()
513 {
514+ std::unique_lock<std::mutex> lock(mutex_);
515+ if (thread_exception_)
516+ {
517+ std::rethrow_exception(thread_exception_);
518+ }
519+
520 std::shared_ptr<GList> enabled_accounts(ag_manager_list(manager_.get()), ag_manager_list_free);
521 GList* it;
522 for (it = enabled_accounts.get(); it; it = it->next)
523@@ -297,19 +418,25 @@
524 std::string provider_name = ag_account_get_provider_name(account.get());
525 if (provider_name == provider_name_)
526 {
527+ lock.unlock();
528 account_enabled_cb(manager_.get(), account_id, this);
529+ lock.lock();
530 }
531+ flush_pending_session(account_id, lock);
532 }
533- flush_pending_sessions();
534 }
535
536 std::vector<OnlineAccountClient::ServiceStatus> OnlineAccountClientImpl::get_service_statuses()
537 {
538 std::lock_guard<std::mutex> lock(mutex_);
539+ if (thread_exception_)
540+ {
541+ std::rethrow_exception(thread_exception_);
542+ }
543
544 // Return all service statuses
545 std::vector<OnlineAccountClient::ServiceStatus> service_statuses;
546- for (auto const info : accounts_)
547+ for (auto const& info : accounts_)
548 {
549 service_statuses.push_back(info_to_details(info.second.get()));
550 }
551@@ -352,42 +479,112 @@
552 widget.add_attribute_value("online_account_details", Variant(account_details_map));
553 }
554
555-void OnlineAccountClientImpl::flush_pending_sessions()
556-{
557+void OnlineAccountClientImpl::construct()
558+{
559+ manager_.reset(ag_manager_new_for_service_type(service_type_.c_str()), g_object_unref);
560+
561+ // Watch for changes to accounts
562+ account_enabled_signal_id_ = g_signal_connect(manager_.get(), "enabled-event", G_CALLBACK(account_enabled_cb), this);
563+ account_deleted_signal_id_ = g_signal_connect(manager_.get(), "account-deleted", G_CALLBACK(account_deleted_cb), this);
564+
565+ // Now check initial state
566+ refresh_service_statuses();
567+}
568+
569+void OnlineAccountClientImpl::tear_down()
570+{
571+ // Disconnect signal handlers
572+ g_signal_handler_disconnect(manager_.get(), account_enabled_signal_id_);
573+ g_signal_handler_disconnect(manager_.get(), account_deleted_signal_id_);
574+
575+ // Remove all accounts
576+ {
577+ std::unique_lock<std::mutex> lock(mutex_);
578+ for (auto const& info : accounts_)
579+ {
580+ // Before we nuke the map, ensure that any pending sessions are done
581+ g_signal_handler_disconnect(info.second->account_service.get(), info.second->service_update_signal_id_);
582+ clear_session(info.second.get());
583+ flush_pending_session(info.second->account_id, lock);
584+ }
585+ accounts_.clear();
586+ }
587+}
588+
589+void OnlineAccountClientImpl::flush_pending_session(AgAccountId const& account_id, std::unique_lock<std::mutex>& lock)
590+{
591+ // Get account info
592+ auto info_it = accounts_.find(account_id);
593+ if (info_it == accounts_.end())
594+ {
595+ return;
596+ }
597+ auto info = info_it->second;
598+
599 // Wait until all currently running login sessions are done
600 // (ensures that accounts_ is up to date)
601- std::unique_lock<std::mutex> lock(mutex_);
602+ std::shared_ptr<GMainLoop> event_loop;
603+ event_loop.reset(g_main_loop_new(g_main_context_get_thread_default(), true), g_main_loop_unref);
604+ while(info->session)
605 {
606- std::shared_ptr<GMainLoop> event_loop;
607- event_loop.reset(g_main_loop_new(nullptr, true), g_main_loop_unref);
608- while(logins_busy_ > 0)
609- {
610- // We need to wait inside an event loop to allow for the main application loop to
611- // process its pending events
612- g_timeout_add(100, wake_up_event_loop_cb, event_loop.get());
613- lock.unlock();
614- g_main_loop_run(event_loop.get());
615- lock.lock();
616- }
617+ // We need to wait inside an event loop to allow for the main application loop to
618+ // process its pending events
619+ std::shared_ptr<GSource> source;
620+ source.reset(g_timeout_source_new(10), g_source_unref);
621+ g_source_set_callback(source.get(), wake_up_event_loop_cb, event_loop.get(), NULL);
622+ g_source_attach(source.get(), g_main_context_get_thread_default());
623+
624+ lock.unlock();
625+ g_main_loop_run(event_loop.get());
626+ lock.lock();
627 }
628 }
629
630 void OnlineAccountClientImpl::main_loop_state_notify(bool is_running)
631 {
632+ bool was_running = false;
633+ {
634+ std::lock_guard<std::mutex> lock(mutex_);
635+ was_running = main_loop_is_running_;
636+ }
637+ if (!was_running && is_running)
638+ {
639+ construct();
640+ }
641+ else if (was_running && !is_running)
642+ {
643+ tear_down();
644+ }
645+
646 std::lock_guard<std::mutex> lock(mutex_);
647 main_loop_is_running_ = is_running;
648 cond_.notify_all();
649 }
650
651+std::shared_ptr<AgManager> OnlineAccountClientImpl::manager()
652+{
653+ return manager_;
654+}
655+
656 std::string OnlineAccountClientImpl::service_name()
657 {
658 std::lock_guard<std::mutex> lock(mutex_);
659 return service_name_;
660 }
661
662-void OnlineAccountClientImpl::callback(OnlineAccountClient::ServiceStatus const& service_status)
663+OnlineAccountClient::MainLoopSelect OnlineAccountClientImpl::main_loop_select()
664+{
665+ return main_loop_select_;
666+}
667+
668+void OnlineAccountClientImpl::callback(AccountInfo const* info, std::string const& error)
669 {
670 std::lock_guard<std::mutex> lock(callback_mutex_);
671+ auto service_status = info_to_details(info, error);
672+ if (service_status.service_authenticated && auth_publisher_)
673+ {
674+ auth_publisher_->send_message(details_to_json(service_status));
675+ }
676 if (callback_)
677 {
678 callback_(service_status);
679@@ -408,42 +605,80 @@
680
681 void OnlineAccountClientImpl::remove_account(AgAccountId const& account_id)
682 {
683- std::lock_guard<std::mutex> lock(mutex_);
684+ std::unique_lock<std::mutex> lock(mutex_);
685+
686+ // Get account info
687+ auto info_it = accounts_.find(account_id);
688+ if (info_it == accounts_.end())
689+ {
690+ return;
691+ }
692+ auto info = info_it->second;
693+
694+ // Before we nuke the pointer, ensure that any pending sessions are done
695+ g_signal_handler_disconnect(info->account_service.get(), info->service_update_signal_id_);
696+ clear_session(info.get());
697+ flush_pending_session(account_id, lock);
698+
699+ // Remove account info from accounts_ map
700 accounts_.erase(account_id);
701 }
702
703-bool OnlineAccountClientImpl::inc_logins()
704-{
705- std::lock_guard<std::mutex> lock(mutex_);
706- if (!client_stopping_)
707- {
708- // Increment number of logins busy
709- ++logins_busy_;
710- return true;
711- }
712- // Client is stopping so don't increment
713- return false;
714-}
715-
716-void OnlineAccountClientImpl::dec_logins()
717-{
718- // Decrement number of logins busy
719- std::lock_guard<std::mutex> lock(mutex_);
720- --logins_busy_;
721-}
722-
723 void OnlineAccountClientImpl::main_loop_thread()
724 {
725- // If something goes wrong causing the main loop not to run, the destruction of this pointer
726- // will break the constructor from its wait
727- std::shared_ptr<void> thread_exit_notifier(nullptr, [this](void*){ main_loop_state_notify(true); });
728-
729- // Stick a method call into the main loop to notify the constructor when the main loop begins running
730- g_idle_add(main_loop_is_running_cb, this);
731-
732- // Run the main loop
733- main_loop_.reset(g_main_loop_new(nullptr, true), g_main_loop_unref);
734- g_main_loop_run(main_loop_.get());
735+ // If something goes wrong causing the thread to abort, the destruction of this pointer update the
736+ // main_loop_is_running_ state accordingly.
737+ std::shared_ptr<void> thread_exit_notifier(nullptr, [this](void*){ main_loop_state_notify(false); });
738+
739+ try
740+ {
741+ main_loop_context_.reset(g_main_context_new(), g_main_context_unref);
742+ g_main_context_push_thread_default(main_loop_context_.get());
743+
744+ // Stick a method call into the main loop to notify the constructor when the main loop begins running
745+ g_main_context_invoke(main_loop_context_.get(), main_loop_is_running_cb, this);
746+
747+ // Run the main loop
748+ main_loop_.reset(g_main_loop_new(main_loop_context_.get(), true), g_main_loop_unref);
749+ g_main_loop_run(main_loop_.get());
750+ }
751+ catch (std::exception const& e)
752+ {
753+ std::cerr << "OnlineAccountClientImpl::main_loop_thread(): Thread aborted: " << e.what() << std::endl;
754+ std::lock_guard<std::mutex> lock(mutex_);
755+ thread_exception_ = std::current_exception();
756+ }
757+ catch (...)
758+ {
759+ std::cerr << "OnlineAccountClientImpl::main_loop_thread(): Thread aborted: unknown exception" << std::endl;
760+ std::lock_guard<std::mutex> lock(mutex_);
761+ thread_exception_ = std::current_exception();
762+ }
763+}
764+
765+void OnlineAccountClientImpl::auth_callback(std::string const& details_json)
766+{
767+ OnlineAccountClient::ServiceStatus details = json_to_details(details_json);
768+
769+ // Update account info
770+ {
771+ std::lock_guard<std::mutex> lock(mutex_);
772+ auto info_it = accounts_.find(details.account_id);
773+ if (info_it == accounts_.end())
774+ {
775+ return;
776+ }
777+ auto info = info_it->second;
778+
779+ GVariantDict dict;
780+ g_variant_dict_init(&dict, nullptr);
781+ g_variant_dict_insert(&dict, "AccessToken", "s", details.access_token.c_str());
782+ g_variant_dict_insert(&dict, "TokenSecret", "s", details.token_secret.c_str());
783+ info->session_data.reset(g_variant_ref_sink(g_variant_dict_end(&dict)), free_variant);
784+ }
785+
786+ std::lock_guard<std::mutex> lock(callback_mutex_);
787+ callback_(details);
788 }
789
790 } // namespace internal
791
792=== modified file 'test/gtest/scopes/OnlineAccountClient/OnlineAccountClient_test.cpp'
793--- test/gtest/scopes/OnlineAccountClient/OnlineAccountClient_test.cpp 2014-09-10 08:57:42 +0000
794+++ test/gtest/scopes/OnlineAccountClient/OnlineAccountClient_test.cpp 2014-10-08 11:00:22 +0000
795@@ -25,6 +25,8 @@
796
797 TEST(OnlineAccountClient, basic)
798 {
799+ setenv("XDG_RUNTIME_DIR", "/tmp", true);
800+
801 OnlineAccountClient oa_client("com.ubuntu.scopes.youtube_youtube", "sharing", "google");
802 auto service_statuses = oa_client.get_service_statuses();
803 }

Subscribers

People subscribed via source and target branches

to all changes: