diff --git a/Fwk/AppFwk/cafProjectDataModel/cafPdmUiCore/cafPdmUiEditorHandle.cpp b/Fwk/AppFwk/cafProjectDataModel/cafPdmUiCore/cafPdmUiEditorHandle.cpp index bad49eace1..cb5a0f1e30 100644 --- a/Fwk/AppFwk/cafProjectDataModel/cafPdmUiCore/cafPdmUiEditorHandle.cpp +++ b/Fwk/AppFwk/cafProjectDataModel/cafPdmUiCore/cafPdmUiEditorHandle.cpp @@ -36,7 +36,7 @@ #include "cafPdmUiEditorHandle.h" -#include "cafAssert.h" +#include namespace caf { @@ -62,11 +62,15 @@ PdmUiEditorHandle::~PdmUiEditorHandle() //-------------------------------------------------------------------------------------------------- void PdmUiEditorHandle::updateUi( const QString& uiConfigName ) { - CAF_ASSERT( !m_isConfiguringUi ); - m_isConfiguringUi = true; + // Skip a re-entrant update, the ongoing update will configure the editor + if ( m_isConfiguringUi ) return; + m_currentConfigName = uiConfigName; - this->configureAndUpdateUi( uiConfigName ); - m_isConfiguringUi = false; + { + // Reset the flag also if configureAndUpdateUi() throws, otherwise all later updates of this editor are blocked + QScopedValueRollback configuringUiGuard( m_isConfiguringUi, true ); + this->configureAndUpdateUi( uiConfigName ); + } // Allow derived classes to update additional state (e.g., validation) this->updateEditorState(); @@ -79,10 +83,14 @@ void PdmUiEditorHandle::updateUi( const QString& uiConfigName ) //-------------------------------------------------------------------------------------------------- void PdmUiEditorHandle::updateUi() { - CAF_ASSERT( !m_isConfiguringUi ); - m_isConfiguringUi = true; - this->configureAndUpdateUi( m_currentConfigName ); - m_isConfiguringUi = false; + // Skip a re-entrant update, the ongoing update will configure the editor + if ( m_isConfiguringUi ) return; + + { + // Reset the flag also if configureAndUpdateUi() throws, otherwise all later updates of this editor are blocked + QScopedValueRollback configuringUiGuard( m_isConfiguringUi, true ); + this->configureAndUpdateUi( m_currentConfigName ); + } // Allow derived classes to update additional state (e.g., validation) this->updateEditorState(); diff --git a/Fwk/AppFwk/cafUserInterface/cafUserInterface_UnitTests/CMakeLists.txt b/Fwk/AppFwk/cafUserInterface/cafUserInterface_UnitTests/CMakeLists.txt index 6a212e6e63..d1acc8e222 100644 --- a/Fwk/AppFwk/cafUserInterface/cafUserInterface_UnitTests/CMakeLists.txt +++ b/Fwk/AppFwk/cafUserInterface/cafUserInterface_UnitTests/CMakeLists.txt @@ -22,6 +22,7 @@ set(PROJECT_FILES cafPdmUiNumberFormatTest.cpp cafPdmUiCheckBoxAndTextEditorTest.cpp cafPdmUiPropertyViewDialogTest.cpp + cafPdmUiEditorHandleTest.cpp cafIconProviderTest.cpp cafProgressInfoTest.cpp gtest/gtest-all.cpp diff --git a/Fwk/AppFwk/cafUserInterface/cafUserInterface_UnitTests/cafPdmUiEditorHandleTest.cpp b/Fwk/AppFwk/cafUserInterface/cafUserInterface_UnitTests/cafPdmUiEditorHandleTest.cpp new file mode 100644 index 0000000000..e30b0f75ee --- /dev/null +++ b/Fwk/AppFwk/cafUserInterface/cafUserInterface_UnitTests/cafPdmUiEditorHandleTest.cpp @@ -0,0 +1,64 @@ + +#include "gtest/gtest.h" + +#include "cafPdmUiEditorHandle.h" + +#include + +using namespace caf; + +namespace +{ +class TestEditor : public caf::PdmUiEditorHandle +{ +public: + int configureCount = 0; + bool throwOnNextConfigure = false; + bool updateFromConfigureUi = false; + +protected: + void configureAndUpdateUi( const QString& uiConfigName ) override + { + configureCount++; + + if ( throwOnNextConfigure ) + { + throwOnNextConfigure = false; + throw std::runtime_error( "configureAndUpdateUi failed" ); + } + + if ( updateFromConfigureUi ) updateUi( uiConfigName ); + } +}; +} // namespace + +//-------------------------------------------------------------------------------------------------- +/// A re-entrant updateUi() from configureAndUpdateUi() used to abort on CAF_ASSERT( !m_isConfiguringUi ) +//-------------------------------------------------------------------------------------------------- +TEST( PdmUiEditorHandleTest, ReentrantUpdateIsSkipped ) +{ + TestEditor editor; + editor.updateFromConfigureUi = true; + + editor.updateUi( "" ); + EXPECT_EQ( 1, editor.configureCount ); + + editor.updateUi(); + EXPECT_EQ( 2, editor.configureCount ); +} + +//-------------------------------------------------------------------------------------------------- +/// An exception from configureAndUpdateUi() used to leave the editor in configuring state, and the next updateUi() +/// aborted on CAF_ASSERT( !m_isConfiguringUi ) +//-------------------------------------------------------------------------------------------------- +TEST( PdmUiEditorHandleTest, UpdateAfterExceptionInConfigure ) +{ + TestEditor editor; + editor.throwOnNextConfigure = true; + + EXPECT_THROW( editor.updateUi( "" ), std::runtime_error ); + EXPECT_EQ( 1, editor.configureCount ); + + editor.updateUi( "" ); + EXPECT_EQ( 2, editor.configureCount ); +}