From cef3507a40fccbd89ed52e7a9dab35fe49b5a216 Mon Sep 17 00:00:00 2001 From: hyunho Date: Thu, 7 May 2020 14:47:28 +0900 Subject: [PATCH] Fix ambient watch surface handle management bug The current watch surface handle should be destroyed after the owner watch application is dead. Change-Id: Ie790c1d5942833f4a0b89de7e6787202da4aac1d Signed-off-by: hyunho --- ambient-viewer/include/ambient_viewer.h | 1 + ambient-viewer/src/ambient-viewer.cc | 55 ++++++++++++++++++++---- ambient-viewer/src/ambient-viewer.hh | 6 ++- ambient-viewer/src/stub.cc | 7 ++- ambient-viewer/src/watch-surface.cc | 5 ++- unittest/src/test_ambient_viewer.cc | 6 ++- unittest/src/test_ambient_viewer_stub.cc | 5 +-- 7 files changed, 69 insertions(+), 16 deletions(-) diff --git a/ambient-viewer/include/ambient_viewer.h b/ambient-viewer/include/ambient_viewer.h index 8eedf112..b2f3bf17 100644 --- a/ambient-viewer/include/ambient_viewer.h +++ b/ambient-viewer/include/ambient_viewer.h @@ -224,6 +224,7 @@ int ambient_viewer_unset_event_listener(ambient_viewer_h handle); * @return #AMBIENT_VIEWER_ERROR_NONE On success, other value on failure * @retval #AMBIENT_VIEWER_ERROR_NONE Success * @retval #AMBIENT_VIEWER_ERROR_INVALID_PARAMETER Invalid parameter + * @retval #AMBIENT_VIEWER_ERROR_INVALID_OPERATION Invalid operation * @see #ambient_viewer_surface_h */ int ambient_viewer_get_watch_surface(ambient_viewer_h handle, diff --git a/ambient-viewer/src/ambient-viewer.cc b/ambient-viewer/src/ambient-viewer.cc index 56f4951a..60dd2ea2 100644 --- a/ambient-viewer/src/ambient-viewer.cc +++ b/ambient-viewer/src/ambient-viewer.cc @@ -20,6 +20,7 @@ #include #include #include +#include #include "ambient-viewer.hh" #include "top-app-surface.hh" @@ -42,7 +43,12 @@ AmbientViewer::AmbientViewer(Evas_Object* win) : win_(win) { if (aul_app_com_create("aod.ambientevent", nullptr, OnReceiveSignal, this, &receive_signal_conn_) != AUL_R_OK) { - LOGE("Failed to listen sendextra signal"); + LOGE("Failed to listen send extra signal"); + } + + if (aul_app_com_create("watch.dead", nullptr, + OnDeadSignal, this, &dead_signal_conn_) != AUL_R_OK) { + LOGE("failed to listen dead signal"); } LOGD("CREATE AMBIENT VIEWER"); @@ -51,6 +57,8 @@ AmbientViewer::AmbientViewer(Evas_Object* win) AmbientViewer::~AmbientViewer() { if (receive_signal_conn_) aul_app_com_leave(receive_signal_conn_); + if (dead_signal_conn_) + aul_app_com_leave(dead_signal_conn_); } std::shared_ptr AmbientViewer::CreateWatchSurface(int rid, @@ -85,9 +93,15 @@ void AmbientViewer::OnChangedSignal(keynode_t *node, void *user_data) { try { Bundle data(safe_raw.get()); string appid = data.GetString(NOTIFY_CHANGED_EVENT_APPID_KEY); + if (!viewer->watch_stack_.empty()) { + if (viewer->watch_stack_.back()->GetAppId() == appid) { + LOGW("Already activated watch(%s)", appid.c_str()); + return; + } + } string rid = data.GetString(NOTIFY_CHANGED_EVENT_RID_KEY); - viewer->watch_surface_ = viewer->CreateWatchSurface( - stoi(rid), viewer->GetUUID(rid), appid, viewer->win_, viewer); + viewer->watch_stack_.push_back(viewer->CreateWatchSurface( + stoi(rid), viewer->GetUUID(rid), appid, viewer->win_, viewer)); viewer->OnReceived(EVENT_WATCH_CHANGED, appid, data); } catch (const std::exception& e) { LOGE("Exception occurred : %s", e.what()); @@ -95,6 +109,24 @@ void AmbientViewer::OnChangedSignal(keynode_t *node, void *user_data) { } } +int AmbientViewer::OnDeadSignal(const char *endpoint, aul_app_com_result_e e, + bundle *envelope, void *user_data) { + AmbientViewer* viewer = (AmbientViewer*)user_data; + Bundle data(envelope, false, false); + string appid = data.GetString(AUL_K_APPID); + LOGW("Watch(%s) DEAD", appid.c_str()); + list>::iterator it; + for (it = viewer->watch_stack_.begin(); it != viewer->watch_stack_.end(); ++it) { + if ((*it)->GetAppId() == appid) { + viewer->watch_stack_.erase(it); + LOGW("REMOVE DEAD WATCH (%s)", appid.c_str()); + break; + } + } + LOGW("DEAD DONE (%s)", appid.c_str()); + return 0; +} + void AmbientViewer::Monitor(bool full) { if (full) { @@ -112,7 +144,7 @@ void AmbientViewer::Monitor(bool full) { void AmbientViewer::Unmonitor() { top_app_surface_.reset(); - watch_surface_.reset(); + watch_stack_.clear(); } void AmbientViewer::BlockUpdate(bool enable) { @@ -121,10 +153,13 @@ void AmbientViewer::BlockUpdate(bool enable) { LOGW("Top App Block Update (%d)", enable); } - if (watch_surface_ != nullptr && watch_surface_.get() != nullptr) { - watch_surface_->BlockUpdate(enable); - LOGW("Watch Block Update (%d)", enable); + if (watch_stack_.empty()) { + LOGW("Empty stack!"); + return; } + + watch_stack_.back()->BlockUpdate(enable); + LOGW("Watch Block Update (%d)", enable); } void AmbientViewer::OnReceived(AmbientViewer::EventType ev, std::string sender, @@ -185,7 +220,11 @@ int AmbientViewer::NotifyAmbientEvent(bool enter, AmbientViewer::Direction dir, } const ISurface& AmbientViewer::GetWatchSurface() const { - return *watch_surface_; + if (watch_stack_.empty()) { + LOGE("watch surface stack is empty !"); + throw std::runtime_error("watch surface stack is empty !"); + } + return *watch_stack_.back(); } const ISurface& AmbientViewer::GetTopAppSurface() const { diff --git a/ambient-viewer/src/ambient-viewer.hh b/ambient-viewer/src/ambient-viewer.hh index a4172c13..a9f2d586 100644 --- a/ambient-viewer/src/ambient-viewer.hh +++ b/ambient-viewer/src/ambient-viewer.hh @@ -25,6 +25,7 @@ #include #include +#include #include "iambient-viewer.hh" #include "isurface.hh" @@ -71,12 +72,15 @@ class EXPORT_API AmbientViewer : public IAmbientViewer { private: std::string GetUUID(std::string rid) const; + static int OnDeadSignal(const char *endpoint, aul_app_com_result_e e, + bundle *envelope, void *user_data); static void OnChangedSignal(keynode_t *node, void *user_data); static int OnReceiveSignal(const char *endpoint, aul_app_com_result_e e, bundle *envelope, void *user_data); Evas_Object* win_; - std::shared_ptr watch_surface_; + std::list> watch_stack_; std::shared_ptr top_app_surface_; + aul_app_com_connection_h dead_signal_conn_ = nullptr; aul_app_com_connection_h receive_signal_conn_ = nullptr; }; diff --git a/ambient-viewer/src/stub.cc b/ambient-viewer/src/stub.cc index be81a29f..e2ec33fe 100644 --- a/ambient-viewer/src/stub.cc +++ b/ambient-viewer/src/stub.cc @@ -323,7 +323,12 @@ extern "C" EXPORT_API int ambient_viewer_get_watch_surface( } AmbientViewerStub* stub = static_cast(handle); - *surface = static_cast(&(stub->GetWatchSurface())); + try { + *surface = static_cast(&(stub->GetWatchSurface())); + } catch (const std::exception& e) { + LOGE("Invalid parameter"); + return AMBIENT_VIEWER_ERROR_INVALID_OPERATION; + } return AMBIENT_VIEWER_ERROR_NONE; } diff --git a/ambient-viewer/src/watch-surface.cc b/ambient-viewer/src/watch-surface.cc index 7de726f0..ca2ededf 100644 --- a/ambient-viewer/src/watch-surface.cc +++ b/ambient-viewer/src/watch-surface.cc @@ -36,7 +36,9 @@ WatchSurface::WatchSurface( LOGI("WatchSurface created (%s)", appid.c_str()); } -WatchSurface::~WatchSurface() = default; +WatchSurface::~WatchSurface() { + LOGI("WatchSurface DESTROYED (%s)", appid_.c_str()); +} Evas_Object* WatchSurface::GetCurrentImage() const { return image_; @@ -116,6 +118,7 @@ void WatchSurface::OnEvasRemoved(const std::string& appId, const screen_connector::EvasObject& image) { image_ = nullptr; listener_->OnRemoved(*this); + LOGW("watch surface removed (%s) (%d)", appid_.c_str(), pid); } void WatchSurface::OnEvasChanged(const std::string& appId, diff --git a/unittest/src/test_ambient_viewer.cc b/unittest/src/test_ambient_viewer.cc index 5f693a4e..ce9516d7 100644 --- a/unittest/src/test_ambient_viewer.cc +++ b/unittest/src/test_ambient_viewer.cc @@ -84,8 +84,10 @@ void __receive_fake_message() { int __aul_app_com_create(const char *endpoint, aul_app_com_permission_h permission, app_com_cb callback, void *user_data, aul_app_com_connection_h *connection) { - __app_com_callback = callback; - __app_com_user_data = user_data; + if(strcmp(endpoint, "aod.ambientevent") == 0) { + __app_com_callback = callback; + __app_com_user_data = user_data; + } return 0; } diff --git a/unittest/src/test_ambient_viewer_stub.cc b/unittest/src/test_ambient_viewer_stub.cc index 426d83a9..9f16c289 100644 --- a/unittest/src/test_ambient_viewer_stub.cc +++ b/unittest/src/test_ambient_viewer_stub.cc @@ -135,7 +135,7 @@ TEST_F(AmbientViewerStubTest, ambient_viewer_get_watch_surface_n) { AMBIENT_VIEWER_ERROR_INVALID_PARAMETER); } -TEST_F(AmbientViewerStubTest, ambient_viewer_get_watch_surface) { +TEST_F(AmbientViewerStubTest, ambient_viewer_get_watch_surface_n2) { ambient_viewer_h handle = nullptr; ambient_viewer_surface_h surface = nullptr; ASSERT_EQ(ambient_viewer_create_mock(reinterpret_cast(&__not_null), @@ -143,8 +143,7 @@ TEST_F(AmbientViewerStubTest, ambient_viewer_get_watch_surface) { ASSERT_NE(handle, nullptr); EXPECT_EQ(ambient_viewer_get_watch_surface(handle, &surface), - AMBIENT_VIEWER_ERROR_NONE); - EXPECT_EQ(surface, nullptr); + AMBIENT_VIEWER_ERROR_INVALID_OPERATION); ASSERT_EQ(ambient_viewer_destroy(handle), AMBIENT_VIEWER_ERROR_NONE); -- 2.34.1