WIN simplify system timer / LO event handling

This removes a level of indirection for the timer callback handling.
It also ensures we just have a single timeout message queued.

Also drops the 16bit MAX duration limit, as CreateTimerQueueTimer
uses a DWORD for the DueTime parameter, which is always UINT32.

CreateTimerQueueTimer already sets the period to 0, which makes it
a one-shot timer, but there is also the WT_EXECUTEONLYONCE, which
enforces the Period parameter to be 0.

Change-Id: I549142394334bdc098f053b42f222b23cf4fcecd

6 files changed, 74 insertions, 125 deletions

diff --git a/vcl/inc/win/saldata.hxx b/vcl/inc/win/saldata.hxx
index 100681050e51..367319743c94 100644
--- a/vcl/inc/win/saldata.hxx
+++ b/vcl/inc/win/saldata.hxx
@@ -84,10 +84,6 @@ public:
     long*                   mpDitherDiff;           // Dither mapping table
     BYTE*                   mpDitherLow;            // Dither mapping table
     BYTE*                   mpDitherHigh;           // Dither mapping table
-    sal_uLong               mnTimerMS;              // Current Time (in MS) of the Timer
-    sal_uLong               mnTimerOrgMS;           // Current Original Time (in MS)
-    DWORD                   mnNextTimerTime;
-    DWORD                   mnLastEventTime;
     HANDLE                  mnTimerId;              ///< Windows timer id
     HHOOK                   mhSalObjMsgHook;        // hook to get interesting msg for SalObject
     HWND                    mhWantLeaveMsg;         // window handle, that want a MOUSELEAVE message
@@ -231,9 +227,9 @@ int ImplSalWICompareAscii( const wchar_t* pStr1, const char* pStr2 );
 // wParam == hWnd; lParam == 0
 #define SAL_MSG_RELEASEDC           (WM_USER+121)
 // wParam == newParentHwnd; lParam == oldHwnd; lResult == newhWnd
-#define SAL_MSG_RECREATEHWND         (WM_USER+122)
+#define SAL_MSG_RECREATEHWND        (WM_USER+122)
 // wParam == newParentHwnd; lParam == oldHwnd; lResult == newhWnd
-#define SAL_MSG_RECREATECHILDHWND    (WM_USER+123)
+#define SAL_MSG_RECREATECHILDHWND   (WM_USER+123)
 // wParam == 0; lParam == HWND;
 #define SAL_MSG_DESTROYHWND         (WM_USER+124)
 
@@ -243,6 +239,7 @@ int ImplSalWICompareAscii( const wchar_t* pStr1, const char* pStr2 );
 #define SAL_MSG_MOUSELEAVE          (WM_USER+131)
 // NULL-Message, should not be processed
 #define SAL_MSG_DUMMY               (WM_USER+132)
+// Used for SETFOCUS and KILLFOCUS
 // wParam == 0; lParam == 0
 #define SAL_MSG_POSTFOCUS           (WM_USER+133)
 // wParam == wParam; lParam == lParam
@@ -269,11 +266,10 @@ int ImplSalWICompareAscii( const wchar_t* pStr1, const char* pStr2 );
 #define SAL_MSG_SETINPUTCONTEXT     (WM_USER+144)
 // wParam == nFlags; lParam == 0
 #define SAL_MSG_ENDEXTTEXTINPUT     (WM_USER+145)
-// POSTTIMER-Message; wparam = 0, lParam == time
-#define SAL_MSG_POSTTIMER        (WM_USER+161)
 
 // SysChild-ToTop; wParam = 0; lParam = 0
 #define SALOBJ_MSG_TOTOP            (WM_USER+160)
+// Used for SETFOCUS and KILLFOCUS
 // POSTFOCUS-Message; wParam == bFocus; lParam == 0
 #define SALOBJ_MSG_POSTFOCUS        (WM_USER+161)
 
diff --git a/vcl/inc/win/salinst.h b/vcl/inc/win/salinst.h
index 359bc275e463..03ba573c523b 100644
--- a/vcl/inc/win/salinst.h
+++ b/vcl/inc/win/salinst.h
@@ -83,8 +83,6 @@ public:
 SalFrame* ImplSalCreateFrame( WinSalInstance* pInst, HWND hWndParent, SalFrameStyleFlags nSalFrameStyle );
 SalObject* ImplSalCreateObject( WinSalInstance* pInst, WinSalFrame* pParent );
 HWND ImplSalReCreateHWND( HWND hWndParent, HWND oldhWnd, bool bAsChild );
