[Libreoffice-commits] core.git: Branch 'libreoffice-5-3' - svx/source vcl/source

[Noel Grandin]

 svx/source/gallery2/galbrws1.cxx |   28 ++++++++++++++--------------
 svx/source/gallery2/galbrws1.hxx |    3 ++-
 vcl/source/window/dialog.cxx     |    2 +-
 3 files changed, 17 insertions(+), 16 deletions(-)

New commits:
commit 4bde081f7ebaf3d6e489a50b287f82ccb14de224
Author: Noel Grandin <noel.grandin at collabora.co.uk>
Date:   Mon Jan 16 13:53:00 2017 +0200

    tdf#105017 Crash when click a "New Theme..." in Gallery on detached SideBar
    
    caused by VclPtr fixes which meant instead of the dialog staying in an
    always-zero ref-count state, it went up to a ref-count of 1, then down
    to zero, which triggered a delete before the dialog had finished
    displaying.
    
    And revert my fix in commit 0c1cd678f71e519f5a4e623d93442e046485005a
    which just worked around the underlying problem
    
    Change-Id: Icb35535dd06a2d1db1016d00d106760847d87430
    (cherry picked from commit cd9d8315141c3070f43e145ed4ee390e837eb73f)
    Reviewed-on: https://gerrit.libreoffice.org/33209
    Tested-by: Jenkins <ci at libreoffice.org>
    Reviewed-by: Noel Grandin <noel.grandin at collabora.co.uk>

diff --git a/svx/source/gallery2/galbrws1.cxx b/svx/source/gallery2/galbrws1.cxx
index dbd09be..3967fc4 100644
--- a/svx/source/gallery2/galbrws1.cxx
+++ b/svx/source/gallery2/galbrws1.cxx
@@ -148,6 +148,7 @@ GalleryBrowser1::~GalleryBrowser1()
 void GalleryBrowser1::dispose()
 {
     EndListening( *mpGallery );
+    mpThemePropertiesDialog.clear();
     mpThemes.disposeAndClear();
     delete mpExchangeData;
     mpExchangeData = nullptr;
@@ -265,24 +266,24 @@ void GalleryBrowser1::ImplGalleryThemeProperties( const OUString & rThemeName, b
 
     SvxAbstractDialogFactory* pFact = SvxAbstractDialogFactory::Create();
     assert(pFact && "Got no AbstractDialogFactory!");
-    VclPtr<VclAbstractDialog2> pThemeProps = pFact->CreateGalleryThemePropertiesDialog( mpExchangeData, mpThemePropsDlgItemSet );
-    assert(pThemeProps && "Got no GalleryThemePropertiesDialog!");
+    mpThemePropertiesDialog = pFact->CreateGalleryThemePropertiesDialog( mpExchangeData, mpThemePropsDlgItemSet );
+    assert(mpThemePropertiesDialog && "Got no GalleryThemePropertiesDialog!");
 
     if ( bCreateNew )
     {
-        pThemeProps->StartExecuteModal(
+        mpThemePropertiesDialog->StartExecuteModal(
             LINK( this, GalleryBrowser1, EndNewThemePropertiesDlgHdl ) );
     }
     else
     {
-        pThemeProps->StartExecuteModal(
+        mpThemePropertiesDialog->StartExecuteModal(
             LINK( this, GalleryBrowser1, EndThemePropertiesDlgHdl ) );
     }
 }
 
-void GalleryBrowser1::ImplEndGalleryThemeProperties(Dialog* pDialog, bool bCreateNew)
+void GalleryBrowser1::ImplEndGalleryThemeProperties(Dialog* /*pDialog*/, bool bCreateNew)
 {
-    long nRet = pDialog->GetResult();
+    long nRet = mpThemePropertiesDialog->GetResult();
 
     if( nRet == RET_OK )
     {
@@ -319,23 +320,22 @@ void GalleryBrowser1::ImplEndGalleryThemeProperties(Dialog* pDialog, bool bCreat
     }
 
     // destroy mpThemeProps asynchronously
-    Application::PostUserEvent( LINK( this, GalleryBrowser1, DestroyThemePropertiesDlgHdl ), pDialog, true );
+    Application::PostUserEvent( LINK( this, GalleryBrowser1, DestroyThemePropertiesDlgHdl ), nullptr, true );
 }
 
-IMPL_LINK( GalleryBrowser1, EndNewThemePropertiesDlgHdl, Dialog&, rDialog, void )
+IMPL_LINK( GalleryBrowser1, EndNewThemePropertiesDlgHdl, Dialog&, /*rDialog*/, void )
 {
-    ImplEndGalleryThemeProperties(&rDialog, true);
+    ImplEndGalleryThemeProperties(nullptr, true);
 }
 
-IMPL_LINK( GalleryBrowser1, EndThemePropertiesDlgHdl, Dialog&, rDialog, void )
+IMPL_LINK( GalleryBrowser1, EndThemePropertiesDlgHdl, Dialog&, /*rDialog*/, void )
 {
-    ImplEndGalleryThemeProperties(&rDialog, false);
+    ImplEndGalleryThemeProperties(nullptr, false);
 }
 
-IMPL_LINK( GalleryBrowser1, DestroyThemePropertiesDlgHdl, void*, p, void )
+IMPL_LINK( GalleryBrowser1, DestroyThemePropertiesDlgHdl, void*, /*p*/, void )
 {
-    VclPtr<VclAbstractDialog2> pDialog = static_cast<VclAbstractDialog2*>(p);
-    pDialog.disposeAndClear();
+    mpThemePropertiesDialog.disposeAndClear();
     delete mpThemePropsDlgItemSet;
     mpThemePropsDlgItemSet = nullptr;
 }
diff --git a/svx/source/gallery2/galbrws1.hxx b/svx/source/gallery2/galbrws1.hxx
index 999d8a6..47118f3 100644
--- a/svx/source/gallery2/galbrws1.hxx
+++ b/svx/source/gallery2/galbrws1.hxx
@@ -77,8 +77,9 @@ class GalleryBrowser1 : public Control, public SfxListener
 
 private:
 
-    VclPtr<GalleryButton>   maNewTheme;
+    VclPtr<GalleryButton>        maNewTheme;
     VclPtr<GalleryThemeListBox>  mpThemes;
+    VclPtr<VclAbstractDialog2>   mpThemePropertiesDialog; // to keep it alive during execution
     Gallery*                mpGallery;
     ExchangeData*           mpExchangeData;
     SfxItemSet*             mpThemePropsDlgItemSet;
diff --git a/vcl/source/window/dialog.cxx b/vcl/source/window/dialog.cxx
index c6fcae3..eef4c5a 100644
--- a/vcl/source/window/dialog.cxx
+++ b/vcl/source/window/dialog.cxx
@@ -1039,7 +1039,7 @@ void Dialog::SetModalInputMode( bool bModal )
         // Disable the prev Modal Dialog, because our dialog must close at first,
         // before the other dialog can be closed (because the other dialog
         // is on stack since our dialog returns)
-        if ( mpPrevExecuteDlg && !mpPrevExecuteDlg->isDisposed() && !mpPrevExecuteDlg->IsWindowOrChild( this, true ) )
+        if ( mpPrevExecuteDlg && !mpPrevExecuteDlg->IsWindowOrChild( this, true ) )
             mpPrevExecuteDlg->EnableInput( false, this );
 
         // determine next overlap dialog parent