Fix scheduled Task priority change handling

If a task is still in the scheduler priority queue and its
priority is changed, it won't be moved into the correct queue.
We have to track the priority of the scheduled task, so we can
warn the developer to fix the code and actually handle re-start
correctly.

Since we don't want to traverse the whole Scheduler queues on
priority change (which sometimes get very long) to remove the
wrong data item, we'll just invalidate it, if a priority change
is detected.

This also reverts commit d24b264c ("vcl opengl: avoid task
priority warning on cursor blink"), which tried to avoid the
warning, which was just half right and independent of the broken
priority change handling.

LO doesn't change priorities of scheduled tasks normally, so
that bug didn't turn out to have much impact, I guess.

Change-Id: I6e46b518a7c3532047c619c013bd8597f73ed7a6
Reviewed-on: https://gerrit.libreoffice.org/69249
Tested-by: Jenkins
Reviewed-by: Jan-Marek Glogowski <glogow@fbihome.de>

include/vcl/task.hxx

@@ -77,7 +77,7 @@ public:
     virtual ~Task() COVERITY_NOEXCEPT_FALSE;
     Task& operator=( const Task& rTask );
 
-    inline void     SetPriority(TaskPriority ePriority);
+    void            SetPriority(TaskPriority ePriority);
     TaskPriority    GetPriority() const { return mePriority; }
 
     void            SetDebugName( const sal_Char *pDebugName ) { mpDebugName = pDebugName; }
@@ -101,13 +101,6 @@ public:
     bool            IsStatic() const { return mbStatic; }
 };
 
-inline void Task::SetPriority(TaskPriority ePriority)
-{
-    SAL_WARN_IF(mpSchedulerData, "vcl.schedule",
-                "Priority will just change after next schedule!");
-    mePriority = ePriority;
-}
-
 #endif // INCLUDED_VCL_TASK_HXX
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */

vcl/inc/schedulerimpl.hxx

@@ -33,6 +33,7 @@ struct ImplSchedulerData final
     Task*              mpTask;        ///< Pointer to VCL Task instance
     bool               mbInScheduler; ///< Task currently processed?
     sal_uInt64         mnUpdateTime;  ///< Last Update Time
+    TaskPriority       mePriority;    ///< Task priority
 
     const char *GetDebugName() const;
 };

vcl/opengl/gdiimpl.cxx

@@ -63,9 +63,8 @@ public:
     virtual void Invoke() override
     {
         m_pImpl->doFlush();
-        if (GetPriority() != TaskPriority::HIGHEST)
-            SetPriority(TaskPriority::HIGHEST);
         Stop();
+        SetPriority(TaskPriority::HIGHEST);
     }
 };
 

vcl/source/app/scheduler.cxx

@@ -316,7 +316,10 @@ static void AppendSchedulerData( ImplSchedulerContext &rSchedCtx,
                                         ImplSchedulerData * const pSchedulerData)
 {
     assert(pSchedulerData->mpTask);
-    const int nTaskPriority = static_cast<int>(pSchedulerData->mpTask->GetPriority());
+    pSchedulerData->mePriority = pSchedulerData->mpTask->GetPriority();
+    pSchedulerData->mpNext = nullptr;
+
+    const int nTaskPriority = static_cast<int>(pSchedulerData->mePriority);
     if (!rSchedCtx.mpLastSchedulerData[nTaskPriority])
     {
         rSchedCtx.mpFirstSchedulerData[nTaskPriority] = pSchedulerData;
@@ -327,7 +330,6 @@ static void AppendSchedulerData( ImplSchedulerContext &rSchedCtx,
         rSchedCtx.mpLastSchedulerData[nTaskPriority]->mpNext = pSchedulerData;
         rSchedCtx.mpLastSchedulerData[nTaskPriority] = pSchedulerData;
     }
-    pSchedulerData->mpNext = nullptr;
 }
 
 static ImplSchedulerData* DropSchedulerData(
@@ -555,7 +557,17 @@ void Task::Start()
     if ( !rSchedCtx.mbActive )
         return;
 
-    // Mark timer active
+    // is the task scheduled in the correct priority queue?
+    // if not we have to get a new data object, as we don't want to traverse
+    // the whole list to move the data to the correct list, as the task list
+    // is just single linked.
+    // Task priority doesn't change that often AFAIK, or we might need to
+    // start caching ImplSchedulerData objects.
+    if (mpSchedulerData && mpSchedulerData->mePriority != mePriority)
+    {
+        mpSchedulerData->mpTask = nullptr;
+        mpSchedulerData = nullptr;
+    }
     mbActive = true;
 
     if ( !mpSchedulerData )
@@ -564,6 +576,7 @@ void Task::Start()
         ImplSchedulerData* pSchedulerData = new ImplSchedulerData;
         pSchedulerData->mpTask            = this;
         pSchedulerData->mbInScheduler     = false;
+        // mePriority is set in AppendSchedulerData
         mpSchedulerData = pSchedulerData;
 
         AppendSchedulerData( rSchedCtx, pSchedulerData );
@@ -584,6 +597,16 @@ void Task::Stop()
     mbActive = false;
 }
 
+void Task::SetPriority(TaskPriority ePriority)
+{
+    // you don't actually need to call Stop() before but Start() after, but we
+    // can't check that and don't know when Start() should be called.
+    SAL_WARN_IF(mpSchedulerData && mbActive, "vcl.schedule",
+                "Stop the task before changing the priority, as it will just "
+                "change after the task was scheduled with the old prio!");
+    mePriority = ePriority;
+}
+
 Task& Task::operator=( const Task& rTask )
 {
     if ( IsActive() )