Bug 1641737 - P1. Set dispatch type to chained promises. r=bholley
☠☠ backed out by 0f4b97c32ba0 ☠ ☠
authorJean-Yves Avenard <jyavenard@mozilla.com>
Tue, 09 Jun 2020 07:23:29 +0000
changeset 534604 0296d0f6c1d120225f2279f378d33f83bdc02eae
parent 534603 4931f2184a22dd1a33111d678024a10e4cb1803d
child 534605 28bca5c5b8b4d5a4f8e6ecaf621738ad955c3121
push id37492
push userapavel@mozilla.com
push dateTue, 09 Jun 2020 15:16:49 +0000
treeherdermozilla-central@7f7b98339065 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbholley
bugs1641737
milestone79.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1641737 - P1. Set dispatch type to chained promises. r=bholley When chaining a MozPromise set to be dispatched via the direct task queue (or synchronous), it makes sense for the chained promise to be dispatched in the same fashion. All MozPromises generated by the IPC bindings are set to use the direct task queue in order to prevent the then runnable to run out of order with other IPC tasks. We want to preserve that task ordering by default. Differential Revision: https://phabricator.services.mozilla.com/D78178
xpcom/tests/gtest/TestMozPromise.cpp
xpcom/threads/MozPromise.h
--- a/xpcom/tests/gtest/TestMozPromise.cpp
+++ b/xpcom/tests/gtest/TestMozPromise.cpp
@@ -1,24 +1,21 @@
 /* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-#include "gtest/gtest.h"
-
+#include "VideoUtils.h"
 #include "base/message_loop.h"
-
-#include "mozilla/TaskQueue.h"
+#include "gtest/gtest.h"
 #include "mozilla/MozPromise.h"
+#include "mozilla/SharedThreadPool.h"
+#include "mozilla/TaskQueue.h"
 #include "mozilla/Unused.h"
-
 #include "nsISupportsImpl.h"
-#include "mozilla/SharedThreadPool.h"
-#include "VideoUtils.h"
 
 using namespace mozilla;
 
 typedef MozPromise<int, double, false> TestPromise;
 typedef MozPromise<int, double, true /* exclusive */> TestPromiseExcl;
 typedef TestPromise::ResolveOrRejectValue RRValue;
 
 class MOZ_STACK_CLASS AutoTaskQueue {
@@ -450,16 +447,31 @@ TEST(MozPromise, MessageLoopEventTarget)
           MessageLoop::current()->SerialEventTarget(), __func__,
           [](int aResolveValue) -> void { EXPECT_EQ(aResolveValue, 42); },
           DO_FAIL);
 
   // Spin the event loop.
   NS_ProcessPendingEvents(nullptr);
 }
 
+TEST(MozPromise, ChainTo)
+{
+  RefPtr<TestPromise> promise1 = TestPromise::CreateAndResolve(42, __func__);
+  RefPtr<TestPromise::Private> promise2 = new TestPromise::Private(__func__);
+  promise2->Then(
+      GetCurrentThreadSerialEventTarget(), __func__,
+      [&](int aResolveValue) -> void { EXPECT_EQ(aResolveValue, 42); },
+      DO_FAIL);
+
+  promise1->ChainTo(promise2.forget(), __func__);
+
+  // Spin the event loop.
+  NS_ProcessPendingEvents(nullptr);
+}
+
 TEST(MozPromise, SynchronousTaskDispatch1)
 {
   bool value = false;
   RefPtr<TestPromiseExcl::Private> promise =
       new TestPromiseExcl::Private(__func__);
   promise->UseSynchronousTaskDispatch(__func__);
   promise->Resolve(42, __func__);
   EXPECT_EQ(value, false);
@@ -501,16 +513,17 @@ TEST(MozPromise, DirectTaskDispatch)
   // a task.
   GetCurrentThreadSerialEventTarget()->Dispatch(
       NS_NewRunnableFunction("test", [&]() {
         GetCurrentThreadSerialEventTarget()->Dispatch(
             NS_NewRunnableFunction("test", [&]() {
               EXPECT_EQ(value1, true);
               value2 = true;
             }));
+
         RefPtr<TestPromise::Private> promise =
             new TestPromise::Private(__func__);
         promise->UseDirectTaskDispatch(__func__);
         promise->Resolve(42, __func__);
         EXPECT_EQ(value1, false);
         promise->Then(
             GetCurrentThreadSerialEventTarget(), __func__,
             [&](int aResolveValue) -> void {
@@ -520,9 +533,99 @@ TEST(MozPromise, DirectTaskDispatch)
             },
             DO_FAIL);
         EXPECT_EQ(value1, false);
       }));
 
   // Spin the event loop.
   NS_ProcessPendingEvents(nullptr);
 }