-void ImplSalStartTimer( sal_uIntPtr nMS, bool bMutex = false );
-void ImplSalStopTimer();
 
 #endif // INCLUDED_VCL_INC_WIN_SALINST_H
 
diff --git a/vcl/inc/win/saltimer.h b/vcl/inc/win/saltimer.h
index 996b2b5137da..084a25745b87 100644
--- a/vcl/inc/win/saltimer.h
+++ b/vcl/inc/win/saltimer.h
@@ -32,6 +32,9 @@ public:
     virtual void Stop() override;
 };
 
+void ImplSalStartTimer( sal_uIntPtr nMS );
+void ImplSalStopTimer();
+
 #endif
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/vcl/win/app/salinst.cxx b/vcl/win/app/salinst.cxx
index 6fbe7bca7021..5ef585891e1b 100644
--- a/vcl/win/app/salinst.cxx
+++ b/vcl/win/app/salinst.cxx
@@ -316,10 +316,6 @@ SalData::SalData()
     mpDitherDiff = nullptr;     // Dither mapping table
     mpDitherLow = nullptr;      // Dither mapping table
     mpDitherHigh = nullptr;     // Dither mapping table
-    mnTimerMS = 0;              // Current Time (in MS) of the Timer
-    mnTimerOrgMS = 0;           // Current Original Time (in MS)
-    mnNextTimerTime = 0;
-    mnLastEventTime = 0;
     mnTimerId = nullptr;        // windows timer id
     mhSalObjMsgHook = nullptr;  // hook to get interesting msg for SalObject
     mhWantLeaveMsg = nullptr;   // window handle, that want a MOUSELEAVE message
@@ -664,12 +660,18 @@ LRESULT CALLBACK SalComWndProc( HWND, UINT nMsg, WPARAM wParam, LPARAM lParam, i
             rDef = FALSE;
             break;
         case SAL_MSG_STARTTIMER:
-            ImplSalStartTimer( (sal_uLong) lParam );
+        {
+            sal_uLong nTime = GetTickCount();
+            if ( nTime < (sal_uLong) lParam )
+                nTime = (sal_uLong) lParam - nTime;
+            else
+                nTime = 0;
+            ImplSalStartTimer( nTime );
             rDef = FALSE;
             break;
+        }
         case SAL_MSG_STOPTIMER:
             ImplSalStopTimer();
-            rDef = FALSE;
             break;
         case SAL_MSG_CREATEFRAME:
             nRet = reinterpret_cast<LRESULT>(ImplSalCreateFrame( GetSalData()->mpFirstInstance, reinterpret_cast<HWND>(lParam), (SalFrameStyleFlags)wParam ));
@@ -715,23 +717,12 @@ LRESULT CALLBACK SalComWndProc( HWND, UINT nMsg, WPARAM wParam, LPARAM lParam, i
             ReleaseDC( reinterpret_cast<HWND>(wParam), reinterpret_cast<HDC>(lParam) );
             rDef = FALSE;
             break;
-        case SAL_MSG_POSTTIMER:
-            EmitTimerCallback();
-            break;
         case SAL_MSG_TIMER_CALLBACK:
-            EmitTimerCallback();
             MSG aMsg;
-            while (PeekMessageW(&aMsg, nullptr, SAL_MSG_TIMER_CALLBACK, SAL_MSG_TIMER_CALLBACK, PM_REMOVE))
-            {
-                // nothing; just remove all the SAL_MSG_TIMER_CALLBACKs that
-                // accumulated in the queue during the EmitTimerCallback(),
-                // otherwise it happens with short timeouts and long callbacks
-                // that no other events will ever be processed, as the queue
-                // is full of SAL_MSG_TIMER_CALLBACKs.
-                // It is impossible to limit the amount of them being emitted
-                // in the first place, as they are emitted asynchronously, but
-                // here we are already fully synchronized.
-            }
+            while ( PeekMessageW(&aMsg, nullptr, SAL_MSG_TIMER_CALLBACK,
+                                 SAL_MSG_TIMER_CALLBACK, PM_REMOVE) )
+                assert( "Multiple timer messages in queue" );
+            EmitTimerCallback();
             break;
     }
 
@@ -837,24 +828,6 @@ bool WinSalInstance::AnyInput( VclInputFlags nType )
     return false;
 }
 
