(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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
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 <dali/devel-api/update/frame-callback-interface.h>
+
+// INTERNAL INCLUDES
+#include <dali/devel-api/update/update-proxy.h>
+#include <dali/internal/event/common/stage-impl.h>
+
+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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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;
   }