[Tizen] Avoid self-destruct during AnimatedImageVisual resource ready

Since AnimatedImageVisual's resource ready is not safe enough
for self-destruction case, we'd better use DiscardVisual
for every visual cases.

Change-Id: If316663699276696bfd0695b85ac53f1cbd2b8f2
Signed-off-by: Eunki, Hong <eunkiki.hong@samsung.com>

diff --git a/automated-tests/src/dali-toolkit/utc-Dali-ImageView.cpp b/automated-tests/src/dali-toolkit/utc-Dali-ImageView.cpp
index 4eb38b8..aa03285 100644 (file)
--- a/automated-tests/src/dali-toolkit/utc-Dali-ImageView.cpp
+++ b/automated-tests/src/dali-toolkit/utc-Dali-ImageView.cpp
@@ -3993,6 +3993,22 @@ void OnResourceReadySignal11(Control control)
   }
 }
 
+void OnResourceReadySignal12(Control control)
+{
+  gResourceReadySignalCounter++;
+
+  if(gImageView1)
+  {
+    gImageView1.Unparent();
+    gImageView1.Reset(); // Destroy visual
+  }
+  if(gImageView2)
+  {
+    gImageView2.Unparent();
+    gImageView2.Reset(); // Destroy visual
+  }
+}
+
 } // namespace
 
 int UtcDaliImageViewSetImageOnResourceReadySignal01(void)
@@ -4999,6 +5015,87 @@ int UtcDaliImageViewSetImageOnResourceReadySignal11(void)
   END_TEST;
 }
 
