Bug 1615607 - Consider to always treat high priority runnables as the highest priority runnables, r=farre

The patch removes the tad odd interleave behavior from the main thread and makes RefreshDriver to give
at least a tiny bit time for non-high priority tasks.
Given the recent change to have similar block-until behavior in child and parent process, this should work
consistently in all the processes.

Differential Revision: https://phabricator.services.mozilla.com/D63098

--HG--
extra : moz-landing-system : lando

layout/base/nsRefreshDriver.cpp

@@ -732,17 +732,25 @@ class VsyncRefreshDriverTimer : public RefreshDriverTimer {
           aVsyncTimestamp <= *&rightnow);
 #endif
 
+      // Let also non-RefreshDriver code to run at least for awhile if we have
+      // a mVsyncRefreshDriverTimer. Note, if nothing else is running,
+      // RefreshDriver will still run as fast as possible, some ticks will
+      // just be triggered from a normal priority runnable.
+      TimeDuration timeForOutsideTick = TimeDuration::FromMilliseconds(0.0f);
+
       // We might have a problem that we call ~VsyncRefreshDriverTimer() before
       // the scheduled TickRefreshDriver() runs. Check mVsyncRefreshDriverTimer
       // before use.
       if (mVsyncRefreshDriverTimer) {
+        timeForOutsideTick = TimeDuration::FromMilliseconds(
+            mVsyncRefreshDriverTimer->GetTimerRate().ToMilliseconds() / 10.0f);
         RefPtr<VsyncRefreshDriverTimer> timer = mVsyncRefreshDriverTimer;
         timer->RunRefreshDrivers(aId, aVsyncTimestamp);
         // Note: mVsyncRefreshDriverTimer might be null now.
       }
 
       TimeDuration tickDuration = TimeStamp::Now() - mLastTick;
-      mBlockUntil = aVsyncTimestamp + tickDuration;
+      mBlockUntil = aVsyncTimestamp + tickDuration + timeForOutsideTick;
     }
 
     // VsyncRefreshDriverTimer holds this RefreshDriverVsyncObserver and it will

xpcom/tests/gtest/TestEventPriorities.cpp

@@ -66,14 +66,14 @@ TEST(EventPriorities, IdleAfterNormal)
       SpinEventLoopUntil([&]() { return normalRan == 3 && idleRan == 3; }));
 }
 
-TEST(EventPriorities, InterleaveHighNormal)
+TEST(EventPriorities, HighNormal)
 {
   int normalRan = 0, highRan = 0;
 
   RefPtr<TestEvent> evNormal = new TestEvent(
-      &normalRan, [&] { ASSERT_TRUE(abs(normalRan - highRan) <= 1); });
+      &normalRan, [&] { ASSERT_TRUE((highRan - normalRan) >= 0); });
   RefPtr<TestEvent> evHigh = new TestEvent(
-      &highRan, [&] { ASSERT_TRUE(abs(normalRan - highRan) <= 1); },
+      &highRan, [&] { ASSERT_TRUE((highRan - normalRan) >= 0); },
       nsIRunnablePriority::PRIORITY_HIGH);
 
   NS_DispatchToMainThread(evNormal);

xpcom/threads/PrioritizedEventQueue.cpp

@@ -89,15 +89,13 @@ EventQueuePriority PrioritizedEventQueue::SelectQueue(
   // are meant to avoid starvation and to ensure that we don't process an event
   // at the wrong time.
   //
-  // HIGH: if mProcessHighPriorityQueue
+  // HIGH:
   // INPUT: if inputCount > 0 && TimeStamp::Now() > mInputHandlingStartTime
   // MEDIUMHIGH: if medium high pending
   // NORMAL: if normal pending
   //
   // If we still don't have an event, then we take events from the queues
   // in the following order:
-  //
-  // HIGH
   // INPUT
   // DEFERREDTIMERS: if GetLocalIdleDeadline()
   // IDLE: if GetLocalIdleDeadline()
@@ -107,9 +105,7 @@ EventQueuePriority PrioritizedEventQueue::SelectQueue(
 
   // This variable determines which queue we will take an event from.
   EventQueuePriority queue;
-  bool highPending = !mHighQueue->IsEmpty(aProofOfLock);
-
-  if (mProcessHighPriorityQueue) {
+  if (!mHighQueue->IsEmpty(aProofOfLock)) {
     queue = EventQueuePriority::High;
   } else if (inputCount > 0 && (mInputQueueState == STATE_FLUSHING ||
                                 (mInputQueueState == STATE_ENABLED &&
@@ -125,8 +121,6 @@ EventQueuePriority PrioritizedEventQueue::SelectQueue(
     MOZ_ASSERT(mInputQueueState != STATE_FLUSHING,
                "Shouldn't consume normal event when flushing input events");
     queue = EventQueuePriority::Normal;
-  } else if (highPending) {
-    queue = EventQueuePriority::High;
   } else if (inputCount > 0 && mInputQueueState != STATE_SUSPEND) {
     MOZ_ASSERT(
         mInputQueueState != STATE_DISABLED,
@@ -144,10 +138,6 @@ EventQueuePriority PrioritizedEventQueue::SelectQueue(
       queue == EventQueuePriority::Input,
       mInputQueueState != STATE_DISABLED && mInputQueueState != STATE_SUSPEND);
 
-  if (aUpdateState) {
-    mProcessHighPriorityQueue = highPending;
-  }
-
   return queue;
 }
 
@@ -193,7 +183,6 @@ already_AddRefed<nsIRunnable> PrioritizedEventQueue::GetEvent(
                                    aHypotheticalInputEventDelay);
       MOZ_ASSERT(event);
       mInputHandlingStartTime = TimeStamp();
-      mProcessHighPriorityQueue = false;
       break;
 
     case EventQueuePriority::Input:

xpcom/threads/PrioritizedEventQueue.h

@@ -117,12 +117,6 @@ class PrioritizedEventQueue final : public AbstractEventQueue {
   TimeDuration mLastEventDelay;
   TimeStamp mLastEventStart;
 
-  // Try to process one high priority runnable after each normal
-  // priority runnable. This gives the processing model HTML spec has for
-  // 'Update the rendering' in the case only vsync messages are in the
-  // secondary queue and prevents starving the normal queue.
-  bool mProcessHighPriorityQueue = false;
-
   TimeStamp mInputHandlingStartTime;
 
   enum InputEventQueueState {