diff --git a/services/abilitymgr/include/skill/skill_execute_manager.h b/services/abilitymgr/include/skill/skill_execute_manager.h index d5466895a4..d95990d8f2 100644 --- a/services/abilitymgr/include/skill/skill_execute_manager.h +++ b/services/abilitymgr/include/skill/skill_execute_manager.h @@ -33,6 +33,7 @@ namespace AAFwk { class SkillExecuteManager { DECLARE_DELAYED_SINGLETON(SkillExecuteManager) +friend class SkillExecuteManagerTest; public: int32_t GenerateSkillWant(const AppExecFwk::SkillInfo &skillInfo, Want &want, int32_t userId, const std::string &requestCode, AppExecFwk::ExtensionAbilityType &targetType, diff --git a/services/abilitymgr/src/skill/skill_execute_manager.cpp b/services/abilitymgr/src/skill/skill_execute_manager.cpp index d6c29fbdf1..ee5c66f5cc 100644 --- a/services/abilitymgr/src/skill/skill_execute_manager.cpp +++ b/services/abilitymgr/src/skill/skill_execute_manager.cpp @@ -186,46 +186,53 @@ int32_t SkillExecuteManager::ExecuteSkillDone(const std::string &requestCode, in TAG_LOGD(AAFwkTag::ABILITYMGR, "execute skill done, requestCode:%{public}s code:%{public}d", requestCode.c_str(), resultCode); - std::lock_guard lock(mutex_); - auto it = records_.find(requestCode); - if (it == records_.end()) { - TAG_LOGE(AAFwkTag::ABILITYMGR, "record not found, requestCode:%{public}s", requestCode.c_str()); - return ERR_CODE_INVALID_ID; - } - auto record = it->second; - if (record->targetBundleName != callerBundleName) { - TAG_LOGE(AAFwkTag::ABILITYMGR, - "bundleName %{public}s and %{public}s mismatch", - callerBundleName.c_str(), record->targetBundleName.c_str()); - return ERR_INVALID_VALUE; - } - if (record->state != SkillExecuteState::EXECUTING) { - TAG_LOGE(AAFwkTag::ABILITYMGR, "invalid state:%{public}d", static_cast(record->state)); - return ERR_INVALID_VALUE; - } + sptr callback; + { + std::lock_guard lock(mutex_); + auto it = records_.find(requestCode); + if (it == records_.end()) { + TAG_LOGE(AAFwkTag::ABILITYMGR, "record not found, requestCode:%{public}s", + requestCode.c_str()); + return ERR_CODE_INVALID_ID; + } + + auto record = it->second; + if (record->targetBundleName != callerBundleName) { + TAG_LOGE(AAFwkTag::ABILITYMGR, + "bundleName %{public}s and %{public}s mismatch", + callerBundleName.c_str(), record->targetBundleName.c_str()); + return ERR_INVALID_VALUE; + } + if (record->state != SkillExecuteState::EXECUTING) { + TAG_LOGE(AAFwkTag::ABILITYMGR, "invalid state:%{public}d", static_cast(record->state)); + return ERR_INVALID_VALUE; + } #ifdef SUPPORT_UPMS - if (!result.uris.empty() && !record->callerBundleName.empty()) { - std::vector uriList; - for (const auto &uriStr : result.uris) { - uriList.emplace_back(uriStr); + if (!result.uris.empty() && !record->callerBundleName.empty()) { + std::vector uriList; + for (const auto &uriStr : result.uris) { + uriList.emplace_back(uriStr); + } + auto &uriPermClient = UriPermissionManagerClient::GetInstance(); + auto ret = uriPermClient.GrantUriPermission( + uriList, result.flags, record->callerBundleName, 0, record->callerTokenId); + if (ret != ERR_OK) { + TAG_LOGE(AAFwkTag::ABILITYMGR, "GrantUriPermission failed, ret:%{public}d", ret); + } } - auto &uriPermClient = UriPermissionManagerClient::GetInstance(); - auto ret = uriPermClient.GrantUriPermission( - uriList, result.flags, record->callerBundleName, 0, record->callerTokenId); - if (ret != ERR_OK) { - TAG_LOGE(AAFwkTag::ABILITYMGR, "GrantUriPermission failed, ret:%{public}d", ret); - } - } #endif - RemoveSkillExecuteTimeoutLocked(record->requestCodeSeq); - record->state = SkillExecuteState::EXECUTE_DONE; - if (record->callback != nullptr) { - record->callback->OnExecuteDone(requestCode, resultCode, result); + RemoveSkillExecuteTimeoutLocked(record->requestCodeSeq); + record->state = SkillExecuteState::EXECUTE_DONE; + callback = record->callback; + RemoveRecord(requestCode); + } + + if (callback != nullptr) { + callback->OnExecuteDone(requestCode, resultCode, result); } - RemoveRecord(requestCode); return ERR_OK; } @@ -354,33 +361,38 @@ void SkillExecuteManager::RemoveSkillExecuteTimeoutLocked(uint64_t requestCodeSe void SkillExecuteManager::OnTimeout(int64_t requestCodeSeq) { TAG_LOGI(AAFwkTag::ABILITYMGR, "called, seq:%{public}" PRId64, requestCodeSeq); + + sptr callback; std::string requestCode; { std::lock_guard lock(mutex_); auto seqIt = seqToRequestCodeMap_.find(requestCodeSeq); if (seqIt == seqToRequestCodeMap_.end()) { - TAG_LOGW(AAFwkTag::ABILITYMGR, "seq not found"); + TAG_LOGW(AAFwkTag::ABILITYMGR, "seq not found, seq:%{public}" PRId64, requestCodeSeq); return; } requestCode = seqIt->second; seqToRequestCodeMap_.erase(seqIt); + + auto it = records_.find(requestCode); + if (it == records_.end()) { + return; + } + auto &record = it->second; + if (record->state != SkillExecuteState::EXECUTING) { + return; + } + + TAG_LOGW(AAFwkTag::ABILITYMGR, "skill execute timed out, req:%{public}s", requestCode.c_str()); + record->state = SkillExecuteState::TIMED_OUT; + callback = record->callback; + RemoveRecord(requestCode); } - std::lock_guard lock(mutex_); - auto it = records_.find(requestCode); - if (it == records_.end()) { - return; - } - auto record = it->second; - if (record->state != SkillExecuteState::EXECUTING) { - return; - } - TAG_LOGW(AAFwkTag::ABILITYMGR, "skill execute timed out, req:%{public}s", requestCode.c_str()); - record->state = SkillExecuteState::TIMED_OUT; - if (record->callback != nullptr) { + + if (callback != nullptr) { AppExecFwk::SkillExecuteResult emptyResult; - record->callback->OnExecuteDone(requestCode, ERR_TIMED_OUT, emptyResult); + callback->OnExecuteDone(requestCode, ERR_TIMED_OUT, emptyResult); } - RemoveRecord(requestCode); } } // namespace AAFwk diff --git a/test/unittest/BUILD.gn b/test/unittest/BUILD.gn index c166f3bd19..462235640f 100644 --- a/test/unittest/BUILD.gn +++ b/test/unittest/BUILD.gn @@ -519,6 +519,7 @@ group("unittest") { "service_router_mgr_service_test:unittest", "skill_execute_callback_proxy_test:unittest", "skill_execute_callback_stub_test:unittest", + "skill_execute_manager_test:unittest", "skill_execute_param_test:unittest", "skill_execute_result_test:unittest", "services/ability_util_test:unittest", diff --git a/test/unittest/skill_execute_manager_test/BUILD.gn b/test/unittest/skill_execute_manager_test/BUILD.gn new file mode 100644 index 0000000000..46466ac569 --- /dev/null +++ b/test/unittest/skill_execute_manager_test/BUILD.gn @@ -0,0 +1,63 @@ +# Copyright (c) 2026 Huawei Device Co., Ltd. +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import("//build/test.gni") +import("//foundation/ability/ability_runtime/ability_runtime.gni") + +module_output_path = "ability_runtime/ability_runtime/skill" + +ohos_unittest("skill_execute_manager_test") { + module_out_path = module_output_path + + include_dirs = [ + "${ability_runtime_test_path}/mock/services_abilitymgr_test/libs/system_ability_mock", + "${ability_runtime_test_path}/unittest/skill_execute_callback_proxy_test", + ] + + sources = [ "skill_execute_manager_test.cpp" ] + + configs = [ + "${ability_runtime_services_path}/abilitymgr:abilityms_config", + "${ability_runtime_test_path}/mock/services_abilitymgr_test:aafwk_mock_config", + ] + + cflags = [] + if (target_cpu == "arm") { + cflags += [ "-DBINDER_IPC_32BIT" ] + } + + deps = [ + "${ability_runtime_innerkits_path}/ability_manager:ability_manager", + "${ability_runtime_innerkits_path}/deps_wrapper:ability_deps_wrapper", + "${ability_runtime_services_path}/abilitymgr:abilityms", + "${ability_runtime_services_path}/abilitymgr:abilityms_target", + ] + + external_deps = [ + "ability_base:want", + "ability_base:zuri", + "bundle_framework:libappexecfwk_common", + "c_utils:utils", + "ffrt:libffrt", + "googletest:gmock_main", + "googletest:gtest_main", + "hilog:libhilog", + "ipc:ipc_core", + "samgr:samgr_proxy", + ] +} + +group("unittest") { + testonly = true + deps = [ ":skill_execute_manager_test" ] +} diff --git a/test/unittest/skill_execute_manager_test/skill_execute_manager_test.cpp b/test/unittest/skill_execute_manager_test/skill_execute_manager_test.cpp new file mode 100644 index 0000000000..862c700369 --- /dev/null +++ b/test/unittest/skill_execute_manager_test/skill_execute_manager_test.cpp @@ -0,0 +1,274 @@ +/* + * Copyright (c) 2026 Huawei Device Co., Ltd. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +#include + +#include "ability_manager_errors.h" +#include "hilog_tag_wrapper.h" +#include "mock_skill_execute_callback_stub.h" +#include "singleton.h" +#include "skill/skill_execute_manager.h" +#include "skill/skill_execute_record.h" +#include "skill_execute_result.h" + +using namespace testing::ext; +using namespace testing; + +namespace OHOS { +namespace AAFwk { + +namespace { +constexpr const char *TARGET_BUNDLE = "com.example.target"; +constexpr const char *CALLER_BUNDLE = "com.example.caller"; +} // namespace + +class SkillExecuteManagerTest : public testing::Test { +public: + static void SetUpTestCase(void) {} + static void TearDownTestCase(void) {} + void SetUp() override; + void TearDown() override; + +protected: + void SeedRecord(const std::string &requestCode, uint64_t seq, + SkillExecuteState state = SkillExecuteState::EXECUTING); + + std::shared_ptr mgr_; + sptr callback_; +}; + +void SkillExecuteManagerTest::SetUp() +{ + mgr_ = DelayedSingleton::GetInstance(); + ASSERT_NE(mgr_, nullptr); + callback_ = new MockSkillExecuteCallbackStub(); + ASSERT_NE(callback_, nullptr); +} + +void SkillExecuteManagerTest::TearDown() +{ + { + std::lock_guard lock(mgr_->mutex_); + mgr_->records_.clear(); + mgr_->seqToRequestCodeMap_.clear(); + } + callback_ = nullptr; + mgr_ = nullptr; +} + +void SkillExecuteManagerTest::SeedRecord(const std::string &requestCode, uint64_t seq, + SkillExecuteState state) +{ + auto record = std::make_shared(); + record->requestCode = requestCode; + record->targetBundleName = TARGET_BUNDLE; + record->callerBundleName = CALLER_BUNDLE; + record->requestCodeSeq = seq; + record->state = state; + record->callback = callback_; + + std::lock_guard lock(mgr_->mutex_); + mgr_->records_[requestCode] = record; + mgr_->seqToRequestCodeMap_[seq] = requestCode; +} + +/** + * @tc.name: ExecuteSkillDone_0100 + * @tc.desc: ExecuteSkillDone happy path: callback invoked, record removed. + * @tc.type: FUNC + */ +HWTEST_F(SkillExecuteManagerTest, ExecuteSkillDone_0100, TestSize.Level1) +{ + TAG_LOGI(AAFwkTag::TEST, "begin"); + SeedRecord("req001", 1); + EXPECT_CALL(*callback_, OnExecuteDone(Eq("req001"), Eq(0), _)).Times(1); + + AppExecFwk::SkillExecuteResult result; + auto ret = mgr_->ExecuteSkillDone("req001", 0, result, TARGET_BUNDLE); + EXPECT_EQ(ret, ERR_OK); + TAG_LOGI(AAFwkTag::TEST, "end"); +} + +/** + * @tc.name: ExecuteSkillDone_0200 + * @tc.desc: ExecuteSkillDone on unknown requestCode returns ERR_CODE_INVALID_ID. + * @tc.type: FUNC + */ +HWTEST_F(SkillExecuteManagerTest, ExecuteSkillDone_0200, TestSize.Level1) +{ + TAG_LOGI(AAFwkTag::TEST, "begin"); + EXPECT_CALL(*callback_, OnExecuteDone(_, _, _)).Times(0); + + AppExecFwk::SkillExecuteResult result; + auto ret = mgr_->ExecuteSkillDone("nonexistent", 0, result, TARGET_BUNDLE); + EXPECT_EQ(ret, ERR_CODE_INVALID_ID); + TAG_LOGI(AAFwkTag::TEST, "end"); +} + +/** + * @tc.name: ExecuteSkillDone_0300 + * @tc.desc: ExecuteSkillDone with bundle mismatch returns ERR_INVALID_VALUE. + * @tc.type: FUNC + */ +HWTEST_F(SkillExecuteManagerTest, ExecuteSkillDone_0300, TestSize.Level1) +{ + TAG_LOGI(AAFwkTag::TEST, "begin"); + SeedRecord("req003", 3); + EXPECT_CALL(*callback_, OnExecuteDone(_, _, _)).Times(0); + + AppExecFwk::SkillExecuteResult result; + auto ret = mgr_->ExecuteSkillDone("req003", 0, result, "com.example.wrong"); + EXPECT_EQ(ret, ERR_INVALID_VALUE); + TAG_LOGI(AAFwkTag::TEST, "end"); +} + +/** + * @tc.name: ExecuteSkillDone_0400 + * @tc.desc: ExecuteSkillDone twice: second call fails because record was removed. + * @tc.type: FUNC + */ +HWTEST_F(SkillExecuteManagerTest, ExecuteSkillDone_0400, TestSize.Level1) +{ + TAG_LOGI(AAFwkTag::TEST, "begin"); + SeedRecord("req004", 4); + EXPECT_CALL(*callback_, OnExecuteDone(_, _, _)).Times(1); + + AppExecFwk::SkillExecuteResult result; + auto ret1 = mgr_->ExecuteSkillDone("req004", 0, result, TARGET_BUNDLE); + EXPECT_EQ(ret1, ERR_OK); + + auto ret2 = mgr_->ExecuteSkillDone("req004", 0, result, TARGET_BUNDLE); + EXPECT_EQ(ret2, ERR_CODE_INVALID_ID); + TAG_LOGI(AAFwkTag::TEST, "end"); +} + +/** + * @tc.name: ExecuteSkillDone_0500 + * @tc.desc: ExecuteSkillDone reentrant from callback must not deadlock. + * Pre-fix: callback ran under mutex_; re-entering deadlocked. + * Post-fix: callback runs outside mutex_; re-entry is safe. + * @tc.type: FUNC + */ +HWTEST_F(SkillExecuteManagerTest, ExecuteSkillDone_0500, TestSize.Level1) +{ + TAG_LOGI(AAFwkTag::TEST, "begin"); + SeedRecord("req005", 5); + + auto mgr = mgr_; + EXPECT_CALL(*callback_, OnExecuteDone(Eq("req005"), _, _)) + .Times(1) + .WillOnce(Invoke([mgr](const std::string &reqCode, int32_t, + const AppExecFwk::SkillExecuteResult &) { + AppExecFwk::SkillExecuteResult inner; + auto ret = mgr->ExecuteSkillDone(reqCode, 0, inner, TARGET_BUNDLE); + EXPECT_EQ(ret, ERR_CODE_INVALID_ID); + })); + + AppExecFwk::SkillExecuteResult result; + auto ret = mgr_->ExecuteSkillDone("req005", 0, result, TARGET_BUNDLE); + EXPECT_EQ(ret, ERR_OK); + TAG_LOGI(AAFwkTag::TEST, "end"); +} + +/** + * @tc.name: OnTimeout_0100 + * @tc.desc: OnTimeout on EXECUTING record flips state, fires callback once. + * @tc.type: FUNC + */ +HWTEST_F(SkillExecuteManagerTest, OnTimeout_0100, TestSize.Level1) +{ + TAG_LOGI(AAFwkTag::TEST, "begin"); + SeedRecord("req101", 101); + EXPECT_CALL(*callback_, OnExecuteDone(Eq("req101"), Eq(ERR_TIMED_OUT), _)).Times(1); + + mgr_->OnTimeout(101); + TAG_LOGI(AAFwkTag::TEST, "end"); +} + +/** + * @tc.name: OnTimeout_0200 + * @tc.desc: OnTimeout with unknown seq does nothing. + * @tc.type: FUNC + */ +HWTEST_F(SkillExecuteManagerTest, OnTimeout_0200, TestSize.Level1) +{ + TAG_LOGI(AAFwkTag::TEST, "begin"); + EXPECT_CALL(*callback_, OnExecuteDone(_, _, _)).Times(0); + + mgr_->OnTimeout(999); + TAG_LOGI(AAFwkTag::TEST, "end"); +} + +/** + * @tc.name: OnTimeout_0300 + * @tc.desc: OnTimeout twice on same seq: second call is a no-op. + * @tc.type: FUNC + */ +HWTEST_F(SkillExecuteManagerTest, OnTimeout_0300, TestSize.Level1) +{ + TAG_LOGI(AAFwkTag::TEST, "begin"); + SeedRecord("req103", 103); + EXPECT_CALL(*callback_, OnExecuteDone(_, _, _)).Times(1); + + mgr_->OnTimeout(103); + mgr_->OnTimeout(103); + TAG_LOGI(AAFwkTag::TEST, "end"); +} + +/** + * @tc.name: OnTimeout_0400 + * @tc.desc: OnTimeout after ExecuteSkillDone: record gone, no callback. + * @tc.type: FUNC + */ +HWTEST_F(SkillExecuteManagerTest, OnTimeout_0400, TestSize.Level1) +{ + TAG_LOGI(AAFwkTag::TEST, "begin"); + SeedRecord("req104", 104); + EXPECT_CALL(*callback_, OnExecuteDone(_, _, _)).Times(1); + + AppExecFwk::SkillExecuteResult result; + mgr_->ExecuteSkillDone("req104", 0, result, TARGET_BUNDLE); + mgr_->OnTimeout(104); + TAG_LOGI(AAFwkTag::TEST, "end"); +} + +/** + * @tc.name: OnTimeout_0500 + * @tc.desc: OnTimeout reentrant from callback must not deadlock. + * Pre-fix: callback ran under mutex_; re-entering deadlocked. + * Post-fix: callback runs outside mutex_; re-entry is safe. + * @tc.type: FUNC + */ +HWTEST_F(SkillExecuteManagerTest, OnTimeout_0500, TestSize.Level1) +{ + TAG_LOGI(AAFwkTag::TEST, "begin"); + SeedRecord("req105", 105); + + auto mgr = mgr_; + EXPECT_CALL(*callback_, OnExecuteDone(Eq("req105"), _, _)) + .Times(1) + .WillOnce(Invoke([mgr](const std::string &, int32_t, + const AppExecFwk::SkillExecuteResult &) { + mgr->OnTimeout(105); // seq already erased, no-op + })); + + mgr_->OnTimeout(105); + TAG_LOGI(AAFwkTag::TEST, "end"); +} + +} // namespace AAFwk +} // namespace OHOS