fpicker/source/office/commonpicker.cxx | 149 +++++++++++++-------------------- fpicker/source/office/commonpicker.hxx | 2 2 files changed, 61 insertions(+), 90 deletions(-)
New commits: commit fc1129760f95401328973cd1d0aa78d836aa0e54 Author: Michael Weghorn <[email protected]> AuthorDate: Fri Dec 5 22:29:02 2025 +0100 Commit: Michael Weghorn <[email protected]> CommitDate: Sat Dec 6 10:06:13 2025 +0100 fpicker: Flatten OCommonPicker::createPicker Return early if m_xDlg is non-null, which allows dropping an additional indentation level for the rest of the method. Change-Id: I7a2cbe08e932d296a6512a9b6113942ea5e3d448 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/195127 Reviewed-by: Michael Weghorn <[email protected]> Tested-by: Jenkins diff --git a/fpicker/source/office/commonpicker.cxx b/fpicker/source/office/commonpicker.cxx index f515c0991dab..19ca730132bf 100644 --- a/fpicker/source/office/commonpicker.cxx +++ b/fpicker/source/office/commonpicker.cxx @@ -197,47 +197,47 @@ namespace svt void OCommonPicker::createPicker() { - if ( !m_xDlg ) - { - m_xDlg = implCreateDialog(Application::GetFrameWeld(m_xDialogParent)); - assert(m_xDlg); + if (m_xDlg) + return; - weld::Dialog* pDlg = m_xDlg->getDialog(); + m_xDlg = implCreateDialog(Application::GetFrameWeld(m_xDialogParent)); + assert(m_xDlg); - ::svt::OControlAccess aAccess(m_xDlg.get(), m_xDlg->GetView()); - // synchronize the help id of the dialog without help URL property - if ( !m_sHelpURL.isEmpty() ) - { // somebody already set the help URL while we had no dialog yet - aAccess.setHelpURL(pDlg, m_sHelpURL); - } - else - { - m_sHelpURL = aAccess.getHelpURL(pDlg); - } + weld::Dialog* pDlg = m_xDlg->getDialog(); - m_xWindow = pDlg->GetXWindow(); + ::svt::OControlAccess aAccess(m_xDlg.get(), m_xDlg->GetView()); + // synchronize the help id of the dialog without help URL property + if ( !m_sHelpURL.isEmpty() ) + { // somebody already set the help URL while we had no dialog yet + aAccess.setHelpURL(pDlg, m_sHelpURL); + } + else + { + m_sHelpURL = aAccess.getHelpURL(pDlg); + } - // add as event listener to the window - OSL_ENSURE( m_xWindow.is(), "OCommonPicker::createFileDialog: invalid window component!" ); - if ( m_xWindow.is() ) - { - m_xWindowListenerAdapter = new OWeakEventListenerAdapter( this, m_xWindow ); - // the adapter will add itself as listener, and forward notifications - } + m_xWindow = pDlg->GetXWindow(); - VclPtr<vcl::Window> xVclDialog(VCLUnoHelper::GetWindow(m_xWindow)); - if (xVclDialog) // this block is quite possibly unnecessary by now - { - // _and_ add as event listener to the parent - in case the parent is destroyed - // before we are disposed, our disposal would access dead VCL windows then... - m_xDialogParent = VCLUnoHelper::GetInterface(xVclDialog->GetParent()); - OSL_ENSURE(m_xDialogParent.is() || !xVclDialog->GetParent(), "OCommonPicker::createFileDialog: invalid window component (the parent this time)!"); - } - if ( m_xDialogParent.is() ) - { - m_xParentListenerAdapter = new OWeakEventListenerAdapter( this, m_xDialogParent ); - // the adapter will add itself as listener, and forward notifications - } + // add as event listener to the window + OSL_ENSURE( m_xWindow.is(), "OCommonPicker::createFileDialog: invalid window component!" ); + if ( m_xWindow.is() ) + { + m_xWindowListenerAdapter = new OWeakEventListenerAdapter( this, m_xWindow ); + // the adapter will add itself as listener, and forward notifications + } + + VclPtr<vcl::Window> xVclDialog(VCLUnoHelper::GetWindow(m_xWindow)); + if (xVclDialog) // this block is quite possibly unnecessary by now + { + // _and_ add as event listener to the parent - in case the parent is destroyed + // before we are disposed, our disposal would access dead VCL windows then... + m_xDialogParent = VCLUnoHelper::GetInterface(xVclDialog->GetParent()); + OSL_ENSURE(m_xDialogParent.is() || !xVclDialog->GetParent(), "OCommonPicker::createFileDialog: invalid window component (the parent this time)!"); + } + if ( m_xDialogParent.is() ) + { + m_xParentListenerAdapter = new OWeakEventListenerAdapter( this, m_xDialogParent ); + // the adapter will add itself as listener, and forward notifications } } commit b433d6d3e37186a61b0fc9ac056406865e5026b3 Author: Michael Weghorn <[email protected]> AuthorDate: Fri Dec 5 22:26:26 2025 +0100 Commit: Michael Weghorn <[email protected]> CommitDate: Sat Dec 6 10:06:05 2025 +0100 fpicker: Drop always true OCommonPicker::createPicker ret val As is more obvious since Change-Id: Iefd760eb6cc059905d4e02be72e2334a933d2d72 Author: Michael Weghorn <[email protected]> Date: Fri Dec 5 22:17:40 2025 +0100 fpicker: Switch warning to assert , the return value of that method is always true and therefore redundant. Drop it and all of the conditional code paths that would only be relevant if the return value had ever been true. Change-Id: Ib184db935629badfb3cf70b0c037feb9b462fbc0 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/195126 Reviewed-by: Michael Weghorn <[email protected]> Tested-by: Jenkins diff --git a/fpicker/source/office/commonpicker.cxx b/fpicker/source/office/commonpicker.cxx index 1f02f2147cdd..f515c0991dab 100644 --- a/fpicker/source/office/commonpicker.cxx +++ b/fpicker/source/office/commonpicker.cxx @@ -101,12 +101,10 @@ namespace svt void OCommonPicker::prepareDialog() { - if(createPicker()) - { - // set the title - if ( !m_aTitle.isEmpty() ) - m_xDlg->set_title(m_aTitle); - } + createPicker(); + // set the title + if (!m_aTitle.isEmpty()) + m_xDlg->set_title(m_aTitle); } @@ -197,7 +195,7 @@ namespace svt } } - bool OCommonPicker::createPicker() + void OCommonPicker::createPicker() { if ( !m_xDlg ) { @@ -241,8 +239,6 @@ namespace svt // the adapter will add itself as listener, and forward notifications } } - - return nullptr != m_xDlg; } // XControlAccess functions @@ -251,11 +247,9 @@ namespace svt checkAlive(); SolarMutexGuard aGuard; - if ( createPicker() ) - { - ::svt::OControlAccess aAccess( m_xDlg.get(), m_xDlg->GetView() ); - aAccess.setControlProperty( aControlName, aControlProperty, aValue ); - } + createPicker(); + ::svt::OControlAccess aAccess(m_xDlg.get(), m_xDlg->GetView()); + aAccess.setControlProperty(aControlName, aControlProperty, aValue); } Any SAL_CALL OCommonPicker::getControlProperty( const OUString& aControlName, const OUString& aControlProperty ) @@ -263,13 +257,9 @@ namespace svt checkAlive(); SolarMutexGuard aGuard; - if ( createPicker() ) - { - ::svt::OControlAccess aAccess( m_xDlg.get(), m_xDlg->GetView() ); - return aAccess.getControlProperty( aControlName, aControlProperty ); - } - - return Any(); + createPicker(); + ::svt::OControlAccess aAccess(m_xDlg.get(), m_xDlg->GetView()); + return aAccess.getControlProperty(aControlName, aControlProperty); } // XControlInformation functions @@ -278,13 +268,9 @@ namespace svt checkAlive(); SolarMutexGuard aGuard; - if ( createPicker() ) - { - ::svt::OControlAccess aAccess( m_xDlg.get(), m_xDlg->GetView() ); - return aAccess.getSupportedControls( ); - } - - return Sequence< OUString >(); + createPicker(); + ::svt::OControlAccess aAccess(m_xDlg.get(), m_xDlg->GetView()); + return aAccess.getSupportedControls(); } sal_Bool SAL_CALL OCommonPicker::isControlSupported( const OUString& aControlName ) @@ -292,12 +278,8 @@ namespace svt checkAlive(); SolarMutexGuard aGuard; - if ( createPicker() ) - { - return svt::OControlAccess::isControlSupported( aControlName ); - } - - return false; + createPicker(); + return svt::OControlAccess::isControlSupported(aControlName); } Sequence< OUString > SAL_CALL OCommonPicker::getSupportedControlProperties( const OUString& aControlName ) @@ -305,13 +287,9 @@ namespace svt checkAlive(); SolarMutexGuard aGuard; - if ( createPicker() ) - { - ::svt::OControlAccess aAccess( m_xDlg.get(), m_xDlg->GetView() ); - return aAccess.getSupportedControlProperties( aControlName ); - } - - return Sequence< OUString >(); + createPicker(); + ::svt::OControlAccess aAccess(m_xDlg.get(), m_xDlg->GetView()); + return aAccess.getSupportedControlProperties(aControlName); } sal_Bool SAL_CALL OCommonPicker::isControlPropertySupported( const OUString& aControlName, const OUString& aControlProperty ) @@ -319,13 +297,9 @@ namespace svt checkAlive(); SolarMutexGuard aGuard; - if ( createPicker() ) - { - ::svt::OControlAccess aAccess( m_xDlg.get(), m_xDlg->GetView() ); - return aAccess.isControlPropertySupported( aControlName, aControlProperty ); - } - - return false; + createPicker(); + ::svt::OControlAccess aAccess(m_xDlg.get(), m_xDlg->GetView()); + return aAccess.isControlPropertySupported(aControlName, aControlProperty); } diff --git a/fpicker/source/office/commonpicker.hxx b/fpicker/source/office/commonpicker.hxx index d03f6efb1680..682701842df8 100644 --- a/fpicker/source/office/commonpicker.hxx +++ b/fpicker/source/office/commonpicker.hxx @@ -169,7 +169,7 @@ namespace svt void prepareDialog(); protected: - bool createPicker(); + void createPicker(); /** handle a single argument from the XInitialization::initialize method commit 95dc756975f4ce58ca83b54cf2b10b2592066d9e Author: Michael Weghorn <[email protected]> AuthorDate: Fri Dec 5 22:17:40 2025 +0100 Commit: Michael Weghorn <[email protected]> CommitDate: Sat Dec 6 10:05:57 2025 +0100 fpicker: Switch warning to assert All OCommonPicker::implCreateDialog overrides actually return a dialog, so assert on that instead of just warning. This also means that the following check is redundant, so drop it. Change-Id: Iefd760eb6cc059905d4e02be72e2334a933d2d72 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/195125 Tested-by: Jenkins Reviewed-by: Michael Weghorn <[email protected]> diff --git a/fpicker/source/office/commonpicker.cxx b/fpicker/source/office/commonpicker.cxx index b57112d56d2c..1f02f2147cdd 100644 --- a/fpicker/source/office/commonpicker.cxx +++ b/fpicker/source/office/commonpicker.cxx @@ -202,46 +202,43 @@ namespace svt if ( !m_xDlg ) { m_xDlg = implCreateDialog(Application::GetFrameWeld(m_xDialogParent)); - SAL_WARN_IF( !m_xDlg, "fpicker.office", "OCommonPicker::createPicker: invalid dialog returned!" ); + assert(m_xDlg); - if ( m_xDlg ) + weld::Dialog* pDlg = m_xDlg->getDialog(); + + ::svt::OControlAccess aAccess(m_xDlg.get(), m_xDlg->GetView()); + // synchronize the help id of the dialog without help URL property + if ( !m_sHelpURL.isEmpty() ) + { // somebody already set the help URL while we had no dialog yet + aAccess.setHelpURL(pDlg, m_sHelpURL); + } + else + { + m_sHelpURL = aAccess.getHelpURL(pDlg); + } + + m_xWindow = pDlg->GetXWindow(); + + // add as event listener to the window + OSL_ENSURE( m_xWindow.is(), "OCommonPicker::createFileDialog: invalid window component!" ); + if ( m_xWindow.is() ) + { + m_xWindowListenerAdapter = new OWeakEventListenerAdapter( this, m_xWindow ); + // the adapter will add itself as listener, and forward notifications + } + + VclPtr<vcl::Window> xVclDialog(VCLUnoHelper::GetWindow(m_xWindow)); + if (xVclDialog) // this block is quite possibly unnecessary by now + { + // _and_ add as event listener to the parent - in case the parent is destroyed + // before we are disposed, our disposal would access dead VCL windows then... + m_xDialogParent = VCLUnoHelper::GetInterface(xVclDialog->GetParent()); + OSL_ENSURE(m_xDialogParent.is() || !xVclDialog->GetParent(), "OCommonPicker::createFileDialog: invalid window component (the parent this time)!"); + } + if ( m_xDialogParent.is() ) { - weld::Dialog* pDlg = m_xDlg->getDialog(); - - ::svt::OControlAccess aAccess(m_xDlg.get(), m_xDlg->GetView()); - // synchronize the help id of the dialog without help URL property - if ( !m_sHelpURL.isEmpty() ) - { // somebody already set the help URL while we had no dialog yet - aAccess.setHelpURL(pDlg, m_sHelpURL); - } - else - { - m_sHelpURL = aAccess.getHelpURL(pDlg); - } - - m_xWindow = pDlg->GetXWindow(); - - // add as event listener to the window - OSL_ENSURE( m_xWindow.is(), "OCommonPicker::createFileDialog: invalid window component!" ); - if ( m_xWindow.is() ) - { - m_xWindowListenerAdapter = new OWeakEventListenerAdapter( this, m_xWindow ); - // the adapter will add itself as listener, and forward notifications - } - - VclPtr<vcl::Window> xVclDialog(VCLUnoHelper::GetWindow(m_xWindow)); - if (xVclDialog) // this block is quite possibly unnecessary by now - { - // _and_ add as event listener to the parent - in case the parent is destroyed - // before we are disposed, our disposal would access dead VCL windows then... - m_xDialogParent = VCLUnoHelper::GetInterface(xVclDialog->GetParent()); - OSL_ENSURE(m_xDialogParent.is() || !xVclDialog->GetParent(), "OCommonPicker::createFileDialog: invalid window component (the parent this time)!"); - } - if ( m_xDialogParent.is() ) - { - m_xParentListenerAdapter = new OWeakEventListenerAdapter( this, m_xDialogParent ); - // the adapter will add itself as listener, and forward notifications - } + m_xParentListenerAdapter = new OWeakEventListenerAdapter( this, m_xDialogParent ); + // the adapter will add itself as listener, and forward notifications } }