-void SalTimer::Start( sal_uLong nMS )
-{
-    // to switch to Main-Thread
-    SalData* pSalData = GetSalData();
-    if ( pSalData->mpFirstInstance )
-    {
-        if ( pSalData->mnAppThreadId != GetCurrentThreadId() )
-        {
-            BOOL const ret = PostMessageW(pSalData->mpFirstInstance->mhComWnd, SAL_MSG_STARTTIMER, 0, (LPARAM)nMS);
-            SAL_WARN_IF(0 == ret, "vcl", "ERROR: PostMessage() failed!");
-        }
-        else
-            SendMessageW( pSalData->mpFirstInstance->mhComWnd, SAL_MSG_STARTTIMER, 0, (LPARAM)nMS );
-    }
-    else
-        ImplSalStartTimer( nMS );
-}
-
 SalFrame* WinSalInstance::CreateChildFrame( SystemParentData* pSystemParentData, SalFrameStyleFlags nSalFrameStyle )
 {
     // to switch to Main-Thread
diff --git a/vcl/win/app/saltimer.cxx b/vcl/win/app/saltimer.cxx
index 25412dcf6186..af379700c5bd 100644
--- a/vcl/win/app/saltimer.cxx
+++ b/vcl/win/app/saltimer.cxx
@@ -22,9 +22,6 @@
 #include <win/saltimer.h>
 #include <win/salinst.h>
 
-// maximum period
-#define MAX_SYSPERIOD     65533
-
 void CALLBACK SalTimerProc(PVOID pParameter, BOOLEAN bTimerOrWaitFired);
 
 // See http://msdn.microsoft.com/en-us/library/windows/desktop/ms687003%28v=vs.85%29.aspx
@@ -37,64 +34,68 @@ void CALLBACK SalTimerProc(PVOID pParameter, BOOLEAN bTimerOrWaitFired);
 void ImplSalStopTimer()
 {
     SalData *const pSalData = GetSalData();
+    assert( !pSalData->mpFirstInstance || pSalData->mnAppThreadId == GetCurrentThreadId() );
+
     HANDLE hTimer = pSalData->mnTimerId;
     if (hTimer)
     {
-        pSalData->mnTimerId = nullptr; // reset so it doesn't restart
+        pSalData->mnTimerId = nullptr;
         DeleteTimerQueueTimer(nullptr, hTimer, INVALID_HANDLE_VALUE);
-        pSalData->mnNextTimerTime = 0;
     }
+
+    // remove all pending SAL_MSG_TIMER_CALLBACK messages
+    // we always have to do this, since ImplSalStartTimer with 0ms just queues
+    // a new SAL_MSG_TIMER_CALLBACK message
     MSG aMsg;
-    // this needs to run on the main thread
-    while (PeekMessageW(&aMsg, nullptr, SAL_MSG_TIMER_CALLBACK, SAL_MSG_TIMER_CALLBACK, PM_REMOVE))
-    {
-        // just remove all the SAL_MSG_TIMER_CALLBACKs
-        // when the application end, this SAL_MSG_TIMER_CALLBACK start the timer again
-        // and then crashed in "SalTimerProc" when the object "SalData" was deleted
-    }
+    int nMsgCount = 0;
+    while ( PeekMessageW(&aMsg, nullptr, SAL_MSG_TIMER_CALLBACK,
+                         SAL_MSG_TIMER_CALLBACK, PM_REMOVE) )
+        nMsgCount++;
+    assert( nMsgCount <= 1 );
 }
 
-void ImplSalStartTimer( sal_uLong nMS, bool bMutex )
+void ImplSalStartTimer( sal_uLong nMS )
 {
     SalData* pSalData = GetSalData();
+    assert( !pSalData->mpFirstInstance || pSalData->mnAppThreadId == GetCurrentThreadId() );
 
-    // Remember the time of the timer
-    pSalData->mnTimerMS = nMS;
-    if (!bMutex)
-        pSalData->mnTimerOrgMS = nMS;
+    // DueTime parameter is a DWORD, which is always an unsigned 32bit
+    if (nMS > SAL_MAX_UINT32)
+        nMS = SAL_MAX_UINT32;
 
-    // duration has to fit into Window's sal_uInt16
-    if (nMS > MAX_SYSPERIOD)
-        nMS = MAX_SYSPERIOD;
+    // cannot change a one-shot timer, so delete it and create a new one
+    ImplSalStopTimer();
 
-    // cannot change a one-shot timer, so delete it and create new one
-    if (pSalData->mnTimerId)
+    // directly post a timer callback message for instant timers / idles
+    if ( 0 == nMS )
     {
-        DeleteTimerQueueTimer(nullptr, pSalData->mnTimerId, INVALID_HANDLE_VALUE);
-        pSalData->mnTimerId = nullptr;
+        BOOL const ret = PostMessageW(pSalData->mpFirstInstance->mhComWnd,
+                                      SAL_MSG_TIMER_CALLBACK, 0, 0);
+        SAL_WARN_IF(0 == ret, "vcl", "ERROR: PostMessage() failed!");
+    }
+    else
+    {
+        // probably WT_EXECUTEONLYONCE is not needed, but it enforces Period
+        // to be 0 and should not hurt; also see
+        // https://www.microsoft.com/msj/0499/pooling/pooling.aspx
+        CreateTimerQueueTimer(&pSalData->mnTimerId, nullptr, SalTimerProc, nullptr,
+                              nMS, 0, WT_EXECUTEINTIMERTHREAD | WT_EXECUTEONLYONCE);
     }
-    CreateTimerQueueTimer(&pSalData->mnTimerId, nullptr, SalTimerProc, nullptr, nMS, 0, WT_EXECUTEINTIMERTHREAD);
-
-    pSalData->mnNextTimerTime = pSalData->mnLastEventTime + nMS;
 }
 
 WinSalTimer::~WinSalTimer()
 {
+    Stop();
 }
 
 void WinSalTimer::Start( sal_uLong nMS )
 {
-    // switch to main thread
     SalData* pSalData = GetSalData();
-    if ( pSalData->mpFirstInstance )
+    if ( pSalData->mpFirstInstance && pSalData->mnAppThreadId != GetCurrentThreadId() )
     {
-        if ( pSalData->mnAppThreadId != GetCurrentThreadId() )
-        {
-            BOOL const ret = PostMessageW(pSalData->mpFirstInstance->mhComWnd, SAL_MSG_STARTTIMER, 0, (LPARAM)nMS);
-            SAL_WARN_IF(0 == ret, "vcl", "ERROR: PostMessage() failed!");
-        }
-        else
-            SendMessageW( pSalData->mpFirstInstance->mhComWnd, SAL_MSG_STARTTIMER, 0, (LPARAM)nMS );
+        BOOL const ret = PostMessageW(pSalData->mpFirstInstance->mhComWnd,
+            SAL_MSG_STARTTIMER, 0, (LPARAM)GetTickCount() + nMS);
+        SAL_WARN_IF(0 == ret, "vcl", "ERROR: PostMessage() failed!");
     }
     else
         ImplSalStartTimer( nMS );
@@ -103,9 +104,14 @@ void WinSalTimer::Start( sal_uLong nMS )
 void WinSalTimer::Stop()
 {
     SalData* pSalData = GetSalData();
-
-    assert(pSalData->mpFirstInstance);
-    SendMessageW(pSalData->mpFirstInstance->mhComWnd, SAL_MSG_STOPTIMER, 0, 0);
+    if ( pSalData->mpFirstInstance && pSalData->mnAppThreadId != GetCurrentThreadId() )
+    {
+        BOOL const ret = PostMessageW(pSalData->mpFirstInstance->mhComWnd,
+            SAL_MSG_STOPTIMER, 0, 0);
+        SAL_WARN_IF(0 == ret, "vcl", "ERROR: PostMessage() failed!");
+    }
+    else
+        ImplSalStopTimer();
 }
 
 /** This gets invoked from a Timer Queue thread.
@@ -122,7 +128,8 @@ void CALLBACK SalTimerProc(PVOID, BOOLEAN)
         // always post message when the timer fires, we will remove the ones
         // that happened during execution of the callback later directly from
         // the message queue
-        BOOL const ret = PostMessageW(pSalData->mpFirstInstance->mhComWnd, SAL_MSG_TIMER_CALLBACK, 0, 0);
+        BOOL const ret = PostMessageW(pSalData->mpFirstInstance->mhComWnd,
+                                      SAL_MSG_TIMER_CALLBACK, 0, 0);
 #if OSL_DEBUG_LEVEL > 0
         if (0 == ret) // SEH prevents using SAL_WARN here?
             fputs("ERROR: PostMessage() failed!", stderr);
@@ -140,31 +147,23 @@ call then happens when the main thread gets SAL_MSG_TIMER_CALLBACK.
 */
 void EmitTimerCallback()
 {
-    SalData* pSalData = GetSalData();
-    ImplSVData* pSVData = ImplGetSVData();
-
     // Test for MouseLeave
     SalTestMouseLeave();
 
+    ImplSVData *pSVData = ImplGetSVData();
+    if ( ! pSVData->maSchedCtx.mpSalTimer )
+        return;
+
     // Try to acquire the mutex. If we don't get the mutex then we
     // try this a short time later again.
-    if (pSVData->maSchedCtx.mpSalTimer && ImplSalYieldMutexTryToAcquire())
+    if (ImplSalYieldMutexTryToAcquire())
     {
         pSVData->maSchedCtx.mpSalTimer->CallCallback();
 
         ImplSalYieldMutexRelease();
-
-        // Run the timer again if it was started before, and also
-        // Run the timer in the correct time, if we started this
-        // with a small timeout, because we didn't get the mutex
-        // - but not if mnTimerId is 0, which is set by ImplSalStopTimer()
-        if (pSalData->mnTimerId)
-            ImplSalStartTimer(pSalData->mnTimerOrgMS);
     }
     else
-    {
-        ImplSalStartTimer(10, true);
-    }
+        ImplSalStartTimer( 10 );
 }
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/vcl/win/window/salframe.cxx b/vcl/win/window/salframe.cxx
index cee3ef1b48de..b31249db81d2 100644
--- a/vcl/win/window/salframe.cxx
+++ b/vcl/win/window/salframe.cxx
@@ -5480,8 +5480,6 @@ LRESULT CALLBACK SalFrameWndProc( HWND hWnd, UINT nMsg, WPARAM wParam, LPARAM lP
         return 0;
     }
 
-    bool bCheckTimers = false;
-
     switch( nMsg )
     {
         case WM_MOUSEMOVE:
@@ -5618,11 +5616,11 @@ LRESULT CALLBACK SalFrameWndProc( HWND hWnd, UINT nMsg, WPARAM wParam, LPARAM lP
             rDef = FALSE;
             break;
         case WM_PAINT:
-            bCheckTimers = ImplHandlePaintMsg( hWnd );
+            ImplHandlePaintMsg( hWnd );
             rDef = FALSE;
             break;
         case SAL_MSG_POSTPAINT:
-            bCheckTimers = ImplHandlePostPaintMsg( hWnd, reinterpret_cast<RECT*>(wParam) );
+            ImplHandlePostPaintMsg( hWnd, reinterpret_cast<RECT*>(wParam) );
             rDef = FALSE;
             break;
 
@@ -5843,24 +5841,6 @@ LRESULT CALLBACK SalFrameWndProc( HWND hWnd, UINT nMsg, WPARAM wParam, LPARAM lP
             break;
     }
 
-    if( bCheckTimers )
-    {
-        SalData* pSalData = GetSalData();
-        if( pSalData->mnNextTimerTime )
-        {
-            DWORD nCurTime = GetTickCount();
-            if( pSalData->mnNextTimerTime < nCurTime )
-            {
-                MSG aMsg;
-                if( ! PeekMessageW( &aMsg, nullptr, WM_PAINT, WM_PAINT, PM_NOREMOVE | PM_NOYIELD ) )
-                {
-                    BOOL const ret = PostMessageW(pSalData->mpFirstInstance->mhComWnd, SAL_MSG_POSTTIMER, 0, nCurTime);
-                    SAL_WARN_IF(0 == ret, "vcl", "ERROR: PostMessage() failed!");
-                }
-            }
-        }
-    }
-
     return nRet;
 }