Bug 1119956 introduce DiscardableRunnable for tasks that might not run but might not implement cancellation r=asuth,sg

Classes that inherit from DiscardableRunnable are only promising that it is OK
for Run() to be skipped, rather than promising that Cancel() is effective.

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

dom/broadcastchannel/BroadcastChannel.cpp

@@ -43,27 +43,24 @@ using namespace ipc;
 
 namespace {
 
-class CloseRunnable final : public nsIRunnable, public nsICancelableRunnable {
+class CloseRunnable final : public DiscardableRunnable {
  public:
-  NS_DECL_ISUPPORTS
-
-  explicit CloseRunnable(BroadcastChannel* aBC) : mBC(aBC) { MOZ_ASSERT(mBC); }
+  explicit CloseRunnable(BroadcastChannel* aBC)
+      : DiscardableRunnable("BroadcastChannel CloseRunnable"), mBC(aBC) {
+    MOZ_ASSERT(mBC);
+  }
 
   NS_IMETHOD Run() override {
     mBC->Shutdown();
     return NS_OK;
   }
 
-  nsresult Cancel() override { return NS_OK; }
-
  private:
   ~CloseRunnable() = default;
 
   RefPtr<BroadcastChannel> mBC;
 };
 
-NS_IMPL_ISUPPORTS(CloseRunnable, nsICancelableRunnable, nsIRunnable)
-
 class TeardownRunnable {
  protected:
   explicit TeardownRunnable(BroadcastChannelChild* aActor) : mActor(aActor) {

dom/workers/WorkerPrivate.cpp

@@ -199,13 +199,11 @@ class ExternalRunnableWrapper final : public WorkerRunnable {
   }
 
   nsresult Cancel() override {
-    nsresult rv;
-    nsCOMPtr<nsICancelableRunnable> cancelable =
+    nsCOMPtr<nsIDiscardableRunnable> doomed =
         do_QueryInterface(mWrappedRunnable);
-    MOZ_ASSERT(cancelable);  // We checked this earlier!
-    rv = cancelable->Cancel();
-    nsresult rv2 = WorkerRunnable::Cancel();
-    return NS_FAILED(rv) ? rv : rv2;
+    MOZ_ASSERT(doomed);  // We checked this earlier!
+    doomed->OnDiscard();
+    return WorkerRunnable::Cancel();
   }
 };
 
@@ -1571,9 +1569,11 @@ already_AddRefed<WorkerRunnable> WorkerPrivate::MaybeWrapAsWorkerRunnable(
     return workerRunnable.forget();
   }
 
-  nsCOMPtr<nsICancelableRunnable> cancelable = do_QueryInterface(runnable);
-  if (!cancelable) {
-    MOZ_CRASH("All runnables destined for a worker thread must be cancelable!");
+  nsCOMPtr<nsIDiscardableRunnable> maybe = do_QueryInterface(runnable);
+  if (!maybe) {
+    MOZ_CRASH(
+        "All runnables destined for a worker thread must be "
+        "nsIDiscardableRunnable!");
   }
 
   workerRunnable = new ExternalRunnableWrapper(this, runnable);

dom/workers/WorkerThread.cpp

@@ -244,15 +244,16 @@ WorkerThread::Dispatch(already_AddRefed<nsIRunnable> aRunnable,
 
 #ifdef DEBUG
   if (runnable && !onWorkerThread) {
-    nsCOMPtr<nsICancelableRunnable> cancelable = do_QueryInterface(runnable);
+    nsCOMPtr<nsIDiscardableRunnable> discardable = do_QueryInterface(runnable);
 
     {
       MutexAutoLock lock(mLock);
 
-      // Only enforce cancelable runnables after we've started the worker loop.
+      // Only enforce discardable runnables after we've started the worker loop.
       if (!mAcceptingNonWorkerRunnables) {
-        MOZ_ASSERT(cancelable,
-                   "Only nsICancelableRunnable may be dispatched to a worker!");
+        MOZ_ASSERT(
+            discardable,
+            "Only nsIDiscardableRunnable may be dispatched to a worker!");
       }
     }
   }

ipc/glue/IPCStreamSource.cpp

@@ -23,12 +23,13 @@ using mozilla::wr::ByteBuffer;
 namespace mozilla {
 namespace ipc {
 
-class IPCStreamSource::Callback final : public nsIInputStreamCallback,
-                                        public nsIRunnable,
-                                        public nsICancelableRunnable {
+class IPCStreamSource::Callback final : public DiscardableRunnable,
+                                        public nsIInputStreamCallback {
  public:
   explicit Callback(IPCStreamSource* aSource)
-      : mSource(aSource), mOwningEventTarget(GetCurrentSerialEventTarget()) {
+      : DiscardableRunnable("IPCStreamSource::Callback"),
+        mSource(aSource),
+        mOwningEventTarget(GetCurrentSerialEventTarget()) {
     MOZ_ASSERT(mSource);
   }
 
@@ -60,12 +61,9 @@ class IPCStreamSource::Callback final : public nsIInputStreamCallback,
     return NS_OK;
   }
 
-  nsresult Cancel() override {
-    // Cancel() gets called when the Worker thread is being shutdown.  We have
+  // OnDiscard() gets called when the Worker thread is being shutdown.  We have
   // nothing to do here because IPCStreamChild handles this case via
   // the WorkerRef.
-    return NS_OK;
-  }
 
   void ClearSource() {
     MOZ_ASSERT(mOwningEventTarget->IsOnCurrentThread());
@@ -88,11 +86,11 @@ class IPCStreamSource::Callback final : public nsIInputStreamCallback,
 
   nsCOMPtr<nsISerialEventTarget> mOwningEventTarget;
 
-  NS_DECL_THREADSAFE_ISUPPORTS
+  NS_DECL_ISUPPORTS_INHERITED
 };
 
-NS_IMPL_ISUPPORTS(IPCStreamSource::Callback, nsIInputStreamCallback,
-                  nsIRunnable, nsICancelableRunnable);
+NS_IMPL_ISUPPORTS_INHERITED(IPCStreamSource::Callback, DiscardableRunnable,
+                            nsIInputStreamCallback);
 
 IPCStreamSource::IPCStreamSource(nsIAsyncInputStream* aInputStream)
     : mStream(aInputStream), mState(ePending) {

xpcom/base/CycleCollectedJSContext.cpp

@@ -504,8 +504,6 @@ void CycleCollectedJSContext::IsIdleGCTaskNeeded() const {
       }
       return NS_OK;
     }
-
-    nsresult Cancel() override { return NS_OK; }
   };
 
   if (Runtime()->IsIdleGCTaskNeeded()) {

xpcom/threads/IdleTaskRunner.cpp

@@ -29,7 +29,7 @@ IdleTaskRunner::IdleTaskRunner(
     const CallbackType& aCallback, const char* aRunnableName, uint32_t aDelay,
     int64_t aBudget, bool aRepeating,
     const MayStopProcessingCallbackType& aMayStopProcessing)
-    : IdleRunnable(aRunnableName),
+    : CancelableIdleRunnable(aRunnableName),
       mCallback(aCallback),
       mDelay(aDelay),
       mBudget(TimeDuration::FromMilliseconds(aBudget)),

xpcom/threads/IdleTaskRunner.h

@@ -19,7 +19,7 @@ namespace mozilla {
 // one has to either explicitly Cancel() the runner or have
 // MayContinueProcessing() callback return false to completely remove
 // the runner.
-class IdleTaskRunner final : public IdleRunnable {
+class IdleTaskRunner final : public CancelableIdleRunnable {
  public:
   // Return true if some meaningful work was done.
   using CallbackType = std::function<bool(TimeStamp aDeadline)>;

xpcom/threads/moz.build

@@ -30,6 +30,7 @@ XPCOM_MANIFESTS += [
 EXPORTS += [
     "MainThreadUtils.h",
     "nsICancelableRunnable.h",
+    "nsIDiscardableRunnable.h",
     "nsIIdleRunnable.h",
     "nsMemoryPressure.h",
     "nsProcess.h",

xpcom/threads/nsIDiscardableRunnable.h

@@ -0,0 +1,41 @@
+/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=2 et sw=2 tw=80: */
+/* 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/. */
+
+#ifndef XPCOM_THREADS_NSIDISCARDABLERUNNABLE_H_
+#define XPCOM_THREADS_NSIDISCARDABLERUNNABLE_H_
+
+#include "nsISupports.h"
+
+/**
+ * An interface implemented by nsIRunnable tasks for which nsIRunnable::Run()
+ * might not be called.
+ */
+#define NS_IDISCARDABLERUNNABLE_IID                  \
+  {                                                  \
+    0xde93dc4c, 0x755c, 0x4cdc, {                    \
+      0x96, 0x76, 0x35, 0xc6, 0x48, 0x81, 0x59, 0x78 \
+    }                                                \
+  }
+
+class NS_NO_VTABLE nsIDiscardableRunnable : public nsISupports {
+ public:
+  NS_DECLARE_STATIC_IID_ACCESSOR(NS_IDISCARDABLERUNNABLE_IID)
+
+  /**
+   * Called exactly once on a queued task only if nsIRunnable::Run() is not
+   * called.
+   */
+  virtual void OnDiscard() = 0;
+
+ protected:
+  nsIDiscardableRunnable() = default;
+  virtual ~nsIDiscardableRunnable() = default;
+};
+
+NS_DEFINE_STATIC_IID_ACCESSOR(nsIDiscardableRunnable,
+                              NS_IDISCARDABLERUNNABLE_IID)
+
+#endif  // XPCOM_THREADS_NSIDISCARDABLERUNNABLE_H_

xpcom/threads/nsThreadUtils.cpp

@@ -87,14 +87,27 @@ Runnable::GetName(nsACString& aName) {
 }
 #  endif
 
-NS_IMPL_ISUPPORTS_INHERITED(CancelableRunnable, Runnable, nsICancelableRunnable)
+NS_IMPL_ISUPPORTS_INHERITED(DiscardableRunnable, Runnable,
+                            nsIDiscardableRunnable)
+
+NS_IMPL_ISUPPORTS_INHERITED(CancelableRunnable, DiscardableRunnable,
+                            nsICancelableRunnable)
+
+void CancelableRunnable::OnDiscard() {
+  // Tasks that implement Cancel() can be safely cleaned up if it turns out
+  // that the task will not run.
+  (void)NS_WARN_IF(NS_FAILED(Cancel()));
+}
 
 nsresult CancelableRunnable::Cancel() {
   // Do nothing
   return NS_OK;
 }
 
-NS_IMPL_ISUPPORTS_INHERITED(IdleRunnable, CancelableRunnable, nsIIdleRunnable)
+NS_IMPL_ISUPPORTS_INHERITED(IdleRunnable, DiscardableRunnable, nsIIdleRunnable)
+
+NS_IMPL_ISUPPORTS_INHERITED(CancelableIdleRunnable, CancelableRunnable,
+                            nsIIdleRunnable)
 
 NS_IMPL_ISUPPORTS_INHERITED(PrioritizableRunnable, Runnable,
                             nsIRunnablePriority)

xpcom/threads/nsThreadUtils.h

@@ -21,6 +21,7 @@
 #include "mozilla/Tuple.h"
 #include "nsCOMPtr.h"
 #include "nsICancelableRunnable.h"
+#include "nsIDiscardableRunnable.h"
 #include "nsIIdlePeriod.h"
 #include "nsIIdleRunnable.h"
 #include "nsINamed.h"
@@ -406,15 +407,42 @@ class Runnable : public nsIRunnable
   Runnable& operator=(const Runnable&&) = delete;
 };
 
-// This class is designed to be subclassed.
-class CancelableRunnable : public Runnable, public nsICancelableRunnable {
+// This is a base class for tasks that might not be run, such as those that may
+// be dispatched to workers.
+// The owner of an event target will call either Run() or OnDiscard()
+// exactly once.
+// Derived classes should override Run().  An OnDiscard() override may
+// provide cleanup when Run() will not be called.
+class DiscardableRunnable : public Runnable, public nsIDiscardableRunnable {
  public:
   NS_DECL_ISUPPORTS_INHERITED
+  // nsIDiscardableRunnable
+  void OnDiscard() override {}
+
+  DiscardableRunnable() = delete;
+  explicit DiscardableRunnable(const char* aName) : Runnable(aName) {}
+
+ protected:
+  virtual ~DiscardableRunnable() = default;
+
+ private:
+  DiscardableRunnable(const DiscardableRunnable&) = delete;
+  DiscardableRunnable& operator=(const DiscardableRunnable&) = delete;
+  DiscardableRunnable& operator=(const DiscardableRunnable&&) = delete;
+};
+
+// This class is designed to be subclassed.
+class CancelableRunnable : public DiscardableRunnable,
+                           public nsICancelableRunnable {
+ public:
+  NS_DECL_ISUPPORTS_INHERITED
+  // nsIDiscardableRunnable
+  void OnDiscard() override;
   // nsICancelableRunnable
   virtual nsresult Cancel() override;
 
   CancelableRunnable() = delete;
-  explicit CancelableRunnable(const char* aName) : Runnable(aName) {}
+  explicit CancelableRunnable(const char* aName) : DiscardableRunnable(aName) {}
 
  protected:
   virtual ~CancelableRunnable() = default;
@@ -426,12 +454,12 @@ class CancelableRunnable : public Runnable, public nsICancelableRunnable {
 };
 
 // This class is designed to be subclassed.
-class IdleRunnable : public CancelableRunnable, public nsIIdleRunnable {
+class IdleRunnable : public DiscardableRunnable, public nsIIdleRunnable {
  public:
   NS_DECL_ISUPPORTS_INHERITED
 
-  IdleRunnable() : CancelableRunnable("IdleRunnable") {}
-  explicit IdleRunnable(const char* aName) : CancelableRunnable(aName) {}
+  IdleRunnable() : DiscardableRunnable("IdleRunnable") {}
+  explicit IdleRunnable(const char* aName) : DiscardableRunnable(aName) {}
 
  protected:
   virtual ~IdleRunnable() = default;
@@ -442,6 +470,25 @@ class IdleRunnable : public CancelableRunnable, public nsIIdleRunnable {
   IdleRunnable& operator=(const IdleRunnable&&) = delete;
 };
 
+// This class is designed to be subclassed.
+class CancelableIdleRunnable : public CancelableRunnable,
+                               public nsIIdleRunnable {
+ public:
+  NS_DECL_ISUPPORTS_INHERITED
+
+  CancelableIdleRunnable() : CancelableRunnable("CancelableIdleRunnable") {}
+  explicit CancelableIdleRunnable(const char* aName)
+      : CancelableRunnable(aName) {}
+
+ protected:
+  virtual ~CancelableIdleRunnable() = default;
+
+ private:
+  CancelableIdleRunnable(const CancelableIdleRunnable&) = delete;
+  CancelableIdleRunnable& operator=(const CancelableIdleRunnable&) = delete;
+  CancelableIdleRunnable& operator=(const CancelableIdleRunnable&&) = delete;
+};
+
 // This class is designed to be a wrapper of a real runnable to support event
 // prioritizable.
 class PrioritizableRunnable : public Runnable, public nsIRunnablePriority {
@@ -671,12 +718,13 @@ class nsRunnableMethod
           Kind == mozilla::RunnableKind::Standard, mozilla::Runnable,
           std::conditional_t<Kind == mozilla::RunnableKind::Cancelable,
                              mozilla::CancelableRunnable,
-                             mozilla::IdleRunnable>>,
+                             mozilla::CancelableIdleRunnable>>,
       protected mozilla::detail::TimerBehaviour<Kind> {
   using BaseType = std::conditional_t<
       Kind == mozilla::RunnableKind::Standard, mozilla::Runnable,
       std::conditional_t<Kind == mozilla::RunnableKind::Cancelable,
-                         mozilla::CancelableRunnable, mozilla::IdleRunnable>>;
+                         mozilla::CancelableRunnable,
+                         mozilla::CancelableIdleRunnable>>;
 
  public:
   nsRunnableMethod(const char* aName) : BaseType(aName) {}
@@ -1123,7 +1171,8 @@ class RunnableMethodImpl final
   virtual ~RunnableMethodImpl() { Revoke(); };
   static void TimedOut(nsITimer* aTimer, void* aClosure) {
     static_assert(IsIdle(Kind), "Don't use me!");
-    RefPtr<IdleRunnable> r = static_cast<IdleRunnable*>(aClosure);
+    RefPtr<CancelableIdleRunnable> r =
+        static_cast<CancelableIdleRunnable*>(aClosure);
     r->SetDeadline(TimeStamp());
     r->Run();
     r->Cancel();