[Libreoffice-commits] core.git: Branch 'libreoffice-6-1' - vcl/win

[Libreoffice Gerrit user]

 vcl/win/app/salinst.cxx     |   41 ++++++++++++++++++++++++++++++++---------
 vcl/win/window/salframe.cxx |    9 ++-------
 2 files changed, 34 insertions(+), 16 deletions(-)

New commits:
commit 3e392904c558d1f9fc91926511d148762813537e
Author:     Jan-Marek Glogowski <glogow at fbihome.de>
AuthorDate: Fri Aug 17 19:41:53 2018 +0200
Commit:     Caolán McNamara <caolanm at redhat.com>
CommitDate: Thu Aug 23 16:37:19 2018 +0200

    tdf#118786 WIN allow nested SendMessage calls
    
    This bug trips "assert( !pInst->mbNoYieldLock )".
    
    There is already a special case, introduced in commit 4baec725e0dc
    ("WIN run main thread redirects ignoring SolarMutex"), to prevent
    tripping the assert for a nested SendMessage call.
    
    So this implements a general solution for nested SendMessage calls.
    We just have to prevent yielding in a call from an other thread,
    as the sending thread still owns the SolarMutex.
    
    This way we can also drop the special handling in
    WinSalFrame::ReleaseFrameGraphicsDC.
    
    Change-Id: I7024b081b26f3545af12a3a3a038fe5e5671af3c
    Reviewed-on: https://gerrit.libreoffice.org/59275
    Tested-by: Jenkins
    Reviewed-by: Jan-Marek Glogowski <glogow at fbihome.de>
    (cherry picked from commit 35a254750392dcd738481f5d6e8719cee9fb41b3)
    Reviewed-on: https://gerrit.libreoffice.org/59318
    Reviewed-by: Caolán McNamara <caolanm at redhat.com>
    Tested-by: Caolán McNamara <caolanm at redhat.com>

diff --git a/vcl/win/app/salinst.cxx b/vcl/win/app/salinst.cxx
index c0bd18f85627..4e57b8406217 100644
--- a/vcl/win/app/salinst.cxx
+++ b/vcl/win/app/salinst.cxx
@@ -468,6 +468,11 @@ bool ImplSalYield( bool bWait, bool bHandleAllCurrentEvents )
     bool bWasMsg = false, bOneEvent = false, bWasTimeoutMsg = false;
     ImplSVData *const pSVData = ImplGetSVData();
     WinSalTimer* pTimer = static_cast<WinSalTimer*>( pSVData->maSchedCtx.mpSalTimer );
+    const bool bNoYieldLock = GetSalData()->mpInstance->mbNoYieldLock;
+
+    assert( !bNoYieldLock );
+    if ( bNoYieldLock )
+        return false;
 
     sal_uInt32 nCurTicks = 0;
     if ( bHandleAllCurrentEvents )
@@ -562,27 +567,45 @@ bool WinSalInstance::DoYield(bool bWait, bool bHandleAllCurrentEvents)
 
 #define CASE_NOYIELDLOCK( salmsg, function ) \
     case salmsg: \
-        assert( !pInst->mbNoYieldLock ); \
-        pInst->mbNoYieldLock = true; \
-        function; \
-        pInst->mbNoYieldLock = false; \
+        if (bIsOtherThreadMessage) \
+        { \
+            assert( !pInst->mbNoYieldLock ); \
+            pInst->mbNoYieldLock = true; \
+            function; \
+            pInst->mbNoYieldLock = false; \
+        } \
+        else \
+        { \
+            DBG_TESTSOLARMUTEX(); \
+            function; \
+        } \
         break;
 
 #define CASE_NOYIELDLOCK_RESULT( salmsg, function ) \
     case salmsg: \
-        assert( !pInst->mbNoYieldLock ); \
-        pInst->mbNoYieldLock = true; \
-        nRet = reinterpret_cast<LRESULT>( function ); \
-        pInst->mbNoYieldLock = false; \
+        if (bIsOtherThreadMessage) \
+        { \
+            assert( !pInst->mbNoYieldLock ); \
+            pInst->mbNoYieldLock = true; \
+            nRet = reinterpret_cast<LRESULT>( function ); \
+            pInst->mbNoYieldLock = false; \
+        } \
+        else \
+        { \
+            DBG_TESTSOLARMUTEX(); \
+            nRet = reinterpret_cast<LRESULT>( function ); \
+        } \
         break;
 
 LRESULT CALLBACK SalComWndProc( HWND, UINT nMsg, WPARAM wParam, LPARAM lParam, bool& rDef )
 {
+    const BOOL bIsOtherThreadMessage = InSendMessage();
     LRESULT nRet = 0;
     WinSalInstance *pInst = GetSalData()->mpInstance;
     WinSalTimer *const pTimer = static_cast<WinSalTimer*>( ImplGetSVData()->maSchedCtx.mpSalTimer );
 
-SAL_INFO("vcl.gdi.wndproc", "SalComWndProc(nMsg=" << nMsg << ", wParam=" << wParam << ", lParam=" << lParam << ")");
+    SAL_INFO("vcl.gdi.wndproc", "SalComWndProc(nMsg=" << nMsg << ", wParam=" << wParam
+                                << ", lParam=" << lParam << "); inSendMsg: " << bIsOtherThreadMessage);
 
     switch ( nMsg )
     {
diff --git a/vcl/win/window/salframe.cxx b/vcl/win/window/salframe.cxx
index 633e97c6670e..5691c37cfcc9 100644
--- a/vcl/win/window/salframe.cxx
+++ b/vcl/win/window/salframe.cxx
@@ -916,13 +916,8 @@ bool WinSalFrame::ReleaseFrameGraphicsDC( WinSalGraphics* pGraphics )
     if ( pGraphics->getDefPal() )
         SelectPalette( hDC, pGraphics->getDefPal(), TRUE );
     pGraphics->DeInitGraphics();
-    // we don't want to run the WinProc in the main thread directly
-    // so we don't hit the mbNoYieldLock assert
-    if ( !pSalData->mpInstance->IsMainThread() )
-        SendMessageW( pSalData->mpInstance->mhComWnd, SAL_MSG_RELEASEDC,
-            reinterpret_cast<WPARAM>(mhWnd), reinterpret_cast<LPARAM>(hDC) );
-    else
-        ReleaseDC( mhWnd, hDC );
+    SendMessageW( pSalData->mpInstance->mhComWnd, SAL_MSG_RELEASEDC,
+        reinterpret_cast<WPARAM>(mhWnd), reinterpret_cast<LPARAM>(hDC) );
     if ( pGraphics == mpThreadGraphics )
         pSalData->mnCacheDCInUse--;
     pGraphics->setHDC(nullptr);