Bug 1752351 - Do not rely on a timer for canvas.captureStream's TimerDriver. r=jib

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

dom/html/HTMLCanvasElement.cpp

@@ -191,7 +191,7 @@ class RequestedFrameRefreshObserver : public nsARefreshObserver {
 
     mOwningElement->ProcessDestroyedFrameListeners();
 
-    if (!mOwningElement->IsFrameCaptureRequested()) {
+    if (!mOwningElement->IsFrameCaptureRequested(aTime)) {
       return;
     }
 
@@ -1327,13 +1327,13 @@ nsresult HTMLCanvasElement::RegisterFrameCaptureListener(
   return NS_OK;
 }
 
-bool HTMLCanvasElement::IsFrameCaptureRequested() const {
+bool HTMLCanvasElement::IsFrameCaptureRequested(const TimeStamp& aTime) const {
   for (WeakPtr<FrameCaptureListener> listener : mRequestedFrameListeners) {
     if (!listener) {
       continue;
     }
 
-    if (listener->FrameCaptureRequested()) {
+    if (listener->FrameCaptureRequested(aTime)) {
       return true;
     }
   }

dom/html/HTMLCanvasElement.h

@@ -83,23 +83,18 @@ class HTMLCanvasElementObserver final : public nsIObserver {
  * FrameCaptureListener is used by captureStream() as a way of getting video
  * frames from the canvas. On a refresh driver tick after something has been
  * drawn to the canvas since the last such tick, all registered
- * FrameCaptureListeners whose `mFrameCaptureRequested` equals `true`,
- * will be given a copy of the just-painted canvas.
+ * FrameCaptureListeners that report true for FrameCaptureRequested() will be
+ * given a copy of the just-painted canvas.
  * All FrameCaptureListeners get the same copy.
  */
 class FrameCaptureListener : public SupportsWeakPtr {
  public:
-  FrameCaptureListener() : mFrameCaptureRequested(false) {}
-
-  /*
-   * Called when a frame capture is desired on next paint.
-   */
-  void RequestFrameCapture() { mFrameCaptureRequested = true; }
+  FrameCaptureListener() = default;
 
   /*
    * Indicates to the canvas whether or not this listener has requested a frame.
    */
-  bool FrameCaptureRequested() const { return mFrameCaptureRequested; }
+  virtual bool FrameCaptureRequested(const TimeStamp& aTime) const = 0;
 
   /*
    * Interface through which new video frames will be provided while
@@ -110,8 +105,6 @@ class FrameCaptureListener : public SupportsWeakPtr {
 
  protected:
   virtual ~FrameCaptureListener() = default;
-
-  bool mFrameCaptureRequested;
 };
 
 class HTMLCanvasElement final : public nsGenericHTMLElement,
@@ -249,7 +242,7 @@ class HTMLCanvasElement final : public nsGenericHTMLElement,
    * Returns true when there is at least one registered FrameCaptureListener
    * that has requested a frame capture.
    */
-  bool IsFrameCaptureRequested() const;
+  bool IsFrameCaptureRequested(const TimeStamp& aTime) const;
 
   /*
    * Processes destroyed FrameCaptureListeners and removes them if necessary.

dom/media/CanvasCaptureMediaStream.cpp

@@ -28,9 +28,6 @@ OutputStreamDriver::OutputStreamDriver(SourceMediaTrack* aSourceStream,
       mPrincipalHandle(aPrincipalHandle) {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(mSourceStream);
-
-  // All CanvasCaptureMediaStreams shall at least get one frame.
-  mFrameCaptureRequested = true;
 }
 
 OutputStreamDriver::~OutputStreamDriver() {
@@ -65,49 +62,48 @@ class TimerDriver : public OutputStreamDriver {
   explicit TimerDriver(SourceMediaTrack* aSourceStream, const double& aFPS,
                        const PrincipalHandle& aPrincipalHandle)
       : OutputStreamDriver(aSourceStream, aPrincipalHandle),
-        mFPS(aFPS),
-        mTimer(nullptr) {
-    if (mFPS == 0.0) {
-      return;
+        mFrameInterval(aFPS == 0.0 ? TimeDuration::Forever()
+                                   : TimeDuration::FromSeconds(1.0 / aFPS)) {}
+
+  void RequestFrameCapture() override { mExplicitCaptureRequested = true; }
+
+  bool FrameCaptureRequested(const TimeStamp& aTime) const override {
+    if (mLastFrameTime.IsNull()) {
+      // All CanvasCaptureMediaStreams shall at least get one frame.
+      return true;
     }
 
-    NS_NewTimerWithFuncCallback(
-        getter_AddRefs(mTimer), &TimerTick, this, int(1000 / mFPS),
-        nsITimer::TYPE_REPEATING_SLACK, "dom::TimerDriver::TimerDriver");
-  }
+    if (mExplicitCaptureRequested) {
+      return true;
+    }
 
-  static void TimerTick(nsITimer* aTimer, void* aClosure) {
-    MOZ_ASSERT(aClosure);
-    TimerDriver* driver = static_cast<TimerDriver*>(aClosure);
+    if ((aTime - mLastFrameTime) >= mFrameInterval) {
+      return true;
+    }
 
-    driver->RequestFrameCapture();
+    return false;
   }
 
   void NewFrame(already_AddRefed<Image> aImage,
                 const TimeStamp& aTime) override {
     RefPtr<Image> image = aImage;
 
-    if (!mFrameCaptureRequested) {
+    if (!FrameCaptureRequested(aTime)) {
       return;
     }
 
-    mFrameCaptureRequested = false;
+    mLastFrameTime = aTime;
+    mExplicitCaptureRequested = false;
     SetImage(std::move(image), aTime);
   }
 
-  void Forget() override {
-    if (mTimer) {
-      mTimer->Cancel();
-      mTimer = nullptr;
-    }
-  }
-
  protected:
   virtual ~TimerDriver() = default;
 
  private:
-  const double mFPS;
-  nsCOMPtr<nsITimer> mTimer;
+  const TimeDuration mFrameInterval;
+  bool mExplicitCaptureRequested = false;
+  TimeStamp mLastFrameTime;
 };
 
 // ----------------------------------------------------------------------
@@ -118,13 +114,14 @@ class AutoDriver : public OutputStreamDriver {
                       const PrincipalHandle& aPrincipalHandle)
       : OutputStreamDriver(aSourceStream, aPrincipalHandle) {}
 
+  void RequestFrameCapture() override {}
+
+  bool FrameCaptureRequested(const TimeStamp& aTime) const override {
+    return true;
+  }
+
   void NewFrame(already_AddRefed<Image> aImage,
                 const TimeStamp& aTime) override {
-    // Don't reset `mFrameCaptureRequested` since AutoDriver shall always have
-    // `mFrameCaptureRequested` set to true.
-    // This also means we should accept every frame as NewFrame is called only
-    // after something changed.
-
     RefPtr<Image> image = aImage;
     SetImage(std::move(image), aTime);
   }
@@ -148,11 +145,7 @@ CanvasCaptureMediaStream::CanvasCaptureMediaStream(nsPIDOMWindowInner* aWindow,
                                                    HTMLCanvasElement* aCanvas)
     : DOMMediaStream(aWindow), mCanvas(aCanvas) {}
 
-CanvasCaptureMediaStream::~CanvasCaptureMediaStream() {
-  if (mOutputStreamDriver) {
-    mOutputStreamDriver->Forget();
-  }
-}
+CanvasCaptureMediaStream::~CanvasCaptureMediaStream() = default;
 
 JSObject* CanvasCaptureMediaStream::WrapObject(
     JSContext* aCx, JS::Handle<JSObject*> aGivenProto) {
@@ -195,7 +188,6 @@ void CanvasCaptureMediaStream::StopCapture() {
   }
 
   mOutputStreamDriver->EndTrack();
-  mOutputStreamDriver->Forget();
   mOutputStreamDriver = nullptr;
 }
 

dom/media/CanvasCaptureMediaStream.h

@@ -66,6 +66,12 @@ class OutputStreamDriver : public FrameCaptureListener {
 
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(OutputStreamDriver);
 
+  /*
+   * Called from js' requestFrame() when it wants the next painted frame to be
+   * explicitly captured.
+   */
+  virtual void RequestFrameCapture() = 0;
+
   /*
    * Sub classes can SetImage() to update the image being appended to the
    * output stream. It will be appended on the next NotifyPull from MTG.
@@ -78,12 +84,6 @@ class OutputStreamDriver : public FrameCaptureListener {
    */
   void EndTrack();
 
-  /*
-   * Makes sure any internal resources this driver is holding that may create
-   * reference cycles are released.
-   */
-  virtual void Forget() {}
-
   const RefPtr<SourceMediaTrack> mSourceStream;
   const PrincipalHandle mPrincipalHandle;