Bug 1593802 - don't drop dispatch flags in TaskQueue's EventTargetWrapper; r=erahm

`TaskQueue` wraps an `nsIEventTarget` to provide "one runnable at a
time" execution policies regardless of the underlying implementation of
the wrapped `nsIEventTarget` (e.g. a thread pool).  `TaskQueue` also
provides a `nsISerialEventTarget` wrapper, `EventTargetWrapper`, around
itself (!) for consumers who want to continue to provide a more
XPCOM-ish API.

One would think that dispatching tasks to `EventTargetWrapper` with a
given set of flags would pass that set of flags through, unchanged, to
the underlying event target of the wrapped `TaskQueue`.

This pass-through is not the case.  `TaskQueue` supports a "tail
dispatch" mode of operation that is somewhat underdocumented.  Roughly,
tail dispatch to a `TaskQueue` says (with several other conditions) that
dispatched tasks are held separately and not passed through to the
underlying event target.  If a given `TaskQueue` supports tail dispatch
and the dispatcher also supports tail dispatch, any tasks dispatched to
said `TaskQueue` are silently converted to tail dispatched tasks.  Since
tail dispatched tasks can't meaningfully have flags associated with
them, the current implementation simply drops any passed flags on the floor.

These flags, however, might be meaningful, and we should attempt to
honor them in the cases we're not doing tail dispatch.  (And when we are
doing tail dispatch, we can verify that the requested flags are not
asking for peculiar things.)

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

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

Author: froydnj

xpcom/threads/TaskQueue.cpp

@@ -31,7 +31,7 @@ class TaskQueue::EventTargetWrapper final : public nsISerialEventTarget {
   Dispatch(already_AddRefed<nsIRunnable> aEvent, uint32_t aFlags) override {
     nsCOMPtr<nsIRunnable> runnable = aEvent;
     MonitorAutoLock mon(mTaskQueue->mQueueMonitor);
-    return mTaskQueue->DispatchLocked(/* passed by ref */ runnable,
+    return mTaskQueue->DispatchLocked(/* passed by ref */ runnable, aFlags,
                                       NormalDispatch);
   }
 
@@ -85,6 +85,7 @@ TaskDispatcher& TaskQueue::TailDispatcher() {
 // Note aRunnable is passed by ref to support conditional ownership transfer.
 // See Dispatch() in TaskQueue.h for more details.
 nsresult TaskQueue::DispatchLocked(nsCOMPtr<nsIRunnable>& aRunnable,
+                                   uint32_t aFlags,
                                    DispatchReason aReason) {
   mQueueMonitor.AssertCurrentThreadOwns();
   if (mIsShutdown) {
@@ -94,6 +95,8 @@ nsresult TaskQueue::DispatchLocked(nsCOMPtr<nsIRunnable>& aRunnable,
   AbstractThread* currentThread;
   if (aReason != TailDispatch && (currentThread = GetCurrent()) &&
       RequiresTailDispatch(currentThread)) {
+    MOZ_ASSERT(aFlags == NS_DISPATCH_NORMAL,
+               "Tail dispatch doesn't support flags");
     return currentThread->TailDispatcher().AddTask(this, aRunnable.forget());
   }
 
@@ -102,7 +105,7 @@ nsresult TaskQueue::DispatchLocked(nsCOMPtr<nsIRunnable>& aRunnable,
     return NS_OK;
   }
   RefPtr<nsIRunnable> runner(new Runner(this));
-  nsresult rv = mTarget->Dispatch(runner.forget(), NS_DISPATCH_NORMAL);
+  nsresult rv = mTarget->Dispatch(runner.forget(), aFlags);
   if (NS_FAILED(rv)) {
     NS_WARNING("Failed to dispatch runnable to run TaskQueue");
     return rv;

xpcom/threads/TaskQueue.h

@@ -67,7 +67,7 @@ class TaskQueue : public AbstractThread {
     nsCOMPtr<nsIRunnable> r = aRunnable;
     {
       MonitorAutoLock mon(mQueueMonitor);
-      return DispatchLocked(/* passed by ref */ r, aReason);
+      return DispatchLocked(/* passed by ref */ r, NS_DISPATCH_NORMAL, aReason);
     }
     // If the ownership of |r| is not transferred in DispatchLocked() due to
     // dispatch failure, it will be deleted here outside the lock. We do so
@@ -111,7 +111,7 @@ class TaskQueue : public AbstractThread {
   // mQueueMonitor must be held.
   void AwaitIdleLocked();
 
-  nsresult DispatchLocked(nsCOMPtr<nsIRunnable>& aRunnable,
+  nsresult DispatchLocked(nsCOMPtr<nsIRunnable>& aRunnable, uint32_t aFlags,
                           DispatchReason aReason = NormalDispatch);
 
   void MaybeResolveShutdown() {