+
+TEST(MozPromise, ChainedDirectTaskDispatch)
+{
+  bool value1 = false;
+  bool value2 = false;
+
+  // For direct task dispatch to be working, we must be within a
+  // nested event loop. So the test itself must be dispatched within
+  // a task.
+  GetCurrentThreadSerialEventTarget()->Dispatch(
+      NS_NewRunnableFunction("test", [&]() {
+        GetCurrentThreadSerialEventTarget()->Dispatch(
+            NS_NewRunnableFunction("test", [&]() {
+              EXPECT_EQ(value1, true);
+              value2 = true;
+            }));
+
+        RefPtr<TestPromise::Private> promise1 =
+            new TestPromise::Private(__func__);
+        promise1->UseDirectTaskDispatch(__func__);
+        promise1->Resolve(42, __func__);
+        EXPECT_EQ(value1, false);
+        promise1
+            ->Then(
+                GetCurrentThreadSerialEventTarget(), __func__,
+                [&](int aResolveValue) -> RefPtr<TestPromise> {
+                  EXPECT_EQ(aResolveValue, 42);
+                  EXPECT_EQ(value2, false);
+                  RefPtr<TestPromise::Private> promise2 =
+                      new TestPromise::Private(__func__);
+                  promise2->UseDirectTaskDispatch(__func__);
+                  promise2->Resolve(43, __func__);
+                  return promise2;
+                },
+                DO_FAIL)
+            ->Then(
+                GetCurrentThreadSerialEventTarget(), __func__,
+                [&](int aResolveValue) -> void {
+                  EXPECT_EQ(aResolveValue, 43);
+                  EXPECT_EQ(value2, false);
+                  value1 = true;
+                },
+                DO_FAIL);
+        EXPECT_EQ(value1, false);
+      }));
+
+  // Spin the event loop.
+  NS_ProcessPendingEvents(nullptr);
+}
+
+TEST(MozPromise, ChainToDirectTaskDispatch)
+{
+  bool value1 = false;
+  bool value2 = false;
+
+  // For direct task dispatch to be working, we must be within a
+  // nested event loop. So the test itself must be dispatched within
+  // a task.
+  GetCurrentThreadSerialEventTarget()->Dispatch(
+      NS_NewRunnableFunction("test", [&]() {
+        GetCurrentThreadSerialEventTarget()->Dispatch(
+            NS_NewRunnableFunction("test", [&]() {
+              EXPECT_EQ(value1, true);
+              value2 = true;
+            }));
+
+        RefPtr<TestPromise::Private> promise1 =
+            new TestPromise::Private(__func__);
+        promise1->UseDirectTaskDispatch(__func__);
+
+        RefPtr<TestPromise::Private> promise2 =
+            new TestPromise::Private(__func__);
+        promise2->Then(
+            GetCurrentThreadSerialEventTarget(), __func__,
+            [&](int aResolveValue) -> void {
+              EXPECT_EQ(aResolveValue, 42);
+              EXPECT_EQ(value2, false);
+              value1 = true;
+            },
+            DO_FAIL);
+
+        promise1->ChainTo(promise2.forget(), __func__);
+        EXPECT_EQ(value1, false);
+        promise1->Resolve(42, __func__);
+      }));
+
+  // Spin the event loop.
+  NS_ProcessPendingEvents(nullptr);
+}
+
 #undef DO_FAIL
--- a/xpcom/threads/MozPromise.h
+++ b/xpcom/threads/MozPromise.h
@@ -977,16 +977,32 @@ class MozPromise : public MozPromiseBase
     MOZ_DIAGNOSTIC_ASSERT(
         !IsExclusive || !mHaveRequest,
         "Using an exclusive promise in a non-exclusive fashion");
     mHaveRequest = true;
     RefPtr<Private> chainedPromise = aChainedPromise;
     PROMISE_LOG(
         "%s invoking Chain() [this=%p, chainedPromise=%p, isPending=%d]",
         aCallSite, this, chainedPromise.get(), (int)IsPending());
+
+    // We want to use the same type of dispatching method with the chained
+    // promises.
+
+    // We need to ensure that the UseSynchronousTaskDispatch branch isn't taken
+    // at compilation time to ensure we're not triggering the static_assert in
+    // UseSynchronousTaskDispatch method. if constexpr (IsExclusive) ensures
+    // that.
+    if (mUseDirectTaskDispatch) {
+      chainedPromise->UseDirectTaskDispatch(aCallSite);
+    } else if constexpr (IsExclusive) {
+      if (mUseSynchronousTaskDispatch) {
+        chainedPromise->UseSynchronousTaskDispatch(aCallSite);
+      }
+    }
+
     if (!IsPending()) {
       ForwardTo(chainedPromise);
     } else {
       mChainedPromises.AppendElement(chainedPromise);
     }
   }
 
 #  ifdef MOZ_WIDGET_ANDROID