+int UtcDaliImageViewSetImageOnResourceReadySignal12(void)
+{
+  tet_infoline("Test ResourceReady Add AnimatedImageVisual with invalid url, and then Remove immediately.");
+
+  ToolkitTestApplication application;
+
+  gResourceReadySignalCounter = 0;
+
+  // Clear image view for clear test
+
+  if(gImageView1)
+  {
+    gImageView1.Reset();
+  }
+  if(gImageView2)
+  {
+    gImageView2.Reset();
+  }
+  if(gImageView3)
+  {
+    gImageView3.Reset();
+  }
+
+  try
+  {
+    gImageView1 = ImageView::New();
+    gImageView1.SetProperty(Toolkit::ImageView::Property::IMAGE, "invalid.gif");
+    gImageView1.ResourceReadySignal().Connect(&OnResourceReadySignal12);
+    application.GetScene().Add(gImageView1); // It will call resourceReady signal 1 time.
+
+    gImageView2 = ImageView::New();
+    gImageView2.SetProperty(Toolkit::ImageView::Property::IMAGE, "invalid.gif");
+    gImageView2.ResourceReadySignal().Connect(&OnResourceReadySignal12);
+    application.GetScene().Add(gImageView2); // It will call resourceReady signal 1 time.
+
+    tet_printf("ResourceReady called %d times\n", gResourceReadySignalCounter);
+
+    DALI_TEST_EQUALS(gResourceReadySignalCounter, 0, TEST_LOCATION);
+
+    application.SendNotification();
+    application.Render();
+
+    // Load gImageView1
+    DALI_TEST_EQUALS(Test::WaitForEventThreadTrigger(1), true, TEST_LOCATION);
+
+    tet_printf("ResourceReady called %d times\n", gResourceReadySignalCounter);
+
+    DALI_TEST_EQUALS(gResourceReadySignalCounter, 1, TEST_LOCATION);
+
+    DALI_TEST_CHECK(true);
+  }
+  catch(...)
+  {
+    // Exception should not happened
+    DALI_TEST_CHECK(false);
+  }
+
+  // Clear cache.
+  application.SendNotification();
+  application.Render();
+
+  gResourceReadySignalCounter = 0;
+
+  // Clear image view for clear test
+
+  if(gImageView1)
+  {
+    gImageView1.Reset();
+  }
+  if(gImageView2)
+  {
+    gImageView2.Reset();
+  }
+  if(gImageView3)
+  {
+    gImageView3.Reset();
+  }
+
+  END_TEST;
+}
+
 int UtcDaliImageViewUseSameUrlWithAnimatedImageVisual(void)
 {
   tet_infoline("Test multiple views with same image in animated image visual");
@@ -5042,6 +5139,9 @@ int UtcDaliImageViewNpatchImageCacheTest01(void)
     // Ensure remove npatch cache if required.
     application.SendNotification();
     application.Render();
+    application.RunIdles();
+    application.SendNotification();
+    application.Render();
 
     imageView[index] = ImageView::New(nPatchImageUrl);
     imageView[index].SetProperty(Actor::Property::SIZE, Vector2(100.0f, 200.0f));
@@ -5080,6 +5180,9 @@ int UtcDaliImageViewNpatchImageCacheTest01(void)
 
   application.SendNotification();
   application.Render();
+  application.RunIdles();
+  application.SendNotification();
+  application.Render();
 
   textureCallStack.Reset();
   // Let we use deference textures
@@ -5106,6 +5209,9 @@ int UtcDaliImageViewNpatchImageCacheTest01(void)
 
   application.SendNotification();
   application.Render();
+  application.RunIdles();
+  application.SendNotification();
+  application.Render();
 
   // Check memory leak
   DALI_TEST_EQUALS(textureCallStack.CountMethod("GenTextures"), textureCallStack.CountMethod("DeleteTextures"), TEST_LOCATION);

diff --git a/dali-toolkit/internal/controls/control/control-data-impl.cpp b/dali-toolkit/internal/controls/control/control-data-impl.cpp
index e67be8e..482ca3d 100644 (file)
--- a/dali-toolkit/internal/controls/control/control-data-impl.cpp
+++ b/dali-toolkit/internal/controls/control/control-data-impl.cpp
@@ -581,14 +581,22 @@ Control::Impl::Impl(Control& controlImpl)
 
 Control::Impl::~Impl()
 {
-  for(auto&& iter : mVisuals)
+  while(!mVisuals.Empty())
   {
-    StopObservingVisual(iter->visual);
+    auto iter = mVisuals.End() - 1u;
+    StopObservingVisual((*iter)->visual);
+
+    // Discard removed visual. It will be destroyed at next Idle time.
+    DiscardVisual(iter, mVisuals);
   }
 
-  for(auto&& iter : mRemoveVisuals)
+  while(!mRemoveVisuals.Empty())
   {
-    StopObservingVisual(iter->visual);
+    auto removalIter = mRemoveVisuals.End() - 1u;
+    StopObservingVisual((*removalIter)->visual);
+
+    // Discard removed visual. It will be destroyed at next Idle time.
+    DiscardVisual(removalIter, mRemoveVisuals);
   }
 
   // All gesture detectors will be destroyed so no need to disconnect.
@@ -2217,35 +2225,38 @@ void Control::Impl::UpdateVisualProperties(const std::vector<std::pair<Dali::Pro
 
 void Control::Impl::EmitResourceReadySignal()
 {
-  if(!mIsEmittingResourceReadySignal)
+  if(DALI_LIKELY(Stage::IsInstalled())) ///< Avoid resource ready callback during shutting down
   {
-    // Guard against calls to emit the signal during the callback
-    mIsEmittingResourceReadySignal = true;
+    if(!mIsEmittingResourceReadySignal)
+    {
+      // Guard against calls to emit the signal during the callback
+      mIsEmittingResourceReadySignal = true;
 
-    // If the signal handler changes visual, it may become ready during this call & therefore this method will
-    // get called again recursively. If so, mIdleCallbackRegistered is set below, and we act on it after that secondary
-    // invocation has completed by notifying in an Idle callback to prevent further recursion.
-    Dali::Toolkit::Control handle(mControlImpl.GetOwner());
-    mResourceReadySignal.Emit(handle);
+      // If the signal handler changes visual, it may become ready during this call & therefore this method will
+      // get called again recursively. If so, mIdleCallbackRegistered is set below, and we act on it after that secondary
+      // invocation has completed by notifying in an Idle callback to prevent further recursion.
+      Dali::Toolkit::Control handle(mControlImpl.GetOwner());
+      mResourceReadySignal.Emit(handle);
 
-    mIsEmittingResourceReadySignal = false;
-  }
-  else
-  {
-    if(!mIdleCallbackRegistered)
+      mIsEmittingResourceReadySignal = false;
+    }
+    else
     {
-      mIdleCallbackRegistered = true;
-
-      // Add idler to emit the signal again
-      if(!mIdleCallback)
+      if(!mIdleCallbackRegistered)
       {
-        // The callback manager takes the ownership of the callback object.
-        mIdleCallback = MakeCallback(this, &Control::Impl::OnIdleCallback);
-        if(DALI_UNLIKELY(!Adaptor::Get().AddIdle(mIdleCallback, true)))
+        mIdleCallbackRegistered = true;
+
+        // Add idler to emit the signal again
+        if(!mIdleCallback)
         {
-          DALI_LOG_ERROR("Fail to add idle callback for control resource ready. Skip this callback.\n");
-          mIdleCallback           = nullptr;
-          mIdleCallbackRegistered = false;
+          // The callback manager takes the ownership of the callback object.
+          mIdleCallback = MakeCallback(this, &Control::Impl::OnIdleCallback);
+          if(DALI_UNLIKELY(!Adaptor::Get().AddIdle(mIdleCallback, true)))
+          {
+            DALI_LOG_ERROR("Fail to add idle callback for control resource ready. Skip this callback.\n");
+            mIdleCallback           = nullptr;
+            mIdleCallbackRegistered = false;
+          }
         }
       }
     }

diff --git a/dali-toolkit/internal/controls/image-view/image-view-impl.cpp b/dali-toolkit/internal/controls/image-view/image-view-impl.cpp
index 7f11261..8a71d65 100644 (file)
--- a/dali-toolkit/internal/controls/image-view/image-view-impl.cpp
+++ b/dali-toolkit/internal/controls/image-view/image-view-impl.cpp
@@ -19,6 +19,7 @@
 #include "image-view-impl.h"
 
 // EXTERNAL INCLUDES
+#include <dali/devel-api/common/stage.h>
 #include <dali/devel-api/scripting/scripting.h>
 #include <dali/public-api/math/math-utils.h>
 #include <dali/public-api/object/type-registry-helper.h>
@@ -45,8 +46,8 @@ namespace
 {
 const Vector4 FULL_TEXTURE_RECT(0.f, 0.f, 1.f, 1.f);
 
-constexpr float FULL_OPACITY = 1.0f;
-constexpr float LOW_OPACITY  = 0.2f;
+constexpr float FULL_OPACITY            = 1.0f;
+constexpr float LOW_OPACITY             = 0.2f;
 constexpr float TRANSITION_EFFECT_SPEED = 0.3f;
 
 constexpr int PLACEHOLDER_DEPTH_INDEX = -2;
@@ -67,6 +68,20 @@ DALI_PROPERTY_REGISTRATION(Toolkit, ImageView, "enableTransitionEffect", BOOLEAN
 DALI_ANIMATABLE_PROPERTY_REGISTRATION_WITH_DEFAULT(Toolkit, ImageView, "pixelArea", Vector4(0.f, 0.f, 1.f, 1.f), PIXEL_AREA)
 DALI_TYPE_REGISTRATION_END()
 
+/**
+ * @brief Discard the given visual into VisualFactory. The visual will be destroyed at next idle time.
+ *
+ * @param[in,out] visual Visual to be discarded. It will be reset to an empty handle.
+ */
+void DiscardImageViewVisual(Dali::Toolkit::Visual::Base& visual)
+{
+  if(DALI_LIKELY(Dali::Stage::IsInstalled() && visual))
+  {
+    Dali::Toolkit::VisualFactory::Get().DiscardVisual(visual);
+  }
+  visual.Reset();
+}
+
 } // anonymous namespace
 
 using namespace Dali;
@@ -85,6 +100,9 @@ ImageView::ImageView(ControlBehaviour additionalBehaviour)
 
 ImageView::~ImageView()
 {
+  DiscardImageViewVisual(mVisual);
+  DiscardImageViewVisual(mPreviousVisual);
+  DiscardImageViewVisual(mPlaceholderVisual);
 }
 
 Toolkit::ImageView ImageView::New(ControlBehaviour additionalBehaviour)
@@ -133,6 +151,8 @@ void ImageView::SetImage(const Property::Map& map)
     // This previous visual will be deleted when transition effect is done.
     Internal::Control::Impl& controlDataImpl = Internal::Control::Impl::Get(*this);
     controlDataImpl.EnableReadyTransitionOverriden(mVisual, true);
+
+    DiscardImageViewVisual(mPreviousVisual);
     mPreviousVisual = mVisual;
   }
 
@@ -207,6 +227,8 @@ void ImageView::SetImage(const std::string& url, ImageDimensions size)
     // This previous visual will be deleted when transition effect is done.
     Internal::Control::Impl& controlDataImpl = Internal::Control::Impl::Get(*this);
     controlDataImpl.EnableReadyTransitionOverriden(mVisual, true);
+
+    DiscardImageViewVisual(mPreviousVisual);
     mPreviousVisual = mVisual;
   }
 
@@ -257,7 +279,8 @@ void ImageView::ClearImageVisual()
   // Clear cached properties
   mPropertyMap.Clear();
   mUrl.clear();
-  mVisual.Reset();
+
+  DiscardImageViewVisual(mVisual);
 
   // Unregister the exsiting visual
   DevelControl::UnregisterVisual(*this, Toolkit::ImageView::Property::IMAGE);
@@ -296,7 +319,7 @@ void ImageView::SetPlaceholderUrl(const std::string& url)
   mPlaceholderUrl = url;
   if(!url.empty())
   {
-    mPlaceholderVisual.Reset();
+    DiscardImageViewVisual(mPlaceholderVisual);
     CreatePlaceholderImage();
   }
   else
@@ -308,7 +331,7 @@ void ImageView::SetPlaceholderUrl(const std::string& url)
       DevelControl::UnregisterVisual(*this, Toolkit::ImageView::Property::PLACEHOLDER_IMAGE);
     }
 
-    mPlaceholderVisual.Reset();
+    DiscardImageViewVisual(mPlaceholderVisual);
     mPlaceholderUrl = url;
   }
 }
@@ -464,7 +487,13 @@ void ImageView::OnResourceReady(Toolkit::Control control)
   }
 
   // Visual ready so update visual attached to this ImageView, following call to RelayoutRequest will use this visual.
-  mVisual = DevelControl::GetVisual(*this, Toolkit::ImageView::Property::IMAGE);
+  auto currentVisual = DevelControl::GetVisual(*this, Toolkit::ImageView::Property::IMAGE);
+  if(mVisual != currentVisual)
+  {
+    // If the current visual is not the same as the previous holded visual, then we need to discard old one.
+    DiscardImageViewVisual(mVisual);
+  }
+  mVisual = currentVisual;
 
   // Applying FittingMode again if it is not working well on OnRelayout().
   if(mNeedLazyFittingMode)
@@ -647,7 +676,7 @@ void ImageView::CreatePlaceholderImage()
   else
   {
     DevelControl::UnregisterVisual(*this, Toolkit::ImageView::Property::PLACEHOLDER_IMAGE);
-    mPlaceholderVisual.Reset();
+    DiscardImageViewVisual(mPlaceholderVisual);
   }
 }
 
@@ -726,7 +755,7 @@ void ImageView::ClearTransitionAnimation()
     Internal::Control::Impl& controlDataImpl = Internal::Control::Impl::Get(*this);
     controlDataImpl.EnableReadyTransitionOverriden(mVisual, false);
     Toolkit::GetImplementation(mPreviousVisual).SetOffScene(self);
-    mPreviousVisual.Reset();
+    DiscardImageViewVisual(mPreviousVisual);
   }
 
   if(mTransitionAnimation)

diff --git a/dali-toolkit/internal/controls/text-controls/text-label-impl.cpp b/dali-toolkit/internal/controls/text-controls/text-label-impl.cpp
index a07471b..6e952ba 100644 (file)
--- a/dali-toolkit/internal/controls/text-controls/text-label-impl.cpp
+++ b/dali-toolkit/internal/controls/text-controls/text-label-impl.cpp
@@ -232,6 +232,20 @@ void ParseTextFitProperty(Text::ControllerPtr& controller, const Property::Map*
   }
 }
 
+/**
+ * @brief Discard the given visual into VisualFactory. The visual will be destroyed at next idle time.
+ *
+ * @param[in,out] visual Visual to be discarded. It will be reset to an empty handle.
+ */
+void DiscardTextLabelVisual(Dali::Toolkit::Visual::Base& visual)
+{
+  if(DALI_LIKELY(Dali::Stage::IsInstalled() && visual))
+  {
+    Dali::Toolkit::VisualFactory::Get().DiscardVisual(visual);
+  }
+  visual.Reset();
+}
+
 } // namespace
 
 Toolkit::TextLabel TextLabel::New(ControlBehaviour additionalBehaviour)
@@ -1403,6 +1417,7 @@ TextLabel::TextLabel(ControlBehaviour additionalBehaviour)
 
 TextLabel::~TextLabel()
 {
+  DiscardTextLabelVisual(mVisual);
 }
 
 Vector<Vector2> TextLabel::GetTextSize(const uint32_t startIndex, const uint32_t endIndex) const