From 007d0a26178ff4d44d87030c14b8d50f7048da66 Mon Sep 17 00:00:00 2001 From: "minho.sun" Date: Thu, 27 Jul 2017 12:02:43 +0900 Subject: [PATCH] Fixed focus manager crash Fixed focus manager crash. When pre focused actor which is in history stack is removed, we cannot know about that via base-object pointer. So, contain WeakHandle of Actor instead of base-object pointer in std::vector. Change-Id: Id523747099013319c72ae402ceea12fea9cf2ccb Signed-off-by: minho.sun --- .../dali-toolkit/utc-Dali-KeyboardFocusManager.cpp | 51 ++++++++++++++-------- .../focus-manager/keyboard-focus-manager-impl.cpp | 51 +++++++++++++--------- .../focus-manager/keyboard-focus-manager-impl.h | 5 ++- 3 files changed, 68 insertions(+), 39 deletions(-) diff --git a/automated-tests/src/dali-toolkit/utc-Dali-KeyboardFocusManager.cpp b/automated-tests/src/dali-toolkit/utc-Dali-KeyboardFocusManager.cpp index 8957dd6..6083bc2 100644 --- a/automated-tests/src/dali-toolkit/utc-Dali-KeyboardFocusManager.cpp +++ b/automated-tests/src/dali-toolkit/utc-Dali-KeyboardFocusManager.cpp @@ -971,15 +971,6 @@ int UtcDaliKeyboardFocusManagerMoveFocusBackward(void) KeyboardFocusManager manager = KeyboardFocusManager::Get(); DALI_TEST_CHECK(manager); - // Make history stack full - for(int i = 0 ; i < 31 ; i ++) - { - Actor actor = Actor::New(); - actor.SetKeyboardFocusable(true); - Stage::GetCurrent().Add(actor); - manager.SetCurrentFocusActor(actor); - } - // Create the first actor and add it to the stage Actor first = Actor::New(); first.SetKeyboardFocusable(true); @@ -990,11 +981,16 @@ int UtcDaliKeyboardFocusManagerMoveFocusBackward(void) second.SetKeyboardFocusable(true); Stage::GetCurrent().Add(second); - // Create the second actor and add it to the stage + // Create the third actor and add it to the stage Actor third = Actor::New(); third.SetKeyboardFocusable(true); Stage::GetCurrent().Add(third); + // Create the fourth actor and add it to the stage + Actor fourth = Actor::New(); + fourth.SetKeyboardFocusable(true); + Stage::GetCurrent().Add(fourth); + // Check that the focus is set on the second actor DALI_TEST_CHECK(manager.SetCurrentFocusActor(first) == true); DALI_TEST_CHECK(manager.GetCurrentFocusActor() == first); @@ -1007,24 +1003,45 @@ int UtcDaliKeyboardFocusManagerMoveFocusBackward(void) DALI_TEST_CHECK(manager.SetCurrentFocusActor(third) == true); DALI_TEST_CHECK(manager.GetCurrentFocusActor() == third); + // Check that the focus is set on the third actor + DALI_TEST_CHECK(manager.SetCurrentFocusActor(fourth) == true); + DALI_TEST_CHECK(manager.GetCurrentFocusActor() == fourth); + // Move the focus backward manager.MoveFocusBackward(); - // Check that it current focused actor is second actor - DALI_TEST_CHECK(manager.GetCurrentFocusActor() == second); - - // Check that the focus is set on the third actor - DALI_TEST_CHECK(manager.SetCurrentFocusActor(third) == true); + // Check that it current focused actor is third actor DALI_TEST_CHECK(manager.GetCurrentFocusActor() == third); // Remove the second actor on stage second.Unparent(); + // Reset the first actor + first.Unparent(); + first.Reset(); + // Move the focus backward manager.MoveFocusBackward(); - // Check that it current focused actor is first actor - DALI_TEST_CHECK(manager.GetCurrentFocusActor() == first); + // Check that it current focused actor is third actor + DALI_TEST_CHECK(manager.GetCurrentFocusActor() == third); + + // Make history stack full + for(int i = 0 ; i < 31 ; i ++) + { + Actor actor = Actor::New(); + actor.SetKeyboardFocusable(true); + Stage::GetCurrent().Add(actor); + manager.SetCurrentFocusActor(actor); + } + + for(int i = 0 ; i < 31 ; i ++) + { + manager.MoveFocusBackward(); + } + + // Check that it current focused actor is not second actor + DALI_TEST_CHECK(manager.GetCurrentFocusActor() != second); END_TEST; } diff --git a/dali-toolkit/internal/focus-manager/keyboard-focus-manager-impl.cpp b/dali-toolkit/internal/focus-manager/keyboard-focus-manager-impl.cpp index 5225c46..c3ebe62 100644 --- a/dali-toolkit/internal/focus-manager/keyboard-focus-manager-impl.cpp +++ b/dali-toolkit/internal/focus-manager/keyboard-focus-manager-impl.cpp @@ -208,13 +208,13 @@ bool KeyboardFocusManager::DoSetCurrentFocusActor( Actor actor ) } // Push Current Focused Actor to FocusHistory - mFocusHistory.PushBack( &actor.GetBaseObject() ); + mFocusHistory.push_back( actor ); // Delete first element before add new element when Stack is full. - if( mFocusHistory.Count() > MAX_HISTORY_AMOUNT ) + if( mFocusHistory.size() > MAX_HISTORY_AMOUNT ) { - FocusStackIterator beginPos = mFocusHistory.Begin(); - mFocusHistory.Erase( beginPos ); + FocusStackIterator beginPos = mFocusHistory.begin(); + mFocusHistory.erase( beginPos ); } DALI_LOG_INFO( gLogFilter, Debug::General, "[%s:%d] SUCCEED\n", __FUNCTION__, __LINE__); @@ -249,28 +249,39 @@ Actor KeyboardFocusManager::GetCurrentFocusGroup() void KeyboardFocusManager::MoveFocusBackward() { // Find Pre Focused Actor when the list size is more than 1 - if( mFocusHistory.Count() > 1 ) + if( mFocusHistory.size() > 1 ) { // Delete current focused actor in history - FocusStackIterator endPos = mFocusHistory.End(); - endPos = mFocusHistory.Erase( --endPos ); + mFocusHistory.pop_back(); - // If pre-focused actors are not on stage, remove them in stack - while( !Dali::Actor::DownCast(BaseHandle(mFocusHistory[ mFocusHistory.Count() - 1 ])).OnStage() ) + // If pre-focused actors are not on stage or deleted, remove them in stack + while( mFocusHistory.size() > 0 ) { - endPos = mFocusHistory.Erase( --endPos ); - } - - // Get pre focused actor - BaseObject* object = mFocusHistory[ mFocusHistory.Count() - 1 ]; - BaseHandle handle( object ); - Actor preFocusedActor = Dali::Actor::DownCast( handle ); + // Get pre focused actor + Actor target = mFocusHistory[ mFocusHistory.size() -1 ].GetHandle(); - // Delete pre focused actor in history because it will pushed again by SetCurrentFocusActor() - mFocusHistory.Erase( --endPos ); + // Impl of Actor is not null + if( target && target.OnStage() ) + { + // Delete pre focused actor in history because it will pushed again by SetCurrentFocusActor() + mFocusHistory.pop_back(); + SetCurrentFocusActor( target ); + break; + } + else + { + // Target is empty handle or off stage. Erase from queue + mFocusHistory.pop_back(); + } + } - SetCurrentFocusActor( preFocusedActor ); - } + // if there is no actor which can get focus, then push current focus actor in stack again + if( mFocusHistory.size() == 0 ) + { + Actor currentFocusedActor = GetCurrentFocusActor(); + mFocusHistory.push_back( currentFocusedActor ); + } + } } bool KeyboardFocusManager::IsLayoutControl(Actor actor) const diff --git a/dali-toolkit/internal/focus-manager/keyboard-focus-manager-impl.h b/dali-toolkit/internal/focus-manager/keyboard-focus-manager-impl.h index 7283b1e..049ce1f 100644 --- a/dali-toolkit/internal/focus-manager/keyboard-focus-manager-impl.h +++ b/dali-toolkit/internal/focus-manager/keyboard-focus-manager-impl.h @@ -21,6 +21,7 @@ // EXTERNAL INCLUDES #include #include +#include // INTERNAL INCLUDES #include @@ -161,8 +162,8 @@ protected: private: - typedef Dali::Vector< Dali::BaseObject* > FocusStack; ///< Define Dali::Vector< Dali::BaseObject* > as FocusStack to contain focus history - typedef FocusStack::Iterator FocusStackIterator; ///< Define FocusStack::Iterator as FocusStackIterator to navigate FocusStack + typedef std::vector< WeakHandle< Actor > > FocusStack; ///< Define Dali::Vector< Dali::BaseObject* > as FocusStack to contain focus history + typedef FocusStack::iterator FocusStackIterator; ///< Define FocusStack::Iterator as FocusStackIterator to navigate FocusStack /** * Get configuration from StyleManager. -- 2.7.4