From: Tomasz Marciniak Date: Fri, 18 Nov 2016 11:54:45 +0000 (+0100) Subject: [Convergence] Fixes for start() stop() and send() methods. X-Git-Tag: submit/tizen_3.0/20161123.043947~5^2 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=881742d658c94ebb6c50ad7a146e9c4002bedc75;p=platform%2Fcore%2Fapi%2Fwebapi-plugins.git [Convergence] Fixes for start() stop() and send() methods. [Features] Removed dynamic variables which could cause memory leaks, added variable which shows if service is connected, fixed validation. [Verification] Code compiles. Change-Id: I522797ef4d39114fe88fdc7b1bcaee96ea42ab7c Signed-off-by: Tomasz Marciniak --- diff --git a/src/convergence/convergence_api.js b/src/convergence/convergence_api.js index 7cad8500..87644437 100644 --- a/src/convergence/convergence_api.js +++ b/src/convergence/convergence_api.js @@ -589,6 +589,11 @@ function AppCommunicationService() { value: null, writable: true, enumerable: false + }, + _isStarted : { + value: false, + writable: true, + enumerable: false } }); @@ -633,6 +638,7 @@ native_.addListener('APP_COMMUNICATION_SERVICE_LISTENER', function(result) { native_.callIfPossible(s._connectCallback, s); break; case 'onStart': + s._isStarted = true; native_.callIfPossible(s._startCallback, new ChannelInfo(result.channel.uri, result.channel.id), null); break; @@ -641,6 +647,7 @@ native_.addListener('APP_COMMUNICATION_SERVICE_LISTENER', function(result) { new ChannelInfo(result.channel.uri, result.channel.id), null); break; case 'onStop': + s._isStarted = false; native_.callIfPossible(s._stopCallback, new ChannelInfo(result.channel.uri, result.channel.id), null); break; @@ -696,21 +703,20 @@ AppCommunicationService.prototype.start = function(channel, successCallback, err { name: 'successCallback', type: types_.FUNCTION, - //values: ConnectSuccessCallback, optional: false, nullable: false }, { name: 'errorCallback', type: types_.FUNCTION, - //values: ErrorCallback, optional: true, nullable: true } ]); - // TODO check if the service is connected and started - // Raise the exception instead + if (this._isStarted) { + throw new WebAPIException(WebAPIException.INVALID_VALUES_ERR, 'Service already started.'); + } var lid = this._serviceId; this._startCallback = successCallback; @@ -725,6 +731,7 @@ AppCommunicationService.prototype.start = function(channel, successCallback, err native_.callIfPossible(errorCallback, native_.getErrorObject(result)); } }); + if (native_.isFailure(result)) { throw native_.getErrorObject(result); } @@ -744,21 +751,20 @@ AppCommunicationService.prototype.stop = function(channel, successCallback, erro { name: 'successCallback', type: types_.FUNCTION, - //values: ConnectSuccessCallback, optional: true, nullable: true }, { name: 'errorCallback', type: types_.FUNCTION, - //values: ErrorCallback, optional: true, nullable: true } ]); - // TODO check if the service is connected and started - // Raise the exception instead + if (!this._isStarted) { + throw new WebAPIException(WebAPIException.INVALID_VALUES_ERR, 'Service is not started.'); + } var lid = -1; if (successCallback) { @@ -776,6 +782,7 @@ AppCommunicationService.prototype.stop = function(channel, successCallback, erro native_.callIfPossible(errorCallback, native_.getErrorObject(result)); } }); + if (native_.isFailure(result)) { throw native_.getErrorObject(result); } @@ -794,29 +801,27 @@ AppCommunicationService.prototype.send = function(channel, payload, successCallb }, { name: 'payload', - type: types_.PLATFORM_OBJECT, - values: [tizen.PayloadString, tizen.PayloadRawBytes], + type: types_.ARRAY, optional: false, nullable: false }, { name: 'successCallback', type: types_.FUNCTION, - //values: ConnectSuccessCallback, optional: false, nullable: false }, { name: 'errorCallback', type: types_.FUNCTION, - //values: ErrorCallback, optional: true, nullable: true } ]); - // TODO check if the service is connected and started - // Raise the exception instead + if (!this._isStarted) { + throw new WebAPIException(WebAPIException.INVALID_VALUES_ERR, 'Service is not started.'); + } var lid = this._serviceId; this._sendCallback = successCallback; @@ -832,6 +837,7 @@ AppCommunicationService.prototype.send = function(channel, payload, successCallb native_.callIfPossible(errorCallback, native_.getErrorObject(result)); } }); + if (native_.isFailure(result)) { throw native_.getErrorObject(result); } diff --git a/src/convergence/convergence_app_communication_service.cc b/src/convergence/convergence_app_communication_service.cc index afd5a1e0..c2c33e5a 100644 --- a/src/convergence/convergence_app_communication_service.cc +++ b/src/convergence/convergence_app_communication_service.cc @@ -59,7 +59,7 @@ ConvergenceAppCommunicationService::~ConvergenceAppCommunicationService() { } common::TizenResult ConvergenceAppCommunicationService::Start( - ConvergenceChannel *channel, + ConvergenceChannel& channel, const int cur_listener_id) { ScopeLogger(); @@ -67,119 +67,62 @@ common::TizenResult ConvergenceAppCommunicationService::Start( conv_service_h service_handle = FindServiceHandle(); if (!service_handle) { - LoggerE("AAAAAA!!! Service not found"); - return LogAndCreateTizenError(NotFoundError, - "Service with specified type does not exist"); + return LogAndCreateTizenError(AbortError, "Service with specified type does not exist"); } -LoggerI("1"); - UpdateListener(cur_listener_id); -LoggerI("2"); - - - { // DBG - conv_channel_h ch = channel->GetHandle(); - char *id = NULL; - conv_channel_get_string(ch, "channel_id", &id); - char *uri = NULL; - conv_channel_get_string(ch, "uri", &uri); - LoggerI("===== CHANNEL ID [%s] URI [%s]", id, uri); - } - - const int error = conv_service_start(service_handle, - channel->GetHandle(), nullptr); + const int error = conv_service_start(service_handle, channel.GetHandle(), nullptr); if (CONV_ERROR_NONE != error) { - // TODO: Handle error - trace_conv_error(error, __LINE__, "conv_service_start"); + return LogAndCreateTizenError(AbortError, "Failed to start service"); } -LoggerI("3"); - delete channel; -LoggerI("4"); return TizenSuccess(); } common::TizenResult ConvergenceAppCommunicationService::Stop( - ConvergenceChannel *channel, + ConvergenceChannel& channel, const int cur_listener_id) { ScopeLogger(); conv_service_h service_handle = FindServiceHandle(); if (!service_handle) { - return LogAndCreateTizenError(NotFoundError, - "Service with specified type does not exist"); + return LogAndCreateTizenError(AbortError, "Service with specified type does not exist"); } UpdateListener(cur_listener_id); - const int error = conv_service_stop(service_handle, - channel->GetHandle(), nullptr); + const int error = conv_service_stop(service_handle, channel.GetHandle(), nullptr); if (CONV_ERROR_NONE != error) { - // TODO: Handle error - trace_conv_error(error, __LINE__, "conv_service_stop"); + return LogAndCreateTizenError(AbortError, "Failed to stop service"); } - delete channel; return TizenSuccess(); } common::TizenResult ConvergenceAppCommunicationService::Send( - ConvergenceChannel *channel, - ConvergencePayloadArray *payload, + ConvergenceChannel& channel, + ConvergencePayloadArray& payload, const int cur_listener_id) { ScopeLogger(); conv_service_h service_handle = FindServiceHandle(); if (!service_handle) { - return LogAndCreateTizenError(NotFoundError, - "Service with specified type does not exist"); + return LogAndCreateTizenError(AbortError, "Service with specified type does not exist"); } UpdateListener(cur_listener_id); - { // DBG - LoggerI("...PUBLISHING for service handle [0x0%x]", service_handle); - - conv_service_e t = CONV_SERVICE_NONE; - int e = conv_service_get_type(service_handle, &t); - if (CONV_ERROR_NONE != e) { - trace_conv_error(e, __LINE__, "get service type"); - } - LoggerI("....type [%d]", t); - - char *sid = nullptr; - e = conv_service_get_property_string(service_handle, CONV_SERVICE_ID, &sid); - if (CONV_ERROR_NONE != e) { - trace_conv_error(e, __LINE__, "get service id"); - } - LoggerI("....id [%s]", sid); - free(sid); - - conv_channel_h ch = channel->GetHandle(); - char *id = NULL; - conv_channel_get_string(ch, "channel_id", &id); - char *uri = NULL; - conv_channel_get_string(ch, "uri", &uri); - LoggerI("===== CHANNEL ID [%s] URI [%s]", id, uri); - } - - const int error = conv_service_publish(service_handle, - channel->GetHandle(), payload->GetHandle()); + const int error = conv_service_publish(service_handle, channel.GetHandle(), payload.GetHandle()); if (CONV_ERROR_NONE != error) { - // TODO: Handle error - trace_conv_error(error, __LINE__, "conv_service_publish"); + return LogAndCreateTizenError(AbortError, "Failed to publish message"); } else { LoggerI("PUBLISHED SUCCESSFULY listener is [%d]", cur_listener_id); } - delete channel; - delete payload; return TizenSuccess(); } - // TODO move to Payload class @@ -381,7 +324,7 @@ ConvergenceAppCommunicationServerService::~ConvergenceAppCommunicationServerServ } common::TizenResult ConvergenceAppCommunicationServerService::Start( - ConvergenceChannel *channel, + ConvergenceChannel& channel, const int cur_listener_id) { ScopeLogger(); @@ -393,24 +336,20 @@ common::TizenResult ConvergenceAppCommunicationServerService::Start( picojson::object param; param[kServiceListenerStatus] = picojson::value(kServiceListenerStatusOk); - // The object 'channel' will be deleted in the super::Start() so we should - // add its json before - param[kChannel] = ConvergenceChannel::ToJson(channel->GetHandle()); // Define string as constant + param[kChannel] = ConvergenceChannel::ToJson(channel.GetHandle()); // Define string as constant param[kServiceResultType] = picojson::value(kServiceResultTypeOnStart); - common::TizenResult result = ConvergenceAppCommunicationService::Start( - channel, cur_listener_id); + common::TizenResult result = ConvergenceAppCommunicationService::Start(channel, cur_listener_id); if (!result) { return result; } - convergence_plugin_->ReplyAsync(kAppCommunicationListenerCallback, - cur_listener_id, true, param); + convergence_plugin_->ReplyAsync(kAppCommunicationListenerCallback, cur_listener_id, true, param); return result; } common::TizenResult ConvergenceAppCommunicationServerService::Stop( - ConvergenceChannel *channel, + ConvergenceChannel& channel, const int cur_listener_id) { ScopeLogger(); @@ -421,19 +360,15 @@ common::TizenResult ConvergenceAppCommunicationServerService::Stop( picojson::object param; param[kServiceListenerStatus] = picojson::value(kServiceListenerStatusOk); - // The object 'channel' will be deleted in the super::Start() so we should - // add its json before - param[kChannel] = ConvergenceChannel::ToJson(channel->GetHandle()); + param[kChannel] = ConvergenceChannel::ToJson(channel.GetHandle()); param[kServiceResultType] = picojson::value(kServiceResultTypeOnStop); - common::TizenResult result = ConvergenceAppCommunicationService::Stop( - channel, cur_listener_id); + common::TizenResult result = ConvergenceAppCommunicationService::Stop(channel, cur_listener_id); if (!result) { return result; } - convergence_plugin_->ReplyAsync(kAppCommunicationListenerCallback, - cur_listener_id, true, param); + convergence_plugin_->ReplyAsync(kAppCommunicationListenerCallback, cur_listener_id, true, param); return result; } diff --git a/src/convergence/convergence_app_communication_service.h b/src/convergence/convergence_app_communication_service.h index df35762c..ca91bb38 100644 --- a/src/convergence/convergence_app_communication_service.h +++ b/src/convergence/convergence_app_communication_service.h @@ -43,13 +43,11 @@ class ConvergenceAppCommunicationService : public ConvergenceService { ConvergenceAppCommunicationService& operator=(const ConvergenceAppCommunicationService&) = delete; ConvergenceAppCommunicationService& operator=(ConvergenceAppCommunicationService&&) = delete; public: - virtual common::TizenResult Start(ConvergenceChannel *channel, - const int cur_listener_id); - virtual common::TizenResult Stop(ConvergenceChannel *channel, - const int cur_listener_id); - virtual common::TizenResult Send(ConvergenceChannel *channel, - ConvergencePayloadArray *payload, - const int cur_listener_id); + virtual common::TizenResult Start(ConvergenceChannel& channel, const int cur_listener_id); + virtual common::TizenResult Stop(ConvergenceChannel& channel, const int cur_listener_id); + virtual common::TizenResult Send(ConvergenceChannel& channel, + ConvergencePayloadArray& payload, + const int cur_listener_id); common::TizenResult SetListener(const int cur_listener_id); common::TizenResult RemoveListener(); private: @@ -73,9 +71,9 @@ class ConvergenceAppCommunicationServerService : public ConvergenceAppCommunicat ConvergenceAppCommunicationServerService& operator=(ConvergenceAppCommunicationServerService&&) = delete; public: - virtual common::TizenResult Start(ConvergenceChannel *channel, + virtual common::TizenResult Start(ConvergenceChannel& channel, const int cur_listener_id); - virtual common::TizenResult Stop(ConvergenceChannel *channel, + virtual common::TizenResult Stop(ConvergenceChannel& channel, const int cur_listener_id); }; diff --git a/src/convergence/convergence_instance.cc b/src/convergence/convergence_instance.cc index 51316fcf..a9d69022 100644 --- a/src/convergence/convergence_instance.cc +++ b/src/convergence/convergence_instance.cc @@ -425,11 +425,16 @@ common::TizenResult ConvergenceInstance::AppCommunicationServiceStart(const pico result = LogAndCreateTizenError(NotFoundError, "Can not find the service type = 1"); } else { // Running the service start procedure - result = service->Start(new ConvergenceChannel(ConvergenceUtils::GetArg(args, kJSArgumentChannel)), - static_cast(ConvergenceUtils::GetArg(args, kJSCurrentListenerId).get())); + ConvergenceChannel channel(ConvergenceUtils::GetArg(args, kJSArgumentChannel)); + const int id = static_cast(ConvergenceUtils::GetArg(args, kJSCurrentListenerId).get()); + + result = service->Start(channel, id); } - this->Post(token, result); + //in case of failure call error callback, success callback will be triggered by listener + if (!result) { + this->Post(token, result); + } }; std::thread(start, token).detach(); @@ -463,13 +468,17 @@ common::TizenResult ConvergenceInstance::AppCommunicationServiceSend(const picoj result = LogAndCreateTizenError(NotFoundError, "Can not find the service type = 1"); } else { // Running the service send procedure - result = service->Send(new ConvergenceChannel(ConvergenceUtils::GetArg(args, kJSArgumentChannel)), - new ConvergencePayloadArray(ConvergenceUtils::GetArg(args, kJSArgumentPayload)), - static_cast(ConvergenceUtils::GetArg(args, kJSCurrentListenerId).get())); + ConvergenceChannel channel(ConvergenceUtils::GetArg(args, kJSArgumentChannel)); + ConvergencePayloadArray payload(ConvergenceUtils::GetArg(args, kJSArgumentPayload)); + const int id = static_cast(ConvergenceUtils::GetArg(args, kJSCurrentListenerId).get()); + result = service->Send(channel, payload, id); } - this->Post(token, result); + //in case of failure call error callback, success callback will be triggered by listener + if (!result) { + this->Post(token, result); + } }; std::thread(send, token).detach(); @@ -502,11 +511,16 @@ common::TizenResult ConvergenceInstance::AppCommunicationServiceStop(const picoj result = LogAndCreateTizenError(NotFoundError, "Can not find the service type = 1"); } else { // Running the service stop procedure - result = service->Stop(new ConvergenceChannel(ConvergenceUtils::GetArg(args, kJSArgumentChannel)), - static_cast(ConvergenceUtils::GetArg(args, kJSCurrentListenerId).get())); + ConvergenceChannel channel(ConvergenceUtils::GetArg(args, kJSArgumentChannel)); + const int id = static_cast(ConvergenceUtils::GetArg(args, kJSCurrentListenerId).get()); + + result = service->Stop(channel, id); } - this->Post(token, result); + //in case of failure call error callback, success callback will be triggered by listener + if (!result) { + this->Post(token, result); + } }; std::thread(stop, token).detach();