From: Adeel Kazmi Date: Fri, 12 Oct 2018 14:25:07 +0000 (+0100) Subject: (FrameCallback) Ensure the callback is removed if the implementation is deleted X-Git-Tag: dali_1.3.46~2^2 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;ds=sidebyside;h=refs%2Fchanges%2F21%2F191221%2F3;p=platform%2Fcore%2Fuifw%2Fdali-core.git (FrameCallback) Ensure the callback is removed if the implementation is deleted Also remove unnecessary mRootNode in FrameCallbackProcessor Change-Id: Ia7941a77ba5deed4c5abe10870af9935d8b5112d --- diff --git a/automated-tests/src/dali/utc-Dali-FrameCallbackInterface.cpp b/automated-tests/src/dali/utc-Dali-FrameCallbackInterface.cpp index 9c4f31b..858e20b 100644 --- a/automated-tests/src/dali/utc-Dali-FrameCallbackInterface.cpp +++ b/automated-tests/src/dali/utc-Dali-FrameCallbackInterface.cpp @@ -804,6 +804,8 @@ int UtcDaliFrameCallbackMultipleCallbacks(void) int UtcDaliFrameCallbackActorDestroyed(void) { + // Test to ensure that the frame-callback behaves gracefully if the connected root-actor is destroyed + TestApplication application; Stage stage = Stage::GetCurrent(); @@ -847,3 +849,33 @@ int UtcDaliFrameCallbackActorDestroyed(void) END_TEST; } + +int UtcDaliFrameCallbackDestroyedBeforeRemoving(void) +{ + // Ensure there's no segmentation fault if the callback is deleted without being removed + + TestApplication application; + Stage stage = Stage::GetCurrent(); + + Actor actor = Actor::New(); + stage.Add( actor ); + + { + FrameCallbackBasic frameCallback; + DevelStage::AddFrameCallback( stage, frameCallback, actor ); + + application.SendNotification(); + application.Render(); + + DALI_TEST_EQUALS( frameCallback.mCalled, true, TEST_LOCATION ); + frameCallback.Reset(); + } + + // frameCallback has now been destroyed but not removed + + application.SendNotification(); + application.Render(); + DALI_TEST_CHECK( true ); // If it runs to here then there's no segmentation fault + + END_TEST; +} diff --git a/dali/devel-api/CMakeLists.txt b/dali/devel-api/CMakeLists.txt index d0663c3..5572eeb 100644 --- a/dali/devel-api/CMakeLists.txt +++ b/dali/devel-api/CMakeLists.txt @@ -19,6 +19,8 @@ SET( SOURCES ${SOURCES} ${CMAKE_CURRENT_SOURCE_DIR}/threading/conditional-wait.cpp ${CMAKE_CURRENT_SOURCE_DIR}/threading/mutex.cpp ${CMAKE_CURRENT_SOURCE_DIR}/threading/thread.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/update/frame-callback-interface.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/update/update-proxy.cpp PARENT_SCOPE ) @@ -63,4 +65,7 @@ SET( DEVEL_API_HEADERS ${CMAKE_CURRENT_SOURCE_DIR}/threading/mutex.h ${CMAKE_CURRENT_SOURCE_DIR}/threading/thread.h + ${CMAKE_CURRENT_SOURCE_DIR}/update/frame-callback-interface.h + ${CMAKE_CURRENT_SOURCE_DIR}/update/update-proxy.h + PARENT_SCOPE ) diff --git a/dali/devel-api/common/stage-devel.h b/dali/devel-api/common/stage-devel.h index 3735f04..05b7390 100644 --- a/dali/devel-api/common/stage-devel.h +++ b/dali/devel-api/common/stage-devel.h @@ -85,6 +85,8 @@ DALI_IMPORT_API void AddFrameCallback( Dali::Stage stage, FrameCallbackInterface * * @param[in] stage The stage to clear the FrameCallbackInterface on * @param[in] frameCallback The FrameCallbackInterface implementation to remove + * + * @note If the callback implementation has already been removed, then this is a no-op. */ DALI_IMPORT_API void RemoveFrameCallback( Dali::Stage stage, FrameCallbackInterface& frameCallback ); diff --git a/dali/devel-api/file.list b/dali/devel-api/file.list index 0487c33..831620d 100644 --- a/dali/devel-api/file.list +++ b/dali/devel-api/file.list @@ -19,6 +19,7 @@ devel_api_src_files = \ $(devel_api_src_dir)/threading/conditional-wait.cpp \ $(devel_api_src_dir)/threading/mutex.cpp \ $(devel_api_src_dir)/threading/thread.cpp \ + $(devel_api_src_dir)/update/frame-callback-interface.cpp \ $(devel_api_src_dir)/update/update-proxy.cpp # Add devel header files here DALi internal developer files used by Adaptor & Toolkit diff --git a/dali/devel-api/update/frame-callback-interface.cpp b/dali/devel-api/update/frame-callback-interface.cpp new file mode 100644 index 0000000..bf4de08 --- /dev/null +++ b/dali/devel-api/update/frame-callback-interface.cpp @@ -0,0 +1,41 @@ +/* + * Copyright (c) 2018 Samsung Electronics 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. + * + */ + +// CLASS HEADER +#include + +// INTERNAL INCLUDES +#include +#include + +namespace Dali +{ + +FrameCallbackInterface::~FrameCallbackInterface() +{ + if( Internal::Stage::IsInstalled() ) + { + Internal::StagePtr stage = Internal::Stage::GetCurrent(); + if( stage ) + { + // This will be a no-op if the callback has already been removed + stage->RemoveFrameCallback( *this ); + } + } +} + +} // namespace Dali diff --git a/dali/devel-api/update/frame-callback-interface.h b/dali/devel-api/update/frame-callback-interface.h index d92c7be..e6ad997 100644 --- a/dali/devel-api/update/frame-callback-interface.h +++ b/dali/devel-api/update/frame-callback-interface.h @@ -64,7 +64,7 @@ protected: /** * @brief Protected virtual destructor. */ - virtual ~FrameCallbackInterface() {} + virtual ~FrameCallbackInterface(); }; } // namespace Dali diff --git a/dali/internal/update/manager/frame-callback-processor.cpp b/dali/internal/update/manager/frame-callback-processor.cpp index 9d16232..f837ce2 100644 --- a/dali/internal/update/manager/frame-callback-processor.cpp +++ b/dali/internal/update/manager/frame-callback-processor.cpp @@ -62,10 +62,9 @@ private: } // unnamed namespace -FrameCallbackProcessor::FrameCallbackProcessor( TransformManager& transformManager, Node& rootNode ) +FrameCallbackProcessor::FrameCallbackProcessor( TransformManager& transformManager ) : mFrameCallbacks(), mTransformManager( transformManager ), - mRootNode( rootNode ), mNodeHierarchyChanged( true ) { } diff --git a/dali/internal/update/manager/frame-callback-processor.h b/dali/internal/update/manager/frame-callback-processor.h index 52e0ff6..440227d 100644 --- a/dali/internal/update/manager/frame-callback-processor.h +++ b/dali/internal/update/manager/frame-callback-processor.h @@ -51,7 +51,7 @@ public: /** * Construct a new FrameCallbackProcessor. */ - FrameCallbackProcessor( TransformManager& transformManager, Node& rootNode ); + FrameCallbackProcessor( TransformManager& transformManager ); /** * Non-virtual Destructor. @@ -146,7 +146,6 @@ private: std::vector< FrameCallbackInfo > mFrameCallbacks; ///< A container of all the frame-callbacks & accompanying update-proxies. TransformManager& mTransformManager; - Node& mRootNode; bool mNodeHierarchyChanged; ///< Set to true if the node hierarchy changes }; diff --git a/dali/internal/update/manager/update-manager.cpp b/dali/internal/update/manager/update-manager.cpp index 8a3bfb9..84722e4 100644 --- a/dali/internal/update/manager/update-manager.cpp +++ b/dali/internal/update/manager/update-manager.cpp @@ -265,7 +265,7 @@ struct UpdateManager::Impl { if( ! frameCallbackProcessor ) { - frameCallbackProcessor = new FrameCallbackProcessor( transformManager, *root ); + frameCallbackProcessor = new FrameCallbackProcessor( transformManager ); } return *frameCallbackProcessor; }