1389314 - Disable the input event queue when dnd is active and shutdown

Status: RESOLVED FIXED

(Core :: DOM: Events, enhancement)

Comment 1

[Ming-Chou Shih [:stone]]

Attachment: Part1: Rename event prioritization to input event queue.

Comment 2

[Ming-Chou Shih [:stone]]

Attachment: Part2: Support enabling and disabling the input event queue in runtime.

This patch changes
1.Control the enable and disable of the input event queue in ContentParent (To control enabling and disabling input event queue in the same class)
2.Use one more IPC to flush all pending events before the disable request
3.Duplicate input IPC messages with normal priority
4.Send input IPC with input priority when the input event queue is enabled, otherwise, send normal priority IPC messages.

Comment 3

[Ming-Chou Shih [:stone]]

Attachment: Part3: Temporarily disable the input event queue when dnd is active

Comment 4

[Ming-Chou Shih [:stone]]

Attachment: Part4: Disable the input event queue when shutting down

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8896863 [details] [diff] [review]
Part3: Temporarily disable the input event queue when dnd is active

># HG changeset patch
># User Stone Shih <[e-mail]>
># Date 1501658615 -28800
>#      Wed Aug 02 15:23:35 2017 +0800
># Node ID 1c10fda53f82c1e06b1fc82afa13a35773296a4d
># Parent  4c6a8c1bbdb69cd20f3060f663be63a4bb5256d4
>Bug 1389314 Part3: Temporarily disable the input event queue when dnd is active
>
>MozReview-Commit-ID: AWfYmhjjsq0
>
>diff --git a/dom/events/EventStateManager.cpp b/dom/events/EventStateManager.cpp
>--- a/dom/events/EventStateManager.cpp
>+++ b/dom/events/EventStateManager.cpp
>@@ -1290,17 +1290,25 @@ EventStateManager::DispatchCrossProcessE
>     // Let the child process synthesize a mouse event if needed, and
>     // ensure we don't synthesize one in this process.
>     *aStatus = nsEventStatus_eConsumeNoDefault;
>     remote->SendRealTouchEvent(*aEvent->AsTouchEvent());
>     return;
>   }
>   case eDragEventClass: {
>     if (remote->Manager()->IsContentParent()) {
>-      remote->Manager()->AsContentParent()->MaybeInvokeDragSession(remote);
>+      ContentParent* parent = remote->Manager()->AsContentParent();
>+      // dnd uses IPCBlob to transfer data to the content process and the IPC
>+      // message is sent as normal priority. When the input event queue is
>+      // enabled, the message may be preempted by the later dnd events. To make
>+      // sure the input events and the blob message are processed in time order
>+      // on the content process, we temporarily disable the input event queue
>+      // when starting a dnd session and re-enable it when stopping the session.
>+      parent->MaybeSetInputEventQueueEnable(false);
I guess the method name should be MaybeSetInputEventQueueEnabled.
Also, couldn't you call MaybeSetInputEventQueueEnabled in MaybeInvokeDragSession?
I think I'd prefer that.
So, just move this new stuff to MaybeInvokeDragSession

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8896862 [details] [diff] [review]
Part2: Support enabling and disabling the input event queue in runtime.

> /* static */ already_AddRefed<nsIRunnable>
> NativeInputRunnable::Create(already_AddRefed<nsIRunnable>&& aEvent)
> {
>+  MOZ_ASSERT(NS_IsMainThread());
>+  if (XRE_IsParentProcess()) {
>+    // We don't need to wrap the input runnable on the parent process because
>+    // we only support the input event queue on content.
>+    nsCOMPtr<nsIRunnable> event(Move(aEvent));
>+    return event.forget();
>+  }
Why we need this check...

>+  bool enabled = false;
>+  NS_GetCurrentThread()->IsInputEventQueueEnabled(&enabled);
>+  if (!enabled) {
when we have this. InputEventQueue shouldn't be enabled in parent process.

>+mozilla::ipc::IPCResult
>+ContentChild::RecvSetInputEventQueueEnable(const bool& aEnable)
RectSetInputEventQueueEnabled

>+{
>+  nsThreadManager::get().SetInputEventQueueEnable(aEnable);
SetInputEventQueueEnabled


> TabParent::SendRealDragEvent(WidgetDragEvent& aEvent, uint32_t aDragAction,
>                              uint32_t aDropEffect)
> {
>   if (mIsDestroyed || !mIsReadyToHandleInputEvents) {
>     return;
>   }
>   aEvent.mRefPoint += GetChildProcessOffset();
>-  DebugOnly<bool> ret =
>-    PBrowserParent::SendRealDragEvent(aEvent, aDragAction, aDropEffect);
>+  DebugOnly<bool> ret = Manager()->AsContentParent()->IsInputEventQueueEnabled()
>+    ? PBrowserParent::SendRealDragEvent(aEvent, aDragAction, aDropEffect)
>+    : PBrowserParent::SendNormalPriorityRealDragEvent(aEvent, aDragAction,
>+                                                      aDropEffect);
Do we ever call SendRealDragEvent?

>+nsThread::nsChainedEventQueue::
>+EnableInputEventQueue(MutexAutoLock& aProofOfLock, bool aEnable)
>+{
>+  MOZ_ASSERT(mIsInputEventQueueEnabled != aEnable);
>+  if (aEnable) {
>+    mCanGetEventFromInputQueue = true;
>+    mInputHandlingStartTime = TimeStamp();
>+  } else {
>+    // When disabling the input event queue, there may be some pending events in
>+    // the input event queue and normal event queue. To avoid the newly added
>+    // input events (which are sent as normal priority after the disable request)
>+    // preempt the pending input events, we have to flush all pending events
>+    // before the disable request. We add this IPC message in the input event
>+    // queue.
No. This is in normal queue.
async SetInputEventQueueEnable(bool aEnable);

 If it's processed before the disable request (which is put in the
>+    // normal event queue), we flush the normal event queue until the disable
>+    // request is handled. Otherwise, we flush the input event queue.
>+    if (mIsFlushingNormalQueue) {
>+      mIsFlushingNormalQueue = false;
>+      mCanGetEventFromInputQueue = false;
>+    } else {
>+      mIsFlushingInputQueue = true;
>+    }
Can we ever have both mIsFlushingNormalQueue and mIsFlushingInputQueue true? If so, please document when that can happen.


>+nsThread::nsChainedEventQueue::
>+FlushNormalQueueUntilDisableInputQueue(MutexAutoLock& aProofOfLock)
> {
>-  MOZ_ASSERT(!mIsInputEventQueueEnabled);
>-  // When enabling input event queue, there may be some pending events with
>-  // different priorities in the normal queue. Create an event in the normal
>-  // queue to consume all pending events in the time order to make sure we won't
>-  // preempt a pending event (e.g. input) in the normal queue by another newly
>-  // created event with the same priority.
>-  mNormalQueue->PutEvent(new EnableInputEventQueueRunnable(this), aProofOfLock);
>-  mInputHandlingStartTime = TimeStamp();
>-  mIsInputEventQueueEnabled = true;
>+  if (mIsFlushingInputQueue) {
>+    mIsFlushingInputQueue = false;
>+    mCanGetEventFromInputQueue = false;
>+  } else {
>+    mIsFlushingNormalQueue = true;
>+  }
I'm having trouble to understand all this mIsFlushingInputQueue and mIsFlushingNormalQueue handling.
I would expect this method to set mIsFlushingNormalQueue, but apparently it is doing also some work to mIsFlushingInputQueue. Why?



Could you clarify mIsFlushing*Queue handling and also explain why we need prio(input) async RealDragEvent?
I could take another look.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

(sorry, I was a bit vague with review, but I just think I need to take another look)

Comment 8

[Ming-Chou Shih [:stone]]

(In reply to Olli Pettay [:smaug] from comment #6)
> Comment on attachment 8896862 [details] [diff] [review]
> Part2: Support enabling and disabling the input event queue in runtime.
> 
> > /* static */ already_AddRefed<nsIRunnable>
> > NativeInputRunnable::Create(already_AddRefed<nsIRunnable>&& aEvent)
> > {
> >+  MOZ_ASSERT(NS_IsMainThread());
> >+  if (XRE_IsParentProcess()) {
> >+    // We don't need to wrap the input runnable on the parent process because
> >+    // we only support the input event queue on content.
> >+    nsCOMPtr<nsIRunnable> event(Move(aEvent));
> >+    return event.forget();
> >+  }
> Why we need this check...
Oh, yes. it's covered by IsInputEventQueueEnabled. I'll remove it.

> > TabParent::SendRealDragEvent(WidgetDragEvent& aEvent, uint32_t aDragAction,
> >                              uint32_t aDropEffect)
> > {
> >   if (mIsDestroyed || !mIsReadyToHandleInputEvents) {
> >     return;
> >   }
> >   aEvent.mRefPoint += GetChildProcessOffset();
> >-  DebugOnly<bool> ret =
> >-    PBrowserParent::SendRealDragEvent(aEvent, aDragAction, aDropEffect);
> >+  DebugOnly<bool> ret = Manager()->AsContentParent()->IsInputEventQueueEnabled()
> >+    ? PBrowserParent::SendRealDragEvent(aEvent, aDragAction, aDropEffect)
> >+    : PBrowserParent::SendNormalPriorityRealDragEvent(aEvent, aDragAction,
> >+                                                      aDropEffect);
> Do we ever call SendRealDragEvent?
Actually, we should never call SendRealDragEvent since we disable the input event queue when the dnd is active. I'll revise it.

> >+nsThread::nsChainedEventQueue::
> >+EnableInputEventQueue(MutexAutoLock& aProofOfLock, bool aEnable)
> >+{
> >+  MOZ_ASSERT(mIsInputEventQueueEnabled != aEnable);
> >+  if (aEnable) {
> >+    mCanGetEventFromInputQueue = true;
> >+    mInputHandlingStartTime = TimeStamp();
> >+  } else {
> >+    // When disabling the input event queue, there may be some pending events in
> >+    // the input event queue and normal event queue. To avoid the newly added
> >+    // input events (which are sent as normal priority after the disable request)
> >+    // preempt the pending input events, we have to flush all pending events
> >+    // before the disable request. We add this IPC message in the input event
> >+    // queue.
> No. This is in normal queue.
Yes. it's normal queue.

> async SetInputEventQueueEnable(bool aEnable);
> 
>  If it's processed before the disable request (which is put in the
> >+    // normal event queue), we flush the normal event queue until the disable
> >+    // request is handled. Otherwise, we flush the input event queue.
> >+    if (mIsFlushingNormalQueue) {
> >+      mIsFlushingNormalQueue = false;
> >+      mCanGetEventFromInputQueue = false;
> >+    } else {
> >+      mIsFlushingInputQueue = true;
> >+    }
> Can we ever have both mIsFlushingNormalQueue and mIsFlushingInputQueue true?
> If so, please document when that can happen.
It should never happen. I'll add some assertions to check it.

> 
> >+nsThread::nsChainedEventQueue::
> >+FlushNormalQueueUntilDisableInputQueue(MutexAutoLock& aProofOfLock)
> > {
> >-  MOZ_ASSERT(!mIsInputEventQueueEnabled);
> >-  // When enabling input event queue, there may be some pending events with
> >-  // different priorities in the normal queue. Create an event in the normal
> >-  // queue to consume all pending events in the time order to make sure we won't
> >-  // preempt a pending event (e.g. input) in the normal queue by another newly
> >-  // created event with the same priority.
> >-  mNormalQueue->PutEvent(new EnableInputEventQueueRunnable(this), aProofOfLock);
> >-  mInputHandlingStartTime = TimeStamp();
> >-  mIsInputEventQueueEnabled = true;
> >+  if (mIsFlushingInputQueue) {
> >+    mIsFlushingInputQueue = false;
> >+    mCanGetEventFromInputQueue = false;
> >+  } else {
> >+    mIsFlushingNormalQueue = true;
> >+  }
> I'm having trouble to understand all this mIsFlushingInputQueue and
> mIsFlushingNormalQueue handling.
> I would expect this method to set mIsFlushingNormalQueue, but apparently it
> is doing also some work to mIsFlushingInputQueue. Why?
When disabling the input event queue, we have to flush all pending events before the disable request (in the normal) and all pending events before the flush request (in input queue). Since we are processing the flush request, it means we already consumed all pending events before it. So we only have to flush the normal queue.

If we processed the disable request first (turn on the flag mIsFlushingInputQueue), it means we have to flush the input queue until the flush request is processed (turn off the flag mIsFlushingInputQueue and mCanGetEventFromInputQueue).

> 
> 
> Could you clarify mIsFlushing*Queue handling and also explain why we need
> prio(input) async RealDragEvent?
> I could take another look.

Comment 9

[Ming-Chou Shih [:stone]]

Attachment: Part2: Support enabling and disabling the input event queue in runtime.

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8897312 [details] [diff] [review]
Part2: Support enabling and disabling the input event queue in runtime.

Have you tested dnd from one child process to another one?

>+
>+  bool IsInputEventQueueSupported();
This could be static...

>+++ b/dom/ipc/TabParent.cpp
>@@ -173,17 +173,17 @@ TabParent::TabParent(nsIContentParent* a
>   , mHasPresented(false)
>   , mHasBeforeUnload(false)
>   , mIsMouseEnterIntoWidgetEventSuppressed(false)
> {
>   MOZ_ASSERT(aManager);
>   // When the input event queue is disabled, we don't need to handle the case
>   // that some input events are dispatched before PBrowserConstructor.
>   mIsReadyToHandleInputEvents =
>-    !Preferences::GetBool("input_event_queue.supported", false);
>+    !Manager()->AsContentParent()->IsInputEventQueueEnabled();
...then this coudl be a bit simpler.
Just nsContentParent::IsInputEventQueueSupported()

>+void
>+nsThread::nsChainedEventQueue::
>+EnableInputEventQueue(MutexAutoLock& aProofOfLock, bool aEnable)
>+{
>+  MOZ_ASSERT(mIsInputEventQueueEnabled != aEnable);
>+  if (aEnable) {
>+    mCanGetEventFromInputQueue = true;
>+    mInputHandlingStartTime = TimeStamp();
>+  } else {
>+    // When disabling the input event queue, there may be some pending events in
>+    // the input event queue and normal event queue. To avoid the newly added
>+    // input events (which are sent as normal priority after the disable request)
>+    // preempt the pending input events, we have to flush all pending events.
>+    // We add the disable request in the normal queue and the flush request in
>+    // the input event queue. If the flush request processed before the disable
>+    // request, we flush the normal event queue until the disable request is
>+    // handled. Otherwise, we flush the input event queue.
>+    MOZ_ASSERT(!mIsFlushingInputQueue);
>+    if (mIsFlushingNormalQueue) {
>+      mIsFlushingNormalQueue = false;
>+      mCanGetEventFromInputQueue = false;
>+    } else {
>+      mIsFlushingInputQueue = true;
>+    }
>+  }
>+  mIsInputEventQueueEnabled = aEnable;
>+  InputEventStatistics::Get().SetEnable(aEnable);
>+  MOZ_ASSERT(!mIsFlushingInputQueue || !mIsFlushingNormalQueue);
>+}
ok, so if we first get FlushNormalQueueUntilDisableInputQueue in input queue and then this, we end up having both
mIsFlushingNormalQueue and mIsFlushingInputQueue false, since mIsFlushingInputQueue was never set to true and this sets mIsFlushingNormalQueue false.
Then mCanGetEventFromInputQueue is also false, so GetEvent will not even try to use input queue. Yet nothing has guaranteed that no other runnables were added to the input queue.
What guarantees all the runnables from input queue get handled?
I feel like I'm missing something here since the setup is rather complicated. Could we just not merge input event queue to normal queue (beginning of it) when we disable input queue or something.


So I think there is a bug there, but I could have made a mistake. This is hard to follow. Using some variable and enum as its type to indicate the current state might make this easier to follow.

Comment 11

[Ming-Chou Shih [:stone]]

(In reply to Olli Pettay [:smaug] from comment #10)
> Comment on attachment 8897312 [details] [diff] [review]
> Part2: Support enabling and disabling the input event queue in runtime.
> >+void
> >+nsThread::nsChainedEventQueue::
> >+EnableInputEventQueue(MutexAutoLock& aProofOfLock, bool aEnable)
> >+{
> >+  MOZ_ASSERT(mIsInputEventQueueEnabled != aEnable);
> >+  if (aEnable) {
> >+    mCanGetEventFromInputQueue = true;
> >+    mInputHandlingStartTime = TimeStamp();
> >+  } else {
> >+    // When disabling the input event queue, there may be some pending events in
> >+    // the input event queue and normal event queue. To avoid the newly added
> >+    // input events (which are sent as normal priority after the disable request)
> >+    // preempt the pending input events, we have to flush all pending events.
> >+    // We add the disable request in the normal queue and the flush request in
> >+    // the input event queue. If the flush request processed before the disable
> >+    // request, we flush the normal event queue until the disable request is
> >+    // handled. Otherwise, we flush the input event queue.
> >+    MOZ_ASSERT(!mIsFlushingInputQueue);
> >+    if (mIsFlushingNormalQueue) {
> >+      mIsFlushingNormalQueue = false;
> >+      mCanGetEventFromInputQueue = false;
> >+    } else {
> >+      mIsFlushingInputQueue = true;
> >+    }
> >+  }
> >+  mIsInputEventQueueEnabled = aEnable;
> >+  InputEventStatistics::Get().SetEnable(aEnable);
> >+  MOZ_ASSERT(!mIsFlushingInputQueue || !mIsFlushingNormalQueue);
> >+}
> ok, so if we first get FlushNormalQueueUntilDisableInputQueue in input queue
> and then this, we end up having both
> mIsFlushingNormalQueue and mIsFlushingInputQueue false, since
> mIsFlushingInputQueue was never set to true and this sets
> mIsFlushingNormalQueue false.
> Then mCanGetEventFromInputQueue is also false, so GetEvent will not even try
> to use input queue. Yet nothing has guaranteed that no other runnables were
> added to the input queue.
At this moment, we shouldn't get and handle any runnables in the input event queue until an enable request is handled (which sets mCanGetEventFromInputQueue to true)

It's possible that an enable request is put in the normal queue and then some input events are put in the input event queue (after the flush request). We have to consume all pending normal events before the enable request then we can start handling the input events again.

> What guarantees all the runnables from input queue get handled?
All input runnables before the flush request should already be handled in this case (when the flush request is handled).

If there are some pending input events after the flush request, it must be the case that we enable the input event queue again. So there must be a pending enable request in the normal queue. Once it's processed, we can continue processing the input event queue.

Comment 12

[Ming-Chou Shih [:stone]]

Here is a possible snapshot of the pending events
R: Runnables
E: Enable request
D: Disable request
F: Flush request

Normal Queue: R1 E  R3    D R5 R6 E  R9
Input Queue:      R2  R4 F         R8

Since we control the priority of the input events on the parent process, the input events are only sent as input priority and put in the input event queue after the enable request and before the disable request. So we only process the input event queue in the time interval.

The enable handling is simpler because we only need to handle the case that all pending events in the normal queue. Once we handled the enable request, we can consume pending events in the normal queue or in the input event queue.

The disable handling is complex because we may consume the pending events in the normal queue or input event queue. To make sure we won't
1) consume any events in the normal queue which are sent after the disable request (e.g. R5) when there are some pending events in the input queue which are sent before the disable request. (e.g. R4)

2) consume any events in the input event queue which are sent after the disable request (e.g. R8) when there are some pending events in the normal queue which are sent before the disable request. (e.g. R3)

We have to consume all pending events before the disable request and the flush request.

Comment 13

[Olli Pettay [:smaug][[e-mail]]]

(In reply to Ming-Chou Shih [:stone] from comment #11)

> > What guarantees all the runnables from input queue get handled?
> All input runnables before the flush request should already be handled in
> this case (when the flush request is handled).
Why? Flush is in input queue sure, but what if one has dispatched input priority event after the flush has been added to the queue?
Or is this all just relying on what we happen to do in TabParent/ContentParent level?

Comment 14

[Ming-Chou Shih [:stone]]

(In reply to Olli Pettay [:smaug] from comment #13)
> (In reply to Ming-Chou Shih [:stone] from comment #11)
> 
> > > What guarantees all the runnables from input queue get handled?
> > All input runnables before the flush request should already be handled in
> > this case (when the flush request is handled).
> Why? Flush is in input queue sure, but what if one has dispatched input
> priority event after the flush has been added to the queue?
> Or is this all just relying on what we happen to do in
> TabParent/ContentParent level?
Yes. It's relying on TabParent/ContentParent.

Comment 15

[Olli Pettay [:smaug][[e-mail]]]

Could we possibly make nsIThread API less error prone, just in case we need to dispatch runnables to input queue inside child process?

Comment 16

[Ming-Chou Shih [:stone]]

(In reply to Olli Pettay [:smaug] from comment #15)
> Could we possibly make nsIThread API less error prone, just in case we need
> to dispatch runnables to input queue inside child process?

I had tried but can't find a simple solution for it. The difficulty is that there may be some pending input events put in the input event queue when processing the disable request. We have no idea whether they are sent before the disable request or not. It's hard to merge them with the pending normal events (after the disable request).
- handling all pending input events before normal events may cause failure of dnd (transferred data is sent as normal priority)
- handling all pending normal events before input events may cause failure if there is a pending enable request.

We can simplify the problem by immediately stop putting new input events in the input queue when receiving the disable ipc message. e.g. add a new priority label and do some hacks in nsChainedEventQueue::PutEvent. Not sure it's better or worse.

Comment 17

[Ming-Chou Shih [:stone]]

(In reply to Ming-Chou Shih [:stone] from comment #16)
> (In reply to Olli Pettay [:smaug] from comment #15)
> > Could we possibly make nsIThread API less error prone, just in case we need
> > to dispatch runnables to input queue inside child process?
> 
> I had tried but can't find a simple solution for it. The difficulty is that
> there may be some pending input events put in the input event queue when
> processing the disable request. We have no idea whether they are sent before
> the disable request or not. It's hard to merge them with the pending normal
> events (after the disable request).
> - handling all pending input events before normal events may cause failure
> of dnd (transferred data is sent as normal priority)
> - handling all pending normal events before input events may cause failure
> if there is a pending enable request.
> 
> We can simplify the problem by immediately stop putting new input events in
> the input queue when receiving the disable ipc message. e.g. add a new
> priority label and do some hacks in nsChainedEventQueue::PutEvent. Not sure
> it's better or worse.

Another solution is re-post all pending messages in MessageChannel after disabling the input event queue.

Comment 18

[Ming-Chou Shih [:stone]]

Attachment: Part2: Support enabling and disabling the input priority events in runtime (v2)

Comment 19

[Ming-Chou Shih [:stone]]

Comment on attachment 8898667 [details] [diff] [review]
Part2: Support enabling and disabling the input priority events in runtime (v2)

This implementation doesn't really disable the input event queue. It only flushes the pending events in the normal queue and the input queue. Wondering if this is better or not.

Comment 20

[Ming-Chou Shih [:stone]]

Comment on attachment 8898667 [details] [diff] [review]
Part2: Support enabling and disabling the input priority events in runtime (v2)

oops. got some bugs.

Comment 21

[Ming-Chou Shih [:stone]]

Attachment: Part2: Support enabling and disabling the input priority events in runtime.

Comment 22

[Ming-Chou Shih [:stone]]

Comment on attachment 8899438 [details] [diff] [review]
Part2: Support enabling and disabling the input priority events in runtime.

Rebase and revise the patch

Comment 23

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8899438 [details] [diff] [review]
Part2: Support enabling and disabling the input priority events in runtime.

>+     *
>+     * When the flush input reuqest is processed before the other two requests,
request

>+     * we consume all input events until the suspend request. After handling the
>+     * suspend request, we stop consuming the input events until the resume
>+     * request to make sure we consume all pending normal events.
>+     *
>+     * If we process the suspend reuqest before the other two requests, we
request

>+     * ignore the flush request and consume all pending normal events until the
>+     * resume reuqest.
request

>   if (mProcessHighPriorityQueue) {
>     queue = EventPriority::High;
>-  } else if (inputCount > 0 && TimeStamp::Now() > mInputHandlingStartTime
>-             && mReadFromInputQueue) {
>+  } else if (inputCount > 0 && (mInputQueueState == STATE_FLUSHING ||
>+                                (mInputQueueState == STATE_ENABLED &&
>+                                 TimeStamp::Now() > mInputHandlingStartTime))) {
>     queue = EventPriority::Input;
>   } else if (normalPending) {
>+    MOZ_ASSERT(mInputQueueState != STATE_FLUSHING);
This assertion could use some explanation, so add the second "some explanation" param.


>     queue = EventPriority::Normal;
>   } else if (highPending) {
>     queue = EventPriority::High;
>-  } else if (inputCount > 0 && mReadFromInputQueue) {
>+  } else if (inputCount > 0 && mInputQueueState != STATE_SUSPEND) {
>+    MOZ_ASSERT(mInputQueueState != STATE_DISABLED);
Same here

>+  enum InputEventQueueState {
Nit, { could go to its own line

Comment 24

[Ming-Chou Shih [:stone]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8aa7981eae09eccc6257156db54c8bfe4eb316b3

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3848c0616c9a
Part1: Rename event prioritization to input event queue. r=smaug.
https://hg.mozilla.org/integration/mozilla-inbound/rev/5f6a7c8712b3
Part2: Support enabling and disabling the input priority events in runtime. r=smaug.
https://hg.mozilla.org/integration/mozilla-inbound/rev/949525c39b95
Part3: Temporarily disable the input priority events when dnd is active. r=smaug.
https://hg.mozilla.org/integration/mozilla-inbound/rev/f57e58155599
Part4: Disable the input priority event when shutting down. r=smaug.

Comment 26

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/3848c0616c9a
https://hg.mozilla.org/mozilla-central/rev/5f6a7c8712b3
https://hg.mozilla.org/mozilla-central/rev/949525c39b95
https://hg.mozilla.org/mozilla-central/rev/f57e58